Skip to content

fix: read-after-write issues #215

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: read-after-write issues #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79a7997
fix: read-after-write issues
lindesvard Oct 19, 2025
738ed65
fix: coderabbit comments
lindesvard Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/api/src/controllers/ai.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@/utils/ai-tools';
import { HttpError } from '@/utils/errors';
import { db, getOrganizationByProjectIdCached } from '@openpanel/db';
import { getProjectAccessCached } from '@openpanel/trpc/src/access';
import { getProjectAccess } from '@openpanel/trpc/src/access';
import { type Message, appendResponseMessages, streamText } from 'ai';
import type { FastifyReply, FastifyRequest } from 'fastify';

Expand Down Expand Up @@ -37,7 +37,7 @@ export async function chat(
}

const organization = await getOrganizationByProjectIdCached(projectId);
const access = await getProjectAccessCached({
const access = await getProjectAccess({
projectId,
userId: session.userId,
});
Expand Down
24 changes: 15 additions & 9 deletions apps/api/src/controllers/webhook.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ export async function slackWebhook(
}
}

async function clearOrganizationCache(organizationId: string) {
const projects = await db.project.findMany({
where: {
organizationId,
},
});
for (const project of projects) {
await getOrganizationByProjectIdCached.clear(project.id);
}
}

export async function polarWebhook(
request: FastifyRequest<{
Querystring: unknown;
Expand Down Expand Up @@ -141,8 +152,11 @@ export async function polarWebhook(
},
data: {
subscriptionPeriodEventsCount: 0,
subscriptionPeriodEventsCountExceededAt: null,
},
});

await clearOrganizationCache(metadata.organizationId);
}
break;
}
Expand Down Expand Up @@ -205,15 +219,7 @@ export async function polarWebhook(
},
});

const projects = await db.project.findMany({
where: {
organizationId: metadata.organizationId,
},
});

for (const project of projects) {
await getOrganizationByProjectIdCached.clear(project.id);
}
await clearOrganizationCache(metadata.organizationId);

