• pnpm-lock.yaml: Language not supported

apps/backend/prisma/migrations/20251125030551_add_sequence_id/migration.sql (1)

1-6: Consider documenting the sequence increment strategy.

The INCREMENT BY 11 in the global sequence is unusual and likely implements a per-instance ID distribution strategy for multi-instance deployments. Add an inline SQL comment explaining this choice for future maintainers.

Ref: Past review feedback suggested consolidating all sequence-related migrations and documenting each step. Consider grouping this with the subsequent shouldUpdateSequenceId and index migrations into a single, well-documented migration file.

-- Add this comment to explain the strategy
CREATE SEQUENCE global_seq_id
    AS BIGINT
    START 1
    INCREMENT BY 11  -- Increment by 11 to enable per-instance ID ranges (e.g., instance 0: 1,12,23...; instance 1: 2,13,24...) in multi-instance deployments
    NO MINVALUE
    NO MAXVALUE;

apps/backend/prisma/migrations/20251128210001_add_should_update_sequence_id_indexes/migration.sql (1)

1-7: LGTM!

The partial indexes on shouldUpdateSequenceId = TRUE are an efficient choice for queries filtering unsynced records, reducing index size by excluding already-synced rows. The naming and SPLIT_STATEMENT_SENTINEL structure are consistent with related migrations.

Consider adding an inline SQL comment explaining the partial index strategy:

-- Partial index for efficient querying of unsynced records
CREATE INDEX "ProjectUser_shouldUpdateSequenceId_idx" ON "ProjectUser"("shouldUpdateSequenceId") WHERE "shouldUpdateSequenceId" = TRUE;

apps/backend/prisma/migrations/20251128210000_add_should_update_sequence_id_columns/migration.sql (1)

1-7: LGTM!

The addition of shouldUpdateSequenceId with default TRUE across ProjectUser, ContactChannel, and DeletedRow ensures all existing records are marked for synchronization to external databases. This flag enables efficient querying of unsynced records paired with the partial indexes in the follow-up migration.

Add inline SQL comments to clarify the column's purpose:

-- Track which records need to be synced to external databases (reset to FALSE after sync)
ALTER TABLE "ProjectUser" ADD COLUMN "shouldUpdateSequenceId" BOOLEAN NOT NULL DEFAULT TRUE;

apps/backend/prisma/migrations/20251125180000_add_deleted_row_table/migration.sql (1)

8-9: Consider data retention strategy for fulfilled deletions.

The fulfilledAt timestamp tracks when deletions have been synced, but there's no documented retention policy or cleanup process for old rows. Over time, the DeletedRow table could accumulate millions of fulfilled records, impacting query performance and storage.

Recommend documenting a data retention policy (e.g., "delete rows where fulfilledAt < NOW() - INTERVAL '30 days'") and implementing a cleanup job.

apps/backend/prisma/migrations/20251125172224_add_outgoing_request/migration.sql (1)

2-12: Consider adding an index on createdAt for query performance.

The poller route orders by createdAt when claiming pending requests (ORDER BY "createdAt"). As the table grows, this ordering operation on unfulfilled requests could benefit from a composite index.

 CREATE INDEX  "OutgoingRequest_fulfilledAt_idx" ON "OutgoingRequest"("fulfilledAt");
+
+CREATE INDEX  "OutgoingRequest_createdAt_idx" ON "OutgoingRequest"("createdAt");

Alternatively, a composite index (fulfilledAt, createdAt) could further optimize the query pattern used in the poller.

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

81-87: Fragile environment detection pattern.

Using .includes("development") on the environment string is unusual and could match unintended values. The standard approach is direct equality comparison.

