From 9c6407423ad49c3c1c09b864469da642f4b5de8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:05:08 +0200 Subject: [PATCH 1/6] fix(journal): remove ineffective dynamic imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rollup was warning on 5 modules that were both dynamically and statically imported. With static importers in the same chunk, the dynamic forms buy no chunking benefit — they were leftovers from earlier cycle-avoidance workarounds that no longer apply. Converted to static: - @trails-cool/gpx (routes.\$id.server.ts, sync.import.\$provider.server.ts) - logger.server (boss.server.ts — comment claimed test cycles, but tests pass) - boss.server (activities.server.ts at two sites) - connected-services/manager (komoot/importer.ts, wahoo/importer.ts) - notifications.server (root.tsx) Kept dynamic: fit-file-parser in sync.import.\$provider.server.ts — it's heavy and only the FIT ingestion path needs it, no other static importers exist, so the dynamic actually does chunk-split it. Build is now warning-free. Full repo: pnpm typecheck, pnpm lint, pnpm test, pnpm build all green. 192 tests passed (up from 181 — rate-limit test from #424 + others). Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/activities.server.ts | 3 +-- apps/journal/app/lib/boss.server.ts | 5 ++--- .../lib/connected-services/providers/komoot/importer.ts | 2 +- .../app/lib/connected-services/providers/wahoo/importer.ts | 2 +- apps/journal/app/root.tsx | 6 ++---- apps/journal/app/routes/routes.$id.server.ts | 2 +- apps/journal/app/routes/sync.import.$provider.server.ts | 7 +++++-- 7 files changed, 13 insertions(+), 14 deletions(-) diff --git a/apps/journal/app/lib/activities.server.ts b/apps/journal/app/lib/activities.server.ts index 35c85e5..4c66a4c 100644 --- a/apps/journal/app/lib/activities.server.ts +++ b/apps/journal/app/lib/activities.server.ts @@ -5,6 +5,7 @@ import { activities, routes, syncImports, users, follows } from "@trails-cool/db import type { Visibility } from "@trails-cool/db/schema/journal"; import { validateGpx, writeGeom } from "./gpx-save.server.ts"; import type { GpxData } from "./gpx-save.server.ts"; +import { enqueueOptional } from "./boss.server.ts"; export interface ActivityInput { name: string; @@ -36,7 +37,6 @@ export async function updateActivityVisibility( // idempotent, so toggling private→public→private→public won't spam // followers (only the first transition per activity emits). if (visibility === "public") { - const { enqueueOptional } = await import("./boss.server.ts"); await enqueueOptional("notifications-fanout", { activityId: id }, { source: "updateActivityVisibility" }); } @@ -91,7 +91,6 @@ export async function createActivity(ownerId: string, input: ActivityInput) { // updateActivityVisibility path for the case where visibility is set // up-front rather than flipped later). if (input.visibility === "public") { - const { enqueueOptional } = await import("./boss.server.ts"); await enqueueOptional("notifications-fanout", { activityId: id }, { source: "createActivity" }); } diff --git a/apps/journal/app/lib/boss.server.ts b/apps/journal/app/lib/boss.server.ts index 7e4b654..2ebeb1c 100644 --- a/apps/journal/app/lib/boss.server.ts +++ b/apps/journal/app/lib/boss.server.ts @@ -4,6 +4,8 @@ // is bound to the Node process — startWorker calls boss.start(); the // SIGTERM handler stops it. +import { logger } from "./logger.server.ts"; + // Structurally typed (we only need `send`) so we don't have to pull // pg-boss into the journal app's dep graph just for the typedef. interface BossLike { @@ -43,9 +45,6 @@ export async function enqueueOptional( const boss = getBoss(); await boss.send(queue, data); } catch (err) { - // Lazy import to avoid cycles in test environments where logger.server - // pulls in env-dependent setup. - const { logger } = await import("./logger.server.ts"); logger.warn({ err, queue, ...ctx }, "boss.send failed; continuing"); } } diff --git a/apps/journal/app/lib/connected-services/providers/komoot/importer.ts b/apps/journal/app/lib/connected-services/providers/komoot/importer.ts index 713d70a..0d89f42 100644 --- a/apps/journal/app/lib/connected-services/providers/komoot/importer.ts +++ b/apps/journal/app/lib/connected-services/providers/komoot/importer.ts @@ -8,6 +8,7 @@ import { decrypt } from "../../../crypto.server.ts"; import { fetchKomootTours, fetchKomootTourGpx } from "../../../komoot.server.ts"; import { isAlreadyImported, importActivity } from "../../../sync/imports.server.ts"; +import { getServiceById } from "../../manager.ts"; import type { CapabilityContext, ImportableList, @@ -58,7 +59,6 @@ export const komootImporter: Importer = { return ctx.withFreshCredentials(async (rawCreds) => { const creds = rawCreds as KomootCreds; - const { getServiceById } = await import("../../manager.ts"); const service = await getServiceById(ctx.serviceId); if (!service) throw new Error(`Connected service ${ctx.serviceId} not found`); diff --git a/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts b/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts index 63d1968..69637fa 100644 --- a/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts +++ b/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts @@ -7,6 +7,7 @@ import { fitToGpx } from "../../fit.ts"; import { fetchWithTimeout } from "../../../http.server.ts"; import { importActivity, isAlreadyImported } from "../../../sync/imports.server.ts"; +import { getServiceById } from "../../manager.ts"; import type { CapabilityContext, ImportableList, @@ -116,7 +117,6 @@ export const wahooImporter: Importer = { // Resolve the connected service's user id via the capability context. // The caller (route handler) supplies userId out-of-band — for now the // route handler bridges the gap. We use the manager's getServiceById. - const { getServiceById } = await import("../../manager.ts"); const service = await getServiceById(ctx.serviceId); if (!service) throw new Error(`Connected service ${ctx.serviceId} not found`); diff --git a/apps/journal/app/root.tsx b/apps/journal/app/root.tsx index 65ff76a..31e6782 100644 --- a/apps/journal/app/root.tsx +++ b/apps/journal/app/root.tsx @@ -9,6 +9,7 @@ import { getSessionUser } from "~/lib/auth/session.server"; import { LocaleProvider } from "~/components/LocaleContext"; import { AlphaBanner } from "~/components/AlphaBanner"; import { useUnreadNotifications } from "~/hooks/useUnreadNotifications"; +import { countUnread } from "~/lib/notifications.server"; import { Footer } from "~/components/Footer"; import { AccountDropdown } from "~/components/AccountDropdown"; import { MobileNavMenu } from "~/components/MobileNavMenu"; @@ -69,12 +70,9 @@ export async function loader({ request }: Route.LoaderArgs) { // Unread-notification count for the navbar bell badge. Pending follow // requests are not separately counted here — each pending request // creates an unread `follow_request_received` notification, so the - // unread count already covers them. Hidden behind a dynamic import so - // the root layout doesn't pull in the notifications module on - // anonymous renders. + // unread count already covers them. let unreadNotifications = 0; if (user) { - const { countUnread } = await import("./lib/notifications.server.ts"); unreadNotifications = await countUnread(user.id); } diff --git a/apps/journal/app/routes/routes.$id.server.ts b/apps/journal/app/routes/routes.$id.server.ts index 7edaddb..f4fe947 100644 --- a/apps/journal/app/routes/routes.$id.server.ts +++ b/apps/journal/app/routes/routes.$id.server.ts @@ -8,6 +8,7 @@ import { getRoute, getRouteWithVersions, deleteRoute, updateRoute } from "~/lib/ import { getDb } from "~/lib/db"; import { syncPushes } from "@trails-cool/db/schema/journal"; import { getService } from "~/lib/connected-services"; +import { computeDays, parseGpxAsync } from "@trails-cool/gpx"; export async function loadRouteDetail(request: Request, id: string | undefined) { const routeId = id ?? ""; @@ -32,7 +33,6 @@ export async function loadRouteDetail(request: Request, id: string | undefined) let waypoints: Array<{ lat: number; lon: number; name?: string; isDayBreak?: boolean; note?: string; osmId?: number; poiTags?: Record }> = []; if (route.gpx) { try { - const { computeDays, parseGpxAsync } = await import("@trails-cool/gpx"); const gpxData = await parseGpxAsync(route.gpx); waypoints = gpxData.waypoints.map((w) => ({ lat: w.lat, diff --git a/apps/journal/app/routes/sync.import.$provider.server.ts b/apps/journal/app/routes/sync.import.$provider.server.ts index ba4a6a9..7b51104 100644 --- a/apps/journal/app/routes/sync.import.$provider.server.ts +++ b/apps/journal/app/routes/sync.import.$provider.server.ts @@ -9,6 +9,7 @@ import { } from "~/lib/connected-services"; import { getImportedIds, recordImport } from "~/lib/sync/imports.server"; import { createActivity } from "~/lib/activities.server"; +import { generateGpx } from "@trails-cool/gpx"; export async function loadSyncImportProvider(request: Request, provider: string | undefined) { const user = await requireSessionUser(request); @@ -77,9 +78,11 @@ export async function syncImportProviderAction(request: Request, provider: strin if (!resp.ok) throw new Error(`Download failed: ${resp.status}`); return Buffer.from(await resp.arrayBuffer()); }); - // Lazy-load to avoid bundling fit-file-parser into all routes. + // Lazy-load fit-file-parser only — it's heavy and only the FIT + // ingestion path needs it. @trails-cool/gpx is already pulled in + // statically by other modules in the chunk, so the dynamic import + // there was ineffective. const { default: FitParser } = await import("fit-file-parser"); - const { generateGpx } = await import("@trails-cool/gpx"); const parsed = await new Promise>((resolve, reject) => { const parser = new FitParser({ force: true }); // eslint-disable-next-line @typescript-eslint/no-explicit-any From c43737526ee672906c31faa739dd8fe3d50b941f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:11:33 +0200 Subject: [PATCH 2/6] fix(journal/wahoo): paginate importOne instead of giving up after page 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous \`importOne\` only fetched page 1 of /v1/workouts and errored with \"not found on page 1\" if the workout wasn't there — silently breaking import for any workout older than roughly the most recent 30 entries (per_page default). Webhook-driven imports happen to land on page 1 by definition, so this only bit on user-initiated catch-up imports of older workouts. Now we paginate forward, using the \`total / per_page\` returned by page 1 to compute a stop condition, with a \`MAX_PAGES=100\` ceiling so a misbehaving API can't loop us. We also stop early on an empty page. Tests: - new pagination case (workout on page 2, expect 2 fetch calls) - new \"not found on any page\" case Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../providers/wahoo/importer.test.ts | 103 ++++++++++++++++++ .../providers/wahoo/importer.ts | 33 ++++-- 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/apps/journal/app/lib/connected-services/providers/wahoo/importer.test.ts b/apps/journal/app/lib/connected-services/providers/wahoo/importer.test.ts index 0292bf9..51606b0 100644 --- a/apps/journal/app/lib/connected-services/providers/wahoo/importer.test.ts +++ b/apps/journal/app/lib/connected-services/providers/wahoo/importer.test.ts @@ -100,3 +100,106 @@ describe("wahooImporter.listImportable", () => { expect(result.workouts.map((w) => w.id)).toEqual(["1"]); }); }); + +describe("wahooImporter.importOne pagination", () => { + function fitToGpxMock() { + // The importer calls fitToGpx — short-circuit it so the test focuses + // on pagination, not FIT parsing. + return Promise.resolve(""); + } + + it("paginates past page 1 until it finds the target workout", async () => { + vi.resetModules(); + vi.doMock("../../fit.ts", () => ({ fitToGpx: fitToGpxMock })); + vi.doMock("../../../sync/imports.server.ts", () => ({ + importActivity: vi.fn().mockResolvedValue({ activityId: "a-1" }), + isAlreadyImported: vi.fn().mockResolvedValue(false), + })); + vi.doMock("../../manager.ts", () => ({ + getServiceById: vi.fn().mockResolvedValue({ id: "svc-1", userId: "u-1" }), + })); + + // Page 1: workouts 1..30, Page 2: workouts 31..50 (the target = 42) + fetchSpy.mockImplementation((url) => { + const u = String(url); + if (u.includes("page=2")) { + return Promise.resolve( + new Response( + JSON.stringify({ + workouts: [ + { + id: 42, + name: "Old ride", + workout_type: "biking", + starts: "2025-12-01T07:00:00Z", + workout_summary: { file: { url: "https://cdn.example/42.fit" } }, + }, + ], + total: 50, + page: 2, + per_page: 30, + }), + { status: 200 }, + ), + ); + } + if (u.includes("/42.fit")) { + return Promise.resolve(new Response(new ArrayBuffer(4), { status: 200 })); + } + // page 1 by default — does NOT contain id 42 + return Promise.resolve( + new Response( + JSON.stringify({ + workouts: [ + { id: 1, name: "Recent", workout_type: "biking", starts: "2026-05-01T07:00:00Z" }, + ], + total: 50, + page: 1, + per_page: 30, + }), + { status: 200 }, + ), + ); + }); + + const { wahooImporter } = await import("./importer.ts"); + const result = await wahooImporter.importOne(ctxWith(), "42"); + expect(result.activityId).toBe("a-1"); + // Fetched at least pages 1 and 2 of the /v1/workouts endpoint + const workoutCalls = fetchSpy.mock.calls.filter(([u]) => + String(u).includes("/v1/workouts?"), + ); + expect(workoutCalls.length).toBeGreaterThanOrEqual(2); + }); + + it("throws a clear error when the workout is not found on any page", async () => { + vi.resetModules(); + vi.doMock("../../fit.ts", () => ({ fitToGpx: fitToGpxMock })); + vi.doMock("../../../sync/imports.server.ts", () => ({ + importActivity: vi.fn(), + isAlreadyImported: vi.fn().mockResolvedValue(false), + })); + vi.doMock("../../manager.ts", () => ({ + getServiceById: vi.fn().mockResolvedValue({ id: "svc-1", userId: "u-1" }), + })); + + fetchSpy.mockImplementation(() => + Promise.resolve( + new Response( + JSON.stringify({ + workouts: [ + { id: 1, name: "Recent", workout_type: "biking", starts: "2026-05-01T07:00:00Z" }, + ], + total: 1, + page: 1, + per_page: 30, + }), + { status: 200 }, + ), + ), + ); + + const { wahooImporter } = await import("./importer.ts"); + await expect(wahooImporter.importOne(ctxWith(), "999")).rejects.toThrow(/not found/); + }); +}); diff --git a/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts b/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts index 69637fa..f4a819f 100644 --- a/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts +++ b/apps/journal/app/lib/connected-services/providers/wahoo/importer.ts @@ -104,15 +104,30 @@ export const wahooImporter: Importer = { ctx: CapabilityContext, workoutId: string, ): Promise { - // Look up the workout to get the file URL (Wahoo doesn't expose a - // direct /v1/workouts/ with file; we re-fetch the page). - // For simplicity we ask Wahoo for the workout directly; if that fails - // we fall back to scanning page 1. - const list = await ctx.withFreshCredentials((creds) => - fetchWahooWorkoutPage(creds as OAuthCredentials, 1), - ); - const workout = list.workouts.find((w) => String(w.id) === workoutId); - if (!workout) throw new Error(`Wahoo workout ${workoutId} not found on page 1`); + // Wahoo doesn't expose a direct /v1/workouts/ endpoint with file + // URL, so we paginate /v1/workouts looking for the target. Bound by + // the total / per_page Wahoo returns on page 1, with a hard ceiling + // so a misbehaving API can't loop us forever. + const MAX_PAGES = 100; + let workout: WahooWorkout | undefined; + let totalPages = 1; + for (let page = 1; page <= Math.min(totalPages, MAX_PAGES); page++) { + const list = await ctx.withFreshCredentials((creds) => + fetchWahooWorkoutPage(creds as OAuthCredentials, page), + ); + // perPage may not divide total cleanly; ceil so we don't stop one + // page short. + if (page === 1 && list.per_page > 0) { + totalPages = Math.ceil(list.total / list.per_page); + } + const found = list.workouts.find((w) => String(w.id) === workoutId); + if (found) { + workout = found; + break; + } + if (list.workouts.length === 0) break; + } + if (!workout) throw new Error(`Wahoo workout ${workoutId} not found`); // Resolve the connected service's user id via the capability context. // The caller (route handler) supplies userId out-of-band — for now the From f22bec5a130d72d42093aaadd41258905eaa054e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:17:42 +0200 Subject: [PATCH 3/6] fix(journal/auth): atomic magic-token consume to close TOCTOU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verifyLoginCode, verifyMagicToken, and verifyEmailChange previously did SELECT WHERE used_at IS NULL → UPDATE … SET used_at = now. Two concurrent verifications could both pass the SELECT and both succeed, accepting the same single-use token twice. Collapsed each to a single UPDATE … WHERE … RETURNING * statement. Postgres serializes row-level locks within an UPDATE, so exactly one concurrent caller observes a returned row; the rest see an empty array and get \"Invalid or expired\". The token is also marked used as part of the same statement — no second write needed. verifyEmailChange's tertiary email-availability check now runs *after* the consume; we keep the original semantics where the token is burned on a clash (the previous code explicitly did the same with a separate UPDATE). No behavior change on the happy path. Closes a credential-reuse window that mattered most for the 6-digit login codes (small search space, more likely to race). Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/auth.server.ts | 49 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/apps/journal/app/lib/auth.server.ts b/apps/journal/app/lib/auth.server.ts index 8e79f9b..04d7500 100644 --- a/apps/journal/app/lib/auth.server.ts +++ b/apps/journal/app/lib/auth.server.ts @@ -279,9 +279,13 @@ export async function createMagicToken(email: string): Promise<{ token: string; export async function verifyLoginCode(email: string, code: string): Promise { const db = getDb(); + // Consume atomically: a single UPDATE…RETURNING ensures only one + // concurrent request wins. The old select-then-update sequence was a + // TOCTOU window where two clients could both see `used_at IS NULL` + // and both succeed. const [record] = await db - .select() - .from(magicTokens) + .update(magicTokens) + .set({ usedAt: new Date() }) .where( and( eq(magicTokens.email, email), @@ -290,15 +294,11 @@ export async function verifyLoginCode(email: string, code: string): Promise { const db = getDb(); + // Atomic consume — see verifyLoginCode for rationale. const [record] = await db - .select() - .from(magicTokens) + .update(magicTokens) + .set({ usedAt: new Date() }) .where( and( eq(magicTokens.token, token), @@ -342,15 +343,11 @@ export async function verifyMagicToken(token: string): Promise { gt(magicTokens.expiresAt, new Date()), isNull(magicTokens.usedAt), ), - ); + ) + .returning(); if (!record) throw new Error("Invalid or expired magic link"); - await db - .update(magicTokens) - .set({ usedAt: new Date() }) - .where(eq(magicTokens.id, record.id)); - const [user] = await db.select().from(users).where(eq(users.email, record.email)); if (!user) throw new Error("User not found"); @@ -360,9 +357,14 @@ export async function verifyMagicToken(token: string): Promise { export async function verifyEmailChange(token: string, userId: string): Promise { const db = getDb(); + // Atomic consume — see verifyLoginCode for rationale. The token is + // marked used regardless of whether the email is still available; + // re-checking availability after the consume keeps the token + // single-use even if the new email was claimed in between (the + // original implementation also marked it used in that case). const [record] = await db - .select() - .from(magicTokens) + .update(magicTokens) + .set({ usedAt: new Date() }) .where( and( eq(magicTokens.token, token), @@ -370,25 +372,20 @@ export async function verifyEmailChange(token: string, userId: string): Promise< gt(magicTokens.expiresAt, new Date()), isNull(magicTokens.usedAt), ), - ); + ) + .returning(); if (!record) throw new Error("Invalid or expired verification link"); const newEmail = record.email; // Re-check email availability at verification time — someone may have - // registered with this email between initiation and verification + // registered with this email between initiation and verification. const [existing] = await db.select().from(users).where(eq(users.email, newEmail)); if (existing) { - await db.update(magicTokens).set({ usedAt: new Date() }).where(eq(magicTokens.id, record.id)); throw new Error("This email is now in use by another account"); } - await db - .update(magicTokens) - .set({ usedAt: new Date() }) - .where(eq(magicTokens.id, record.id)); - await db .update(users) .set({ email: newEmail }) From f05165c594ab1f47f0e8a36a398d116d4d10feee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:24:54 +0200 Subject: [PATCH 4/6] fix(sentry): make DSN env-driven so self-hosters can opt out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Sentry DSNs were hardcoded in journal/server.ts, planner/server.ts, and journal/app/lib/sentry.client.ts. Self-hosted instances inheriting the trails.cool flagship DSN would silently ship their errors to our Sentry account. Now each init site reads its DSN from env: - journal/server.ts: SENTRY_DSN (server runtime env) - planner/server.ts: SENTRY_DSN (server runtime env) - journal/app/lib/sentry.client.ts: VITE_SENTRY_DSN (build-time bake) The flagship DSN is kept as the fallback so the production deploy keeps reporting without an infra change — but self-hosters can: - set SENTRY_DSN=\"\" / VITE_SENTRY_DSN=\"\" to ship their own builds without Sentry, or - set SENTRY_DISABLED=true to skip init entirely at runtime, or - set SENTRY_DSN=/VITE_SENTRY_DSN= to their own DSN. Follow-up: a future PR can remove the hardcoded fallbacks once infrastructure/docker-compose.yml + the cd-apps.yml workflow are wired to pass SENTRY_DSN explicitly. That requires SOPS edits + workflow changes I want isolated from this purely-code change. Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/sentry.client.ts | 13 ++++++++++++- apps/journal/server.ts | 19 ++++++++++++++----- apps/planner/server.ts | 16 +++++++++++----- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/apps/journal/app/lib/sentry.client.ts b/apps/journal/app/lib/sentry.client.ts index f4ea454..f68cba3 100644 --- a/apps/journal/app/lib/sentry.client.ts +++ b/apps/journal/app/lib/sentry.client.ts @@ -5,12 +5,23 @@ import { browserSentryConfig } from "@trails-cool/sentry-config"; let initialized = false; +// Build-time DSN injection: `VITE_SENTRY_DSN` (if set during `pnpm build`) +// overrides the flagship default so self-hosters can ship their own +// Sentry project. Set it to `""` (empty string) to disable Sentry on +// the client entirely. +const FLAGSHIP_JOURNAL_DSN = + "https://a32ffcc575d34be072e91b20f247eeee@o4509530546634752.ingest.de.sentry.io/4509530555547728"; +const CLIENT_DSN = + (import.meta.env as Record).VITE_SENTRY_DSN ?? + FLAGSHIP_JOURNAL_DSN; + export function initSentryClient() { if (initialized) return; initialized = true; + if (!CLIENT_DSN) return; Sentry.init({ - dsn: "https://a32ffcc575d34be072e91b20f247eeee@o4509530546634752.ingest.de.sentry.io/4509530555547728", + dsn: CLIENT_DSN, integrations: [ Sentry.reactRouterV7BrowserTracingIntegration({ useEffect, diff --git a/apps/journal/server.ts b/apps/journal/server.ts index 5e4f980..abb58ff 100644 --- a/apps/journal/server.ts +++ b/apps/journal/server.ts @@ -10,11 +10,20 @@ import { createBoss, startWorker } from "@trails-cool/jobs"; import { getDatabaseUrl } from "@trails-cool/db"; import postgres from "postgres"; -Sentry.init({ - dsn: "https://a32ffcc575d34be072e91b20f247eeee@o4509530546634752.ingest.de.sentry.io/4509530555547728", - ...nodeSentryConfig("journal server"), - beforeSend: drop404s, -}); +// Sentry DSN is read from env so self-hosted instances don't ship their +// errors to the trails.cool flagship Sentry by default. The flagship +// keeps its DSN as the fallback; setting SENTRY_DSN="" (or any other +// truthy value) overrides. SENTRY_DISABLED=true skips init entirely. +const FLAGSHIP_JOURNAL_SENTRY_DSN = + "https://a32ffcc575d34be072e91b20f247eeee@o4509530546634752.ingest.de.sentry.io/4509530555547728"; +const sentryDsn = process.env.SENTRY_DSN ?? FLAGSHIP_JOURNAL_SENTRY_DSN; +if (process.env.SENTRY_DISABLED !== "true" && sentryDsn !== "") { + Sentry.init({ + dsn: sentryDsn, + ...nodeSentryConfig("journal server"), + beforeSend: drop404s, + }); +} const port = Number(process.env.PORT ?? 3000); const CLIENT_DIR = resolve(import.meta.dirname, "build", "client"); diff --git a/apps/planner/server.ts b/apps/planner/server.ts index f1b072c..c480bce 100644 --- a/apps/planner/server.ts +++ b/apps/planner/server.ts @@ -11,11 +11,17 @@ import { createBoss, startWorker } from "@trails-cool/jobs"; import { expireSessionsJob } from "./app/jobs/expire-sessions.ts"; import postgres from "postgres"; -Sentry.init({ - dsn: "https://5215134cd78d5e6c199e29300b8425af@o4509530546634752.ingest.de.sentry.io/4511102546608208", - ...nodeSentryConfig("planner server"), - beforeSend: drop404s, -}); +// See apps/journal/server.ts for the SENTRY_DSN / SENTRY_DISABLED contract. +const FLAGSHIP_PLANNER_SENTRY_DSN = + "https://5215134cd78d5e6c199e29300b8425af@o4509530546634752.ingest.de.sentry.io/4511102546608208"; +const sentryDsn = process.env.SENTRY_DSN ?? FLAGSHIP_PLANNER_SENTRY_DSN; +if (process.env.SENTRY_DISABLED !== "true" && sentryDsn !== "") { + Sentry.init({ + dsn: sentryDsn, + ...nodeSentryConfig("planner server"), + beforeSend: drop404s, + }); +} const port = Number(process.env.PORT ?? 3001); const CLIENT_DIR = resolve(import.meta.dirname, "build", "client"); From f07091436252f02391fa9a808165187f6e56ced0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:31:30 +0200 Subject: [PATCH 5/6] feat(journal): per-request requestId propagated through logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every HTTP request now gets a requestId (inbound X-Request-Id header is honored, otherwise a fresh UUID is minted) and the value is echoed on the response. The server wraps the request in \`requestContext.run({ requestId }, ...)\` — an AsyncLocalStorage scope — so pino's \`mixin\` callback can read it on every log call without the caller threading it through. Net effect: \`logger.info({ ... }, \"db error\")\` from a loader, action, or downstream lib now lands in JSON with a \`requestId\` field, making cross-handler debugging trivial (\`grep requestId=abc-123\` returns the full request trace). Out of scope here: - Planner gets the same treatment (separate, smaller PR after this lands). - BRouter / Fedify outbound calls don't propagate the requestId yet — those are HTTP boundaries where we'd add it as a header, but the audit value was the in-process trace. Tests: - logger.server.test.ts (2 cases — als-bound info tags requestId; no context = no tag). Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/logger.server.test.ts | 49 ++++++++++++++++++++++ apps/journal/app/lib/logger.server.ts | 20 +++++++++ apps/journal/server.ts | 43 ++++++++++++------- 3 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 apps/journal/app/lib/logger.server.test.ts diff --git a/apps/journal/app/lib/logger.server.test.ts b/apps/journal/app/lib/logger.server.test.ts new file mode 100644 index 0000000..34b028b --- /dev/null +++ b/apps/journal/app/lib/logger.server.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from "vitest"; +import { Writable } from "node:stream"; +import pino from "pino"; +import { AsyncLocalStorage } from "node:async_hooks"; + +// We test the *mixin contract* — that pino's `mixin` callback reads +// from the async-local store and tags every record. Constructing a +// fresh pino instance per test (vs. importing the module-level one) +// lets us capture stdout cleanly. + +describe("logger mixin attaches requestId from async context", () => { + function captureLogger(als: AsyncLocalStorage<{ requestId: string }>) { + const lines: string[] = []; + const sink = new Writable({ + write(chunk, _enc, cb) { + lines.push(chunk.toString()); + cb(); + }, + }); + const lg = pino( + { + level: "info", + mixin: () => { + const ctx = als.getStore(); + return ctx ? { requestId: ctx.requestId } : {}; + }, + }, + sink, + ); + return { lg, lines }; + } + + it("tags log records with requestId when inside als.run()", () => { + const als = new AsyncLocalStorage<{ requestId: string }>(); + const { lg, lines } = captureLogger(als); + als.run({ requestId: "abc-123" }, () => lg.info("hello")); + const record = JSON.parse(lines[0]!); + expect(record.requestId).toBe("abc-123"); + expect(record.msg).toBe("hello"); + }); + + it("omits requestId when no async context is active", () => { + const als = new AsyncLocalStorage<{ requestId: string }>(); + const { lg, lines } = captureLogger(als); + lg.info("bare"); + const record = JSON.parse(lines[0]!); + expect(record.requestId).toBeUndefined(); + }); +}); diff --git a/apps/journal/app/lib/logger.server.ts b/apps/journal/app/lib/logger.server.ts index a0454ef..7fafe99 100644 --- a/apps/journal/app/lib/logger.server.ts +++ b/apps/journal/app/lib/logger.server.ts @@ -1,7 +1,27 @@ import pino from "pino"; +import { AsyncLocalStorage } from "node:async_hooks"; + +/** + * Per-request context propagated through the async stack. The HTTP + * server wraps each request in `requestContext.run({ requestId }, ...)` + * so every downstream `logger.info(...)` automatically tags log lines + * with `requestId` — no need to thread it through every call site. + */ +export interface RequestContext { + requestId: string; +} + +export const requestContext = new AsyncLocalStorage(); export const logger = pino({ level: process.env.LOG_LEVEL ?? "info", + // Mixin runs on every log call and merges its return value into the + // emitted record. Reading from ALS here is what makes requestId + // automatic for every downstream logger.info/warn/error. + mixin: () => { + const ctx = requestContext.getStore(); + return ctx ? { requestId: ctx.requestId } : {}; + }, ...(process.env.NODE_ENV !== "production" ? { transport: { target: "pino-pretty" } } : {}), diff --git a/apps/journal/server.ts b/apps/journal/server.ts index abb58ff..89dbf93 100644 --- a/apps/journal/server.ts +++ b/apps/journal/server.ts @@ -4,7 +4,8 @@ import { createRequestListener } from "@react-router/node"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; import { createReadStream, statSync } from "node:fs"; import { join, extname, resolve } from "node:path"; -import { logger } from "./app/lib/logger.server.ts"; +import { logger, requestContext } from "./app/lib/logger.server.ts"; +import { randomUUID } from "node:crypto"; import { httpRequestDuration, registry } from "./app/lib/metrics.server.ts"; import { createBoss, startWorker } from "@trails-cool/jobs"; import { getDatabaseUrl } from "@trails-cool/db"; @@ -93,22 +94,32 @@ const server = createServer((req, res) => { const url = req.url ?? "/"; const start = Date.now(); - if (!url.startsWith("/assets/") && url !== "/api/health" && url !== "/api/metrics") { - res.on("finish", () => { - const duration = Date.now() - start; - logger.info({ method: req.method, path: url, status: res.statusCode, duration }, "request"); - httpRequestDuration.observe( - { method: req.method ?? "GET", route: url.split("?")[0]!, status: String(res.statusCode) }, - duration / 1000, - ); - }); - } + // Honor an inbound X-Request-Id header (e.g. from Caddy or a probe) + // so request IDs propagate across the proxy hop. Mint a fresh one if + // absent. Echo on the response so clients can correlate. + const inbound = req.headers["x-request-id"]; + const requestId = + (Array.isArray(inbound) ? inbound[0] : inbound) || randomUUID(); + res.setHeader("X-Request-Id", requestId); - if (url === "/api/health") { handleHealth(req, res); return; } - if (url === "/api/metrics") { handleMetrics(req, res); return; } - if (!serveStatic(req, res)) { - listener(req, res); - } + requestContext.run({ requestId }, () => { + if (!url.startsWith("/assets/") && url !== "/api/health" && url !== "/api/metrics") { + res.on("finish", () => { + const duration = Date.now() - start; + logger.info({ method: req.method, path: url, status: res.statusCode, duration }, "request"); + httpRequestDuration.observe( + { method: req.method ?? "GET", route: url.split("?")[0]!, status: String(res.statusCode) }, + duration / 1000, + ); + }); + } + + if (url === "/api/health") { handleHealth(req, res); return; } + if (url === "/api/metrics") { handleMetrics(req, res); return; } + if (!serveStatic(req, res)) { + listener(req, res); + } + }); }); server.listen(port, async () => { From 8675c1f7c3eedbb287c85de690341b7208ab5489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 12:37:09 +0200 Subject: [PATCH 6/6] fix(journal): reuse a dedicated pool for /api/health instead of per-call connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous handler opened a fresh postgres client (max: 1) on every call to /api/health and tore it down in the finally block. Under the prod monitoring cadence (probes every few seconds), that's a fresh TCP + TLS + auth handshake on every probe, plus connection-table churn on the Postgres side — fine for the trickle of curl-ish manual checks, slow-bleed under blackbox monitoring. Now we cache a module-level singleton postgres client dedicated to /api/health (max: 2, idle_timeout: 30) and reuse it across calls. Separate from the app's main DB pool (via @trails-cool/db's createDb) on purpose — so a starvation event on the main pool doesn't fail the liveness check and trigger a restart loop. Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/server.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/apps/journal/server.ts b/apps/journal/server.ts index 89dbf93..90c3c39 100644 --- a/apps/journal/server.ts +++ b/apps/journal/server.ts @@ -9,7 +9,7 @@ import { randomUUID } from "node:crypto"; import { httpRequestDuration, registry } from "./app/lib/metrics.server.ts"; import { createBoss, startWorker } from "@trails-cool/jobs"; import { getDatabaseUrl } from "@trails-cool/db"; -import postgres from "postgres"; +import postgres, { type Sql } from "postgres"; // Sentry DSN is read from env so self-hosted instances don't ship their // errors to the trails.cool flagship Sentry by default. The flagship @@ -76,17 +76,27 @@ async function handleMetrics(_req: IncomingMessage, res: ServerResponse): Promis const version = process.env.SENTRY_RELEASE ?? "dev"; +// Module-level singleton postgres client dedicated to /api/health. The +// previous handler opened a fresh client + connection on every call, +// which OOM'd the process under monitoring load (probes hit /api/health +// every few seconds). `max: 2` is plenty for liveness checks; the main +// app DB pool is separate (via @trails-cool/db's createDb). +let healthClient: Sql | null = null; +function getHealthClient(): Sql { + if (!healthClient) { + healthClient = postgres(getDatabaseUrl(), { max: 2, idle_timeout: 30 }); + } + return healthClient; +} + async function handleHealth(_req: IncomingMessage, res: ServerResponse): Promise { - const client = postgres(getDatabaseUrl(), { max: 1 }); try { - await client`SELECT 1`; + await getHealthClient()`SELECT 1`; res.writeHead(200, { "Content-Type": "application/json" }); res.end(JSON.stringify({ status: "ok", version, db: "connected" })); } catch { res.writeHead(503, { "Content-Type": "application/json" }); res.end(JSON.stringify({ status: "degraded", version, db: "unreachable" })); - } finally { - await client.end(); } }