fix: standardize X-Request-ID propagation across middlewares and error responses

backend/src/app.ts

@@ -14,6 +14,7 @@ import { sandboxMiddleware } from "./middleware/sandbox.middleware.js";
 import { globalRateLimiter, healthRateLimiter } from "./middleware/rate-limiter.middleware.js";
 import { metricsMiddleware } from "./middleware/metrics.middleware.js";
 import { requestIdMiddleware } from "./middleware/requestId.js";
+import { getRequestId } from "./lib/request-context.js";
 import v1Routes from "./routes/v1/index.js";
 import healthRoutes from "./routes/health.routes.js";
 import metricsRoutes from "./routes/metrics.routes.js";
@@ -31,12 +32,14 @@ if (!process.env.CORS_ALLOWED_ORIGINS && !isProduction) {
   allowedOrigins.push("http://localhost:3000");
 }
 
-// Apply global rate limiter first
-app.use(globalRateLimiter);
-
-// Request ID tracing
+// Request ID tracing must be the very first middleware so that every response
+// carries X-Request-ID — including responses short-circuited by later
+// middleware such as the rate limiter's 429 or the CORS 403 (Issue #1494).
 app.use(requestIdMiddleware);
 
+// Apply global rate limiter
+app.use(globalRateLimiter);
+
 // Request counting/latency for the Prometheus registry
 app.use(metricsMiddleware);
 
