improvement(logs): move correlation ids from message tags into metadata and enforce it

.agents/skills/add-trigger/SKILL.md

@@ -411,7 +411,7 @@ export const {service}PollingHandler: PollingProviderHandler = {
       await markWebhookSuccess(webhookId, logger)
       return 'success'
     } catch (error) {
-      logger.error(`[${requestId}] Error processing {service} webhook ${webhookId}:`, error)
+      logger.error('Error processing {service} webhook:', error)
       await markWebhookFailed(webhookId, logger)
       return 'failure'
     }

AGENTS.md

@@ -5,7 +5,7 @@ This file (also `AGENTS.md`) holds the repo-wide rules. Area detail lives in `.c
 ## Global Standards
 
 - **Package manager**: `bun` and `bunx`, never `npm` and `npx`.
-- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Inside `withRouteHandler` the logger already carries the request ID — no manual `withMetadata({ requestId })`.
+- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Messages are static text; ids go in metadata (`logger.info('Sync started', { requestId, documentId })`), never a `[${requestId}]` tag or an interpolated request id, so lines group and filter by field (`check:source-conventions` rule `log-message-tag`). Inside `withRouteHandler` every line already carries the request ID, so a `generateRequestId()` id there needs no field and no manual `withMetadata({ requestId })`.
 - **Comments**: name things so the code explains itself. TSDoc documents exported APIs and non-obvious modules. An inline `//` is only for a terse, non-obvious *why*, or for a script-enforced `// <tag>: <reason>` annotation (`boundary-raw-fetch`, `double-cast-allowed`, `boundary-raw-json`, `untyped-response`, `rq-lint-allow`, `client-boundary-allow`, `utils-lint-allow`, …). History belongs in the commit message. No `====` separators, commented-out code, or ESLint directives (`eslint-disable`/`eslint-enable`; nothing runs ESLint; `check:comment-hygiene` enforces all three). The `/you-might-not-need-a-comment` skill applies this to a diff.
 - **ID generation**: `generateId()` (UUID v4, the default) or `generateShortId(size?)` (URL-safe, 21 chars by default) from `@sim/utils/id` — never `crypto.randomUUID()`, `nanoid`, or `uuid`. Both use `crypto.getRandomValues()`, so they also work in non-secure (HTTP) browsers. For other randomness, `@sim/utils/random` (`randomInt`, `randomFloat`, `randomItem`, `generateRandomBytes`, `generateRandomHex`) — never `Math.random()` or `crypto.randomBytes()`.
 - **Common utilities**: use the shared helpers from the `@sim/utils/<module>` subpaths instead of inline implementations (`check:utils` bans most of the inline forms below):

CLAUDE.md

@@ -5,7 +5,7 @@ This file (also `AGENTS.md`) holds the repo-wide rules. Area detail lives in `.c
 ## Global Standards
 
 - **Package manager**: `bun` and `bunx`, never `npm` and `npx`.
-- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Inside `withRouteHandler` the logger already carries the request ID — no manual `withMetadata({ requestId })`.
+- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Messages are static text; ids go in metadata (`logger.info('Sync started', { requestId, documentId })`), never a `[${requestId}]` tag or an interpolated request id, so lines group and filter by field (`check:source-conventions` rule `log-message-tag`). Inside `withRouteHandler` every line already carries the request ID, so a `generateRequestId()` id there needs no field and no manual `withMetadata({ requestId })`.
 - **Comments**: name things so the code explains itself. TSDoc documents exported APIs and non-obvious modules. An inline `//` is only for a terse, non-obvious *why*, or for a script-enforced `// <tag>: <reason>` annotation (`boundary-raw-fetch`, `double-cast-allowed`, `boundary-raw-json`, `untyped-response`, `rq-lint-allow`, `client-boundary-allow`, `utils-lint-allow`, …). History belongs in the commit message. No `====` separators, commented-out code, or ESLint directives (`eslint-disable`/`eslint-enable`; nothing runs ESLint; `check:comment-hygiene` enforces all three). The `/you-might-not-need-a-comment` skill applies this to a diff.
 - **ID generation**: `generateId()` (UUID v4, the default) or `generateShortId(size?)` (URL-safe, 21 chars by default) from `@sim/utils/id` — never `crypto.randomUUID()`, `nanoid`, or `uuid`. Both use `crypto.getRandomValues()`, so they also work in non-secure (HTTP) browsers. For other randomness, `@sim/utils/random` (`randomInt`, `randomFloat`, `randomItem`, `generateRandomBytes`, `generateRandomHex`) — never `Math.random()` or `crypto.randomBytes()`.
 - **Common utilities**: use the shared helpers from the `@sim/utils/<module>` subpaths instead of inline implementations (`check:utils` bans most of the inline forms below):

apps/sim/app/api/auth/oauth/credentials/route.ts