-if (getNodeEnvironment().includes("development") || getNodeEnvironment().includes("test")) {
+const env = getNodeEnvironment();
+if (env === "development" || env === "test") {

---

74-74: Consider defining a type for qstashOptions.

Casting to any loses type safety. Define an interface for the expected shape of qstashOptions to catch errors at compile time.

interface QstashOptions {
  url: string;
  body: unknown;
}
// ...
const options = request.qstashOptions as QstashOptions;

packages/stack-shared/src/config/schema.ts (1)

11-11: Unused import: yupArray

The import yupArray was added but doesn't appear to be used in this file. Based on the code, only yupTuple is used for array-like structures (Line 216).

-import { productSchema, userSpecifiedIdSchema, yupArray, yupBoolean, yupDate, yupMixed, yupNever, yupNumber, yupObject, yupRecord, yupString, yupTuple, yupUnion } from "../schema-fields";
+import { productSchema, userSpecifiedIdSchema, yupBoolean, yupDate, yupMixed, yupNever, yupNumber, yupObject, yupRecord, yupString, yupTuple, yupUnion } from "../schema-fields";

apps/backend/scripts/run-cron-jobs.ts (2)

25-29: Cron jobs don't run immediately on startup.

The setInterval schedules jobs to run after the first 60-second delay. Consider whether the jobs should also run immediately when the script starts:

  for (const endpoint of endpoints) {
+   run(endpoint); // Run immediately on startup
    setInterval(() => {
      run(endpoint);
    }, 60000);
  }

---

16-23: Consider adding a timeout to the fetch request.

The fetch call has no timeout. If the backend hangs, this could cause the cron job to stall indefinitely. Per coding guidelines, blocking calls without timeouts can cause cascading issues.

  const run = (endpoint: string) => runAsynchronously(async () => {
    console.log(`Running ${endpoint}...`);
-   const res = await fetch(`${baseUrl}${endpoint}`, {
-     headers: { 'Authorization': `Bearer ${cronSecret}` },
-   });
+   const controller = new AbortController();
+   const timeoutId = setTimeout(() => controller.abort(), 120000); // 2 minute timeout
+   try {
+     const res = await fetch(`${baseUrl}${endpoint}`, {
+       headers: { 'Authorization': `Bearer ${cronSecret}` },
+       signal: controller.signal,
+     });
+     if (!res.ok) throw new StackAssertionError(`Failed to call ${endpoint}: ${res.status} ${res.statusText}\n${await res.text()}`, { res });
+     console.log(`${endpoint} completed.`);
+   } finally {
+     clearTimeout(timeoutId);
+   }
-   if (!res.ok) throw new StackAssertionError(`Failed to call ${endpoint}: ${res.status} ${res.statusText}\n${await res.text()}`, { res });
-   console.log(`${endpoint} completed.`);
  });

apps/backend/src/app/api/latest/internal/external-db-sync/sequencer/route.ts (1)

50-65: Consider adding a circuit breaker for repeated failures.

The loop catches errors and continues (Lines 53-55), which could lead to repeatedly hitting the database with failing queries for the full 2-minute duration. Consider tracking consecutive failures and breaking early:

    let iterations = 0;
+   let consecutiveFailures = 0;
+   const maxConsecutiveFailures = 10;

    while (performance.now() - startTime < maxDurationMs) {
      try {
        await globalPrismaClient.$executeRaw`SELECT backfill_null_sequence_ids()`;
+       consecutiveFailures = 0; // Reset on success
      } catch (error) {
        console.warn('[sequencer] Failed to run backfill_null_sequence_ids:', error);
+       consecutiveFailures++;
+       if (consecutiveFailures >= maxConsecutiveFailures) {
+         console.error('[sequencer] Too many consecutive failures, breaking loop');
+         break;
+       }
      }

      iterations++;
      // ...
    }

apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-race.test.ts (1)

298-349: Consider parameterizing the long sleep duration.

The 70-second sleep makes this test slow for CI. If the poller interval is configurable in tests, you could reduce the wait time proportionally. Alternatively, mark this as a "slow test" that runs only in nightly builds.

apps/backend/prisma/schema.prisma (1)

895-911: Consider adding an index on sequenceId for DeletedRow if sync performance becomes a concern.

The model has a partial index on shouldUpdateSequenceId (migration 20251128210001) that helps the backfill function. The sync engine's query filters by tenancyId and tableName, then orders by sequenceId. While the @unique constraint on sequenceId provides an index, a composite index on (tenancyId, tableName, sequenceId) could optimize the sync fetch query if DeletedRow volumes grow significantly. This is an optional optimization rather than a requirement.

apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-advanced.test.ts (3)

2-2: Unused import: generateSecureRandomString

This import is not used anywhere in the file.

-import { generateSecureRandomString } from '@stackframe/stack-shared/dist/utils/crypto';

---

390-390: Missing test timeout

This test lacks an explicit timeout, unlike other tests in this file. Consider adding TEST_TIMEOUT for consistency and to prevent hanging tests.

-  });
+  }, TEST_TIMEOUT);

---

1011-1011: Prefer slice() over deprecated substr()

substr() is deprecated in favor of slice() or substring().