@@ -90,7 +93,14 @@ app.use(
 // Convert CORS errors into 403 responses so callers get a clear status code
 app.use((err: unknown, req: Request, res: Response, next: NextFunction) => {
   if (err instanceof Error && err.message === "CORS origin not allowed") {
-    res.status(403).json({ error: "CORS origin not allowed" });
+    const requestId = getRequestId();
+    res
+      .status(403)
+      .json(
+        requestId
+          ? { error: "CORS origin not allowed", requestId }
+          : { error: "CORS origin not allowed" },
+      );
     return;
   }
   next(err);

backend/src/lib/request-context.ts

@@ -0,0 +1,20 @@
+/**
+ * Request-scoped context shared by the logging and error layers (Issue #1494).
+ *
+ * The request id lives in a dedicated AsyncLocalStorage so the winston logger
+ * and `sendApiError` can both read it without importing each other, and so
+ * unit tests that mock the logger still propagate a request id into error
+ * payloads.
+ */
+import { AsyncLocalStorage } from 'node:async_hooks';
+
+export interface RequestContext {
+  requestId: string;
+}
+
+export const requestContext = new AsyncLocalStorage<RequestContext>();
+
+/** The request id bound to the current async execution, if any. */
+export function getRequestId(): string | undefined {
+  return requestContext.getStore()?.requestId;
+}

backend/src/logger.ts

@@ -1,7 +1,9 @@
-import { AsyncLocalStorage } from 'async_hooks';
 import { createLogger, format, transports } from 'winston';
+import { requestContext } from './lib/request-context.js';
 
-export const requestContext = new AsyncLocalStorage<{ requestId: string }>();
+// Re-exported so callers that already read the request context from the logger
+// module (worker correlation ids, SSE controller) keep working unchanged.
+export { requestContext };
 
 const logger = createLogger({
   level: process.env.LOG_LEVEL || 'info',

backend/src/middleware/rate-limiter.middleware.ts

@@ -1,4 +1,6 @@
 import { rateLimit, type Options } from 'express-rate-limit';
+import type { NextFunction, Request, Response } from 'express';
+import { getRequestId } from '../lib/request-context.js';
 
 /**
  * Shared factory to create an express-rate-limit instance with common configuration.
@@ -14,11 +16,32 @@ export function createRateLimiter(options: Partial<Options>) {
   });
 }
 
+/**
+ * Default 429 handler: keeps the configured JSON message and stamps the
+ * request id (Issue #1494), so throttled responses stay traceable in the logs
+ * even though they never reach a route handler or the global error handler.
+ */
+export function rateLimitHandler(
+  _req: Request,
+  res: Response,
+  _next: NextFunction,
+  options: Options,
+): void {
+  const message = options.message;
+  const body: Record<string, unknown> =
+    typeof message === 'object' && message !== null
+      ? { ...(message as Record<string, unknown>) }
+      : { message };
+  const requestId = getRequestId();
+  if (requestId) body.requestId = requestId;
+  res.status(options.statusCode).json(body);
+}
+
 export const globalRateLimiter = createRateLimiter({
   windowMs: 1 * 60 * 1000, // 1 minute
   max: 100, // Limit each IP to 100 requests per `window` (here, per minute)
-  standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
-  legacyHeaders: false, // Disable the `X-RateLimit-*` headers
+  standardHeaders: true,
+  legacyHeaders: false,
   // The Prometheus scrape endpoint must never be throttled: a 429 would make
   // Prometheus mark the target down and blind the whole alerting pipeline.
   // It is protected by its own network/token guard instead.
@@ -27,6 +50,7 @@ export const globalRateLimiter = createRateLimiter({
     message: 'Too many requests, please try again later.',
     status: 429,
   },
+  handler: rateLimitHandler,
 });
 /**
  * Dedicated limiter for `/health` (issue #1511).

backend/src/middleware/requestId.ts

@@ -1,6 +1,7 @@
 import { randomUUID } from 'crypto';
 import type { Request, Response, NextFunction } from 'express';
-import logger, { requestContext } from '../logger.js';
+import logger from '../logger.js';
+import { requestContext } from '../lib/request-context.js';
 
 const MAX_REQUEST_ID_LENGTH = 128;
 
@@ -18,6 +19,9 @@ export function requestIdMiddleware(req: Request, res: Response, next: NextFunct
   const header = req.headers['x-request-id'];
   const requestId = typeof header === 'string' && isValidRequestId(header) ? header : randomUUID();
 
+  // Attach to the request as well as the response so downstream handlers,
+  // error formatters and tests can read the same id without re-parsing headers.
+  (req as Request & { id?: string }).id = requestId;
   res.setHeader('X-Request-ID', requestId);
 
   const startMs = Date.now();

backend/src/types/api-error.ts

@@ -1,9 +1,15 @@
 import type { Response } from "express";
+import { getRequestId } from "../lib/request-context.js";
 
 export interface ApiErrorBody {
   code: string;
   message: string;
   details?: unknown;
+  /**
+   * Correlation id for the failing request (Issue #1494). Clients can quote it
+   * to support engineers, who can then find the matching winston log lines.
+   */
+  requestId?: string;
 }
 
 export class ApiError extends Error {
@@ -29,5 +35,7 @@ export function sendApiError(
 ) {
   const error: ApiErrorBody = { code, message };
   if (details !== undefined) error.details = details;
+  const requestId = getRequestId();
+  if (requestId) error.requestId = requestId;
   return res.status(statusCode).json({ error });
-}
+}

backend/tests/error.middleware.test.ts

@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { errorHandler } from '../src/middleware/error.middleware.js';
 import { ZodError } from 'zod';
 import { Prisma } from '../src/generated/prisma/index.js';
+import { requestContext } from '../src/lib/request-context.js';
 import type { Request, Response, NextFunction } from 'express';
 
 describe('Error Middleware', () => {
@@ -59,4 +60,44 @@ describe('Error Middleware', () => {
     expect(res.status).not.toHaveBeenCalled();
     expect(res.json).not.toHaveBeenCalled();
   });
+
+  it('includes the bound requestId in a 500 error payload (Issue #1494)', () => {
+    const error = new Error('Generic error');
+
+    requestContext.run({ requestId: 'req-error-1' }, () => {
+      errorHandler(error, req as Request, res as Response, next);
+    });
+
+    expect(res.status).toHaveBeenCalledWith(500);
+    expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+      error: expect.objectContaining({
+        code: 'INTERNAL_SERVER_ERROR',
+        requestId: 'req-error-1',
+      }),
+    }));
+  });
+
+  it('includes the bound requestId in a validation error payload (Issue #1494)', () => {
+    const error = new ZodError([{ path: ['field'], message: 'invalid', code: 'custom' }]);
+
+    requestContext.run({ requestId: 'req-error-2' }, () => {
+      errorHandler(error, req as Request, res as Response, next);
+    });
+
+    expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+      error: expect.objectContaining({
+        code: 'VALIDATION_ERROR',
+        requestId: 'req-error-2',
+      }),
+    }));
+  });
+
+  it('omits requestId when no request context is bound', () => {
+    const error = new Error('Generic error');
+
+    errorHandler(error, req as Request, res as Response, next);
+
+    const body = (res.json as any).mock.calls[0][0];
+    expect(body.error.requestId).toBeUndefined();
+  });
 });