@@ -7,7 +7,6 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { oauthCredentialsQuerySchema } from '@/lib/api/contracts/credentials'
 import { getValidationErrorMessage } from '@/lib/api/server'
 import { AuthType, type AuthTypeValue, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
-import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
 import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
@@ -84,8 +83,6 @@ async function integrationsWithheldFromSession(
  * Get credentials for a specific provider
  */
 export const GET = withRouteHandler(async (request: NextRequest) => {
-  const requestId = generateRequestId()
-
   try {
     const { searchParams } = new URL(request.url)
     const rawQuery = {
@@ -100,11 +97,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
     if (!parseResult.success) {
       const refinementError = parseResult.error.issues.find((err) => err.code === 'custom')
       if (refinementError) {
-        logger.warn(`[${requestId}] Invalid query parameters: ${refinementError.message}`)
+        logger.warn(`Invalid query parameters: ${refinementError.message}`)
         return NextResponse.json({ error: refinementError.message }, { status: 400 })
       }
 
-      logger.warn(`[${requestId}] Invalid query parameters`, {
+      logger.warn('Invalid query parameters', {
         errors: parseResult.error.issues,
       })
 
@@ -119,7 +116,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
     // Authenticate requester (supports session and internal JWT)
     const authResult = await checkSessionOrInternalAuth(request)
     if (!authResult.success || !authResult.userId) {
-      logger.warn(`[${requestId}] Unauthenticated credentials request rejected`)
+      logger.warn('Unauthenticated credentials request rejected')
       return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
     }
     const requesterUserId = authResult.userId
@@ -132,7 +129,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
         action: 'read',
       })
       if (!workflowAuthorization.allowed) {
-        logger.warn(`[${requestId}] Forbidden credentials request for workflow`, {
+        logger.warn('Forbidden credentials request for workflow', {
           requesterUserId,
           workflowId,
           status: workflowAuthorization.status,
@@ -378,7 +375,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json({ credentials: [] }, { status: 200 })
   } catch (error) {
-    logger.error(`[${requestId}] Error fetching OAuth credentials`, error)
+    logger.error('Error fetching OAuth credentials', error)
     return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
   }
 })

apps/sim/app/api/auth/oauth/token/route.ts

@@ -33,7 +33,7 @@ const logger = createLogger('OAuthTokenAPI')
 export const POST = withRouteHandler(async (request: NextRequest) => {
   const requestId = generateRequestId()
 
-  logger.info(`[${requestId}] OAuth token API POST request received`)
+  logger.info('OAuth token API POST request received')
 
   try {
     const parsed = await parseRequest(
@@ -42,7 +42,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       {},
       {
         validationErrorResponse: (error) => {
-          logger.warn(`[${requestId}] Invalid token request`, { errors: error.issues })
+          logger.warn('Invalid token request', { errors: error.issues })
           return NextResponse.json(
             { error: getValidationErrorMessage(error, 'Validation failed') },
             { status: 400 }
@@ -64,14 +64,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     const callerUserId = parsed.data.query.userId
 
     if (credentialAccountUserId && providerId) {
-      logger.info(`[${requestId}] Fetching token by credentialAccountUserId + providerId`, {
+      logger.info('Fetching token by credentialAccountUserId + providerId', {
         credentialAccountUserId,
         providerId,
       })
 
       const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
       if (!auth.success || auth.authType !== AuthType.SESSION || !auth.userId) {
-        logger.warn(`[${requestId}] Unauthorized request for credentialAccountUserId path`, {
+        logger.warn('Unauthorized request for credentialAccountUserId path', {
           success: auth.success,
           authType: auth.authType,
         })
@@ -80,7 +80,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
       if (auth.userId !== credentialAccountUserId) {
         logger.warn(
-          `[${requestId}] User ${auth.userId} attempted to access credentials for ${credentialAccountUserId}`
+          `User ${auth.userId} attempted to access credentials for ${credentialAccountUserId}`
         )
         return NextResponse.json({ error: 'Unauthorized' }, { status: 403 })
       }
@@ -117,7 +117,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         return NextResponse.json({ accessToken }, { status: 200 })
       } catch (error) {
         const message = getErrorMessage(error, 'Failed to get OAuth token')
-        logger.warn(`[${requestId}] OAuth token error: ${message}`)
+        logger.warn(`OAuth token error: ${message}`)
         return NextResponse.json({ error: message }, { status: 403 })
       }
     }
@@ -148,7 +148,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json(result.token, { status: 200 })
   } catch (error) {
-    logger.error(`[${requestId}] Error getting access token`, error)
+    logger.error('Error getting access token', error)
     return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
   }
 })
@@ -166,7 +166,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       {},
       {
         validationErrorResponse: (error) => {
-          logger.warn(`[${requestId}] Invalid query parameters`, { errors: error.issues })
+          logger.warn('Invalid query parameters', { errors: error.issues })
           return NextResponse.json(
             { error: getValidationErrorMessage(error, 'Validation failed') },
             { status: 400 }
@@ -203,7 +203,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
     }
 
     if (!credential.accessToken) {
-      logger.warn(`[${requestId}] No access token available for credential`)
+      logger.warn('No access token available for credential')
       return NextResponse.json({ error: 'No access token available' }, { status: 400 })
     }
 
@@ -225,7 +225,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json(result.token, { status: 200 })
   } catch (error) {
-    logger.error(`[${requestId}] Error fetching access token`, error)
+    logger.error('Error fetching access token', error)
     return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
   }
 })

apps/sim/app/api/billing/update-cost/route.ts

@@ -196,12 +196,12 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
   const startTime = Date.now()
 
   try {
-    logger.debug(`[${requestId}] Update cost request started`)
+    logger.debug('Update cost request started')
 
     // Check authentication (internal API key)
     const authResult = checkInternalApiKey(req)
     if (!authResult.success) {
-      logger.warn(`[${requestId}] Authentication failed: ${authResult.error}`)
+      logger.warn(`Authentication failed: ${authResult.error}`)
       span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.AuthFailed)
       span.setAttribute(TraceAttr.HttpStatusCode, 401)
       return NextResponse.json(
@@ -234,7 +234,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
       {},
       {
         validationErrorResponse: (error) => {
-          logger.warn(`[${requestId}] Invalid request body`, {
+          logger.warn('Invalid request body', {
             errors: error.issues,
           })
           span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.InvalidBody)
@@ -324,7 +324,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
       ...(idempotencyKey ? { [TraceAttr.BillingIdempotencyKey]: idempotencyKey } : {}),
     })
 
-    logger.debug(`[${requestId}] Processing cost update`, {
+    logger.debug('Processing cost update', {
       userId,
       cost,
       model,
@@ -408,7 +408,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
           }
         : undefined
 
-    logger.debug(`[${requestId}] Billing payer resolved`, {
+    logger.debug('Billing payer resolved', {
       userId,
       billingProtocol: protocol,
       billingResolution: getBillingResolution(isMarkerlessLegacy, billingAttribution),
@@ -441,7 +441,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
       ...(rolloverSubscriptionId ? { payerSubscriptionId: rolloverSubscriptionId } : {}),
     })
     const billed = result.billed
-    logger.info(`[${requestId}] Cumulative cost top-up`, {
+    logger.info('Cumulative cost top-up', {
       userId,
       billingPayer: billingContext?.billingEntity ?? { type: 'user', id: userId },
       workspaceId: resolvedWorkspaceId,
@@ -476,7 +476,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
         ) {
           throw error
         }
-        logger.info(`[${requestId}] Charge landed in a period that has since rolled over`, {
+        logger.info('Charge landed in a period that has since rolled over', {
           billingPeriodEnd: result.billingPeriod.end.toISOString(),
         })
       }
@@ -494,7 +494,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
     // Same-or-lower cumulative than already recorded: nothing new to bill.
     // Reconciliation has completed, so preserve Go's established 409 outcome.
     if (!billed) {
-      logger.info(`[${requestId}] Duplicate/non-increasing cumulative cost; no new charge`, {
+      logger.info('Duplicate/non-increasing cumulative cost; no new charge', {
         idempotencyKey,
         userId,
         cost,
@@ -514,7 +514,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
       )
     }
 
-    logger.info(`[${requestId}] Cost update completed successfully`, {
+    logger.info('Cost update completed successfully', {
       userId,
       duration,
       cost,
@@ -539,7 +539,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
 
     const isMarkerlessLegacy = !req.headers.get(COPILOT_BILLING_PROTOCOL_HEADER)
     if (error instanceof CumulativeUsageContextMismatchError && !isMarkerlessLegacy) {
-      logger.error(`[${requestId}] Billing context mismatch`, {
+      logger.error('Billing context mismatch', {
         eventKey: error.eventKey,
         mismatchedFields: error.mismatchedFields,
         duration,
@@ -563,7 +563,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
      * the caller resends later, and the monotonic top-up applies the higher cumulative once.
      */
     if (error instanceof CumulativeUsageInFlightError) {
-      logger.info(`[${requestId}] Cost update already in flight for this request`, {
+      logger.info('Cost update already in flight for this request', {
         eventKey: error.eventKey,
         duration,
       })
@@ -596,7 +596,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
 
     /** Old markerless clients treat every 409 as a successful duplicate. */
     if (reconciliationOutcome && !isMarkerlessLegacy) {
-      logger.warn(`[${requestId}] Billing callback requires reconciliation`, {
+      logger.warn('Billing callback requires reconciliation', {
         code: reconciliationOutcome.code,
         duration,
         billingProtocol:
@@ -618,7 +618,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
     }
 
     if (error instanceof ThresholdSettlementError) {
-      logger.error(`[${requestId}] Retryable threshold settlement failure`, {
+      logger.error('Retryable threshold settlement failure', {
         settlementErrorCode: error.code,
         retryable: error.retryable,
         duration,
@@ -646,7 +646,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
     // lock timeout) — Drizzle's "Failed query" wrapper alone cannot
     // distinguish them, which made the dead-workspace incident undiagnosable
     // from logs.
-    logger.error(`[${requestId}] Cost update failed`, {
+    logger.error('Cost update failed', {
       error: toError(error).message,
       ...(pgCode && { pgCode }),
       ...(pgConstraint && { pgConstraint }),