-    const testRunId = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
+    const testRunId = `${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;

apps/backend/src/lib/external-db-sync.ts (3)

48-69: Row-by-row processing may cause performance bottleneck

Each row is processed with a separate await externalClient.query() call. For high-volume syncs (e.g., the 200-user test), this results in 200+ sequential round-trips to the external database.

Consider batching multiple rows into a single transaction or using multi-value inserts for better throughput.

+  // Batch rows within a transaction for better performance
+  await externalClient.query('BEGIN');
+  try {
     for (const row of newRows) {
       const { tenancyId, ...rest } = row;
       await externalClient.query(upsertQuery, Object.values(rest));
     }
+    await externalClient.query('COMMIT');
+  } catch (err) {
+    await externalClient.query('ROLLBACK');
+    throw err;
+  }

---

82-90: Type safety: Excessive as any casts

Using as any to access sourceTables and targetTablePrimaryKey bypasses TypeScript's type checking. If the CompleteConfig type is correct, consider updating it to include these properties properly, or use type guards.

---

192-203: Sequential database syncing limits throughput

Each external database is synced sequentially. For tenants with multiple external DBs configured, this could be parallelized with Promise.all() to reduce total sync time.

-  for (const [dbId, dbConfig] of Object.entries(externalDatabases)) {
-    await syncDatabase(dbId, dbConfig, internalPrisma, tenancy.id);
-  }
+  await Promise.all(
+    Object.entries(externalDatabases).map(([dbId, dbConfig]) =>
+      syncDatabase(dbId, dbConfig, internalPrisma, tenancy.id)
+    )
+  );

apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-utils.ts (1)

209-215: Consider adding type for externalDatabases parameter

The externalDatabases parameter is typed as any. Consider using the proper type from the config schema for better type safety.

-export async function createProjectWithExternalDb(externalDatabases: any, projectOptions?: { display_name?: string, description?: string }) {
+export async function createProjectWithExternalDb(
+  externalDatabases: Record<string, { type: string; connectionString: string; mappings?: unknown }>,
+  projectOptions?: { display_name?: string; description?: string }
+) {

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to packages/stack-shared/src/config/schema.ts : Whenever making backwards-incompatible changes to the config schema, update the migration functions in `packages/stack-shared/src/config/schema.ts`

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to apps/backend/src/app/api/**/*.{ts,tsx} : The project uses a custom route handler system in the backend for consistent API responses

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Always add new E2E tests when changing the API or SDK interface

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to apps-{frontend,config}.ts{,x} : To update the list of apps available, edit `apps-frontend.tsx` and `apps-config.ts`

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

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

46-49: Consider renaming busySleepMs and revisiting the 2‑minute window

Minor/naming and tuning nit:

• busySleepMs is a bit misleading since this is not a busy-spin; something like pollSleepMs or loopSleepMs would be clearer.
• Previous feedback suggested a maxDurationMs of ~3 minutes to tolerate worst‑case Vercel cron skew (e.g. invocations at :05 and :06 still overlapping enough to drain backlogs). If that concern still applies, consider bumping from 2 to 3 minutes.

---

79-85: Document the host.docker.internal rewrite for dev/test

The dev/test rewrite of localhost/127.0.0.1 to host.docker.internal is subtle and environment‑specific. Adding a short comment here explaining that this is required for Dockerized e2e / local QStash to reach the backend from inside the container would help future readers understand why this block exists and avoid accidentally “simplifying” it away.

You might also cache getNodeEnvironment() in a local variable to avoid calling it twice, though that’s purely cosmetic.

---

51-68: Avoid marking OutgoingRequest as fulfilled before QStash publish succeeds

Right now claimPendingRequests sets fulfilledAt = NOW() in a transaction (lines 54-64), before processRequests actually calls upstash.publishJSON (lines 88-91). If publishJSON fails, the request stays marked as fulfilled and is never retried, so the external DB can silently fall out of sync. This matches prior automated feedback and should be treated as at-least-once delivery instead of maybe-once.

A safer pattern here is:

• In claimPendingRequests, only select pending rows with FOR UPDATE SKIP LOCKED and do not touch fulfilledAt.
• After a successful publishJSON call for a given request, set fulfilledAt = NOW() in a separate transaction.
• Leave failed publishes with fulfilledAt IS NULL so they get picked up again on the next poll.

Concretely:

-    async function claimPendingRequests(): Promise<OutgoingRequest[]> {
-      return await retryTransaction(globalPrismaClient, async (tx) => {
-        const rows = await tx.$queryRaw<OutgoingRequest[]>`
-          UPDATE "OutgoingRequest"
-          SET "fulfilledAt" = NOW()
-          WHERE "id" IN (
-            SELECT id
-            FROM "OutgoingRequest"
-            WHERE "fulfilledAt" IS NULL
-            ORDER BY "createdAt"
-            LIMIT 100
-            FOR UPDATE SKIP LOCKED
-          )
-          RETURNING *;
-        `;
-        return rows;
-      });
-    }
+    async function claimPendingRequests(): Promise<OutgoingRequest[]> {
+      return await retryTransaction(globalPrismaClient, async (tx) => {
+        const rows = await tx.$queryRaw<OutgoingRequest[]>`
+          SELECT "id", "createdAt", "qstashOptions", "fulfilledAt"
+          FROM "OutgoingRequest"
+          WHERE "fulfilledAt" IS NULL
+          ORDER BY "createdAt"
+          LIMIT 100
+          FOR UPDATE SKIP LOCKED;
+        `;
+        return rows;
+      });
+    }

and after a successful publish:

-          await upstash.publishJSON({
-            url: fullUrl,
-            body: options.body,
-          });
-
-          processed++;
+          await upstash.publishJSON({
+            url: fullUrl,
+            body: options.body,
+          });
+
+          // Only mark as fulfilled after successful QStash publish
+          await retryTransaction(globalPrismaClient, async (tx) => {
+            await tx.$queryRaw`
+              UPDATE "OutgoingRequest"
+              SET "fulfilledAt" = NOW()
+              WHERE "id" = ${request.id};
+            `;
+          });
+
+          processed++;

This removes the permanent-loss failure mode; downstream code should already be idempotent given the sequencing/sync use case, but if not, please double-check that repeated publishes are acceptable.

Also applies to: 88-93

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to apps/backend/src/app/api/**/*.{ts,tsx} : The project uses a custom route handler system in the backend for consistent API responses

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to **/*.{ts,tsx} : NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error). Use `runAsynchronously` or `runAsynchronouslyWithAlert` instead for asynchronous error handling

Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:21:39.123Z
Learning: Applies to **/*.{ts,tsx} : For blocking alerts and errors, never use `toast`, as they are easily missed by the user. Instead, use alerts

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

56-59: getLocalApiBaseUrl is defined but never used.

This function is not called anywhere in the file. The request processing uses getEnvVariable("NEXT_PUBLIC_STACK_API_URL") instead.

♻️ Remove unused function

-function getLocalApiBaseUrl(): string {
-  const prefix = getEnvVariable("NEXT_PUBLIC_STACK_PORT_PREFIX", "81");
-  return `http://localhost:${prefix}02`;
-}
-