backend/tests/requestId.test.ts

@@ -1,5 +1,9 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
+import request from 'supertest';
 import { requestIdMiddleware } from '../src/middleware/requestId.js';
+import { rateLimitHandler } from '../src/middleware/rate-limiter.middleware.js';
+import { requestContext } from '../src/lib/request-context.js';
+import app from '../src/app.js';
 import type { Request, Response, NextFunction } from 'express';
 
 describe('RequestId Middleware', () => {
@@ -57,4 +61,65 @@ describe('RequestId Middleware', () => {
     const call = (res.setHeader as any).mock.calls[0];
     expect(call[1]).toMatch(/^[a-f0-9-]+$/i);
   });
+
+  it('exposes the same id on the request object and the response header (Issue #1494)', () => {
+    requestIdMiddleware(req as Request, res as Response, next);
+    const headerValue = (res.setHeader as any).mock.calls[0][1];
+    expect((req as Request & { id?: string }).id).toBe(headerValue);
+  });
+});
+
+describe('X-Request-ID propagation (Issue #1494)', () => {
+  it('sets the header on a normal response', async () => {
+    const response = await request(app).get('/');
+    expect(response.status).toBe(200);
+    expect(response.headers['x-request-id']).toBeTruthy();
+  });
+
+  it('echoes a valid client-supplied request id', async () => {
+    const response = await request(app).get('/').set('X-Request-ID', 'client-trace-42');
+    expect(response.headers['x-request-id']).toBe('client-trace-42');
+  });
+
+  it('includes requestId in an error payload produced by sendApiError', async () => {
+    const response = await request(app)
+      .get('/v1/streams/not-a-number')
+      .set('X-Request-ID', 'err-trace-7');
+
+    expect(response.status).toBe(400);
+    expect(response.headers['x-request-id']).toBe('err-trace-7');
+    expect(response.body.error.code).toBe('INVALID_STREAM_ID');
+    expect(response.body.error.requestId).toBe('err-trace-7');
+  });
+
+  it('sets the header on a response short-circuited before the routers (CORS 403)', async () => {
+    const response = await request(app)
+      .get('/')
+      .set('Origin', 'https://evil.example')
+      .set('X-Request-ID', 'cors-trace-9');
+
+    expect(response.status).toBe(403);
+    expect(response.headers['x-request-id']).toBe('cors-trace-9');
+    expect(response.body.requestId).toBe('cors-trace-9');
+  });
+});
+
+describe('Rate limit 429 request id', () => {
+  it('stamps requestId from the request context onto the throttle body', () => {
+    const res = {
+      status: vi.fn().mockReturnThis(),
+      json: vi.fn().mockReturnThis(),
+    };
+    const options = {
+      statusCode: 429,
+      message: { message: 'Too many requests' },
+    } as any;
+
+    requestContext.run({ requestId: 'rl-trace-1' }, () => {
+      rateLimitHandler({} as Request, res as unknown as Response, vi.fn(), options);
+    });
+
+    expect(res.status).toHaveBeenCalledWith(429);
+    expect(res.json).toHaveBeenCalledWith({ message: 'Too many requests', requestId: 'rl-trace-1' });
+  });
 });