await publishEvent('organization', 'subscription_updated', {
organizationId: metadata.organizationId,
Expand Down
17 changes: 14 additions & 3 deletions apps/api/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ import Fastify from 'fastify';
import metricsPlugin from 'fastify-metrics';

import { generateId } from '@openpanel/common';
import type { IServiceClientWithProject } from '@openpanel/db';
import { getRedisPub } from '@openpanel/redis';
import {
type IServiceClientWithProject,
runWithAlsSession,
} from '@openpanel/db';
import { getCache, getRedisPub } from '@openpanel/redis';
import type { AppRouter } from '@openpanel/trpc';
import { appRouter, createContext } from '@openpanel/trpc';

import {
EMPTY_SESSION,
type SessionValidationResult,
decodeSessionToken,
validateSessionToken,
} from '@openpanel/auth';
import sourceMapSupport from 'source-map-support';
Expand Down Expand Up @@ -140,7 +144,14 @@ const startServer = async () => {
instance.addHook('onRequest', async (req) => {
if (req.cookies?.session) {
try {
const session = await validateSessionToken(req.cookies.session);
const sessionId = decodeSessionToken(req.cookies.session);
const session = await runWithAlsSession(sessionId, () =>
sessionId
? getCache(`validateSession:${sessionId}`, 60 * 5, async () =>
validateSessionToken(req.cookies.session),
)
: validateSessionToken(req.cookies.session),
);
Comment on lines +147 to +154
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

ALS context is limited to validation; make it request‑scoped for non‑TRPC routes.

runWithAlsSession(sessionId, () => ...) scopes ALS only around validation. Downstream handlers (e.g., /live, /webhook, /oauth, /misc, /ai) will miss ALS, so the Prisma session‑consistency extension can’t see sessionId during DB reads → potential read‑after‑write inconsistency outside TRPC.

Prefer entering ALS for the entire request after you derive sessionId:

  • If @openpanel/db can expose enterAlsSession(sessionId), call it here before validation and leave it set for the remainder of the request.
  • Otherwise, add a request‑level middleware that wraps route handling under als.run/enterWith.

I can provide a small helper and wire‑up if useful.

Run to confirm coverage (expect TRPC covered; verify non‑TRPC):

🏁 Script executed:

#!/bin/bash
# TRPC ALS coverage
rg -n --hidden -g '!**/dist/**' 'sessionScopeMiddleware|runWithAlsSession\(' packages/trpc

# Non‑TRPC dashboard routes touching DB that may lack ALS
rg -n --hidden -g '!**/dist/**' -C2 'db\.' apps/api/src/routes -g '!**/trpc/**'

Length of output: 401

🏁 Script executed:

#!/bin/bash
# Check structure of apps/api/src to understand route organization
echo "=== Directory structure of apps/api/src ==="
fd -t f -e ts -e js apps/api/src --max-depth 3

# Search for route definitions/handlers in apps/api/src (more permissive pattern)
echo "=== Potential route handlers in apps/api/src ==="
rg -n --hidden -g '!**/dist/**' "app\.(get|post|put|patch|delete|use)\(|router\." apps/api/src | head -30

# Check if non-TRPC routes access database
echo "=== Non-TRPC files with db access ==="
rg -n --hidden -g '!**/dist/**' -g '!**/trpc/**' "db\." apps/api/src | head -20

# Look at index.ts to understand the endpoint structure
echo "=== Line count context around 147-154 in index.ts ==="
wc -l apps/api/src/index.ts

Length of output: 2163

🏁 Script executed:

#!/bin/bash
# View the validation code and surrounding context
echo "=== index.ts lines 130-200 (full context around 147-154) ==="
sed -n '130,200p' apps/api/src/index.ts

# Find where controllers are imported/used
echo "=== Controller imports and usage in index.ts ==="
rg -n 'controller|Controller' apps/api/src/index.ts | head -20

# Check if there's a middleware that sets ALS at request level
echo "=== Middleware patterns in index.ts ==="
rg -n --hidden -g '!**/dist/**' "middleware|use\(|als\." apps/api/src/index.ts | head -20

Length of output: 2551

ALS context is limited to validation; extend it to request-scoped for non-TRPC routes.

The onRequest hook (lines 147-154) validates the session within runWithAlsSession(sessionId, ...) scope, but this context exits after validation completes. The non-TRPC routers registered at lines 191-196 (/live, /webhook, /oauth, /misc, /ai) and their controllers (which we found making DB calls in healthcheck.controller.ts, ai.controller.ts, webhook.controller.ts, oauth-callback.controller.tsx, export.controller.ts) execute outside this ALS context, creating risk of read-after-write inconsistency during database operations.

TRPC is properly covered by its own sessionScopeMiddleware (at packages/trpc/src/trpc.ts:155–169), but non-TRPC routes lack request-level ALS wrapping.

Recommend one of:

  • If @openpanel/db can expose enterAlsSession(sessionId), call it in the hook before validation and leave it set for the entire request lifecycle.
  • Otherwise, add a request-level middleware that wraps all downstream route handling under als.run() or als.enterWith().
🤖 Prompt for AI Agents 
In apps/api/src/index.ts around lines 147 to 154, the ALS is only set during
session validation and exits before non-TRPC routes run; fix by establishing
request-scoped ALS for the whole request: if @openpanel/db exposes
enterAlsSession(sessionId) call it before validation and leave it set for the
request lifecycle, otherwise add a request-level middleware (placed before
registering /live, /webhook, /oauth, /misc, /ai routes) that wraps downstream
handling in als.run() or als.enterWith() using the decoded sessionId so all
subsequent controllers execute inside the same ALS context.

if (session.session) {
req.session = session;
}
Expand Down
4 changes: 3 additions & 1 deletion apps/start/src/hooks/use-session-extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ export function useSessionExtension() {
1000 * 60 * 5,
);

extendSessionFn();
// Delay initial call a bit to prioritize other requests
const timer = setTimeout(() => extendSessionFn(), 5000);

return () => {
clearTimeout(timer);
if (intervalRef.current) {
clearInterval(intervalRef.current);
}
Expand Down
13 changes: 11 additions & 2 deletions packages/auth/src/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,14 @@ export async function createDemoSession(
};
}

export const decodeSessionToken = (token: string): string | null => {
return token
? encodeHexLowerCase(sha256(new TextEncoder().encode(token)))
: null;
};

export async function validateSessionToken(
token: string | null,
token: string | null | undefined,
): Promise<SessionValidationResult> {
if (process.env.DEMO_USER_ID) {
return createDemoSession(process.env.DEMO_USER_ID);
Expand All @@ -69,7 +75,10 @@ export async function validateSessionToken(
if (!token) {
return EMPTY_SESSION;
}
const sessionId = encodeHexLowerCase(sha256(new TextEncoder().encode(token)));
const sessionId = decodeSessionToken(token);
if (!sessionId) {
return EMPTY_SESSION;
}
const result = await db.$primary().session.findUnique({
where: {
id: sessionId,
Expand Down
1 change: 1 addition & 0 deletions packages/db/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export * from './src/buffers';
export * from './src/types';
export * from './src/clickhouse/query-builder';
export * from './src/services/overview.service';
export * from './src/session-context';
3 changes: 3 additions & 0 deletions packages/db/src/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { createLogger } from '@openpanel/logger';

export const logger = createLogger({ name: 'db:prisma' });
24 changes: 15 additions & 9 deletions packages/db/src/prisma-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { createLogger } from '@openpanel/logger';
import { readReplicas } from '@prisma/extension-read-replicas';
import { type Organization, PrismaClient } from './generated/prisma/client';
import {
type Organization,
Prisma,
PrismaClient,
} from './generated/prisma/client';
import { logger } from './logger';
import { sessionConsistency } from './session-consistency';

export * from './generated/prisma/client';

const logger = createLogger({ name: 'db' });

const isWillBeCanceled = (
organization: Pick<
Organization,
Expand All @@ -30,11 +34,6 @@ const getPrismaClient = () => {
const prisma = new PrismaClient({
log: ['error'],
})
.$extends(
readReplicas({
url: process.env.DATABASE_URL_REPLICA ?? process.env.DATABASE_URL!,
}),
)
.$extends({
query: {
async $allOperations({ operation, model, args, query }) {
Expand All @@ -53,6 +52,8 @@ const getPrismaClient = () => {
},
},
})

.$extends(sessionConsistency())
.$extends({
result: {
organization: {
Expand Down Expand Up @@ -258,7 +259,12 @@ const getPrismaClient = () => {
},
},
},
});
})
.$extends(
readReplicas({
url: process.env.DATABASE_URL_REPLICA ?? process.env.DATABASE_URL!,
}),
);

return prisma;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/db/src/services/organization.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export async function getOrganizationByProjectId(projectId: string) {

export const getOrganizationByProjectIdCached = cacheable(
getOrganizationByProjectId,
60 * 60 * 24,
60 * 5,
);

export async function getInvites(organizationId: string) {
Expand Down
Loading