.github/workflows/e2e-source-of-truth-api-tests.yaml (1)

18-26: Annotate placeholder DB credentials to avoid secret-scanner noise.

The connection strings in this env block include password-looking tokens, and Checkov already flagged CKV_SECRET_4. If these are intentionally dummy values for local containers, add a Checkov skip comment or clarify with a safer placeholder to prevent false positives and accidental reuse as real secrets.

✅ Minimal suppression option

     env:
       NODE_ENV: test
       STACK_ENABLE_HARDCODED_PASSKEY_CHALLENGE_FOR_TESTING: yes
       STACK_ACCESS_TOKEN_EXPIRATION_TIME: 30m
+      # checkov:skip=CKV_SECRET_4: local test DB credentials only (non-production)
       STACK_OVERRIDE_SOURCE_OF_TRUTH: '{"type": "postgres", "connectionString": "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@localhost:8128/source-of-truth-db?schema=sot-schema"}'
       STACK_TEST_SOURCE_OF_TRUTH: true
+      # checkov:skip=CKV_SECRET_4: local test DB credentials only (non-production)
       STACK_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@localhost:8128/stackframe"
       STACK_FORCE_EXTERNAL_DB_SYNC: "true"
       STACK_EXTERNAL_DB_SYNC_MAX_DURATION_MS: "20000"
       STACK_EXTERNAL_DB_SYNC_DIRECT: "false"

apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-basics.test.ts (1)

19-28: Consider adding proper typing for externalDatabases parameter.

The externalDatabases parameter uses any type. Per coding guidelines, any should be avoided. Consider defining a proper type or importing the expected shape from the sync configuration types.

Proposed type definition

+type ExternalDbConfig = {
+  [key: string]: {
+    type: 'postgres',
+    connectionString: string,
+  }
+};
+
 const createProjectWithExternalDb = (
-  externalDatabases: any,
+  externalDatabases: ExternalDbConfig,
   projectOptions?: { display_name?: string, description?: string }
 ) => {

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/external-db-sync/page-client.tsx (1)

153-156: Edge case: formatMillis(0) returns "—" instead of a formatted date.

The falsy check if (!value) will treat 0 as a missing value. While timestamp 0 (epoch) is unlikely in practice, this could mask issues during debugging.

Proposed fix

 function formatMillis(value: number | null) {
-  if (!value) return "—";
+  if (value === null || value === undefined) return "—";
   return new Date(value).toLocaleString();
 }