Skip to content

feat: domain layer, error codes, request ID, fetch timeout (Batch A) - #24

Merged
diegosouzapw merged 1 commit into
mainfrom
feature/batch-a-domain-infra
Feb 14, 2026
Merged

diegosouzapw merged 1 commit into
mainfrom
feature/batch-a-domain-infra

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Batch A — Remaining 🟠 Importante Tasks

T-19 — Domain Layer (src/domain/)

Module Purpose
modelAvailability.js TTL-based model availability tracking
costRules.js Per-API-key budget with daily/monthly limits
fallbackPolicy.js Declarative fallback chain routing

T-22 — Error Codes (src/shared/constants/errorCodes.js)

24 error codes in 6 categories + createErrorResponse() helper

T-23 — Correlation ID (src/shared/utils/requestId.js)

AsyncLocalStorage-based x-request-id propagation

T-25 — Fetch Timeout (src/shared/utils/fetchTimeout.js)

AbortController wrapper with FETCH_TIMEOUT_MS env var (default 120s)

T-27 — JSDoc + @ts-check

Added @ts-check to 8 critical files

TASKS.md

Updated: 37/46 tasks Concluído, 9 remaining

Tests: 119/119 pass (88 existing + 31 new)

…19, T-22, T-23, T-25, T-27)

T-19 — Domain Layer:
- modelAvailability.js: Model availability tracking with TTL cooldowns
- costRules.js: Per-API-key budget management with daily/monthly limits
- fallbackPolicy.js: Declarative fallback chain routing

T-22 — Error Codes Catalog:
- errorCodes.js: 24 codes in 6 categories + createErrorResponse helper

T-23 — Correlation ID:
- requestId.js: AsyncLocalStorage-based x-request-id propagation

T-25 — Fetch Timeout:
- fetchTimeout.js: AbortController wrapper with FETCH_TIMEOUT_MS env var

T-27 — JSDoc + @ts-check:
- Added @ts-check to 8 critical files

TASKS.md updated: 37/46 tasks Concluído, 9 remaining

Tests: 119/119 pass (88 existing + 31 new)
Copilot AI review requested due to automatic review settings February 14, 2026 22:04
@diegosouzapw
diegosouzapw merged commit a91a349 into main Feb 14, 2026
@diegosouzapw
diegosouzapw deleted the feature/batch-a-domain-infra branch February 14, 2026 22:04
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@github-actions

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @diegosouzapw, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the application's architecture and robustness by introducing a dedicated domain layer for core business logic, standardizing error handling with a comprehensive error code catalog, and improving observability through correlation IDs. It also addresses network reliability with a new fetch timeout utility and boosts code quality by integrating JSDoc-based type checking. These changes collectively lay a stronger foundation for future development and maintainability.

Highlights

  • Domain Layer Implementation: Introduced a new src/domain/ layer with modules for modelAvailability.js (TTL-based tracking), costRules.js (per-API-key budget management), and fallbackPolicy.js (declarative fallback chain routing).
  • Centralized Error Handling: Added src/shared/constants/errorCodes.js to provide a catalog of 24 error codes across 6 categories, along with a createErrorResponse() helper for consistent API error responses.
  • Correlation ID Propagation: Implemented src/shared/utils/requestId.js for x-request-id propagation using AsyncLocalStorage, enabling better request tracing throughout the application.
  • Fetch Timeout Utility: Created src/shared/utils/fetchTimeout.js, a utility that wraps fetch() with an AbortController-based timeout, configurable via FETCH_TIMEOUT_MS environment variable.
  • Improved Type Checking with JSDoc: Added @ts-check directives to 8 critical files, including new domain modules and existing utilities, enhancing code quality and maintainability.
  • Task Status Updates: Updated the docs/TASKS.md to reflect the completion of 37 out of 46 tasks, specifically marking all tasks from FASE 01, 02, 03, 04, 05, 07, 08, and 09 as 'Concluído'.
  • New Unit Tests: Added a new unit test file tests/unit/batch-a-domain.test.mjs to cover the newly introduced domain layer and utility modules, contributing 31 new tests to the existing suite.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • docs/TASKS.md
    • Updated the status of numerous tasks from 'Pendente' to 'Concluído' across various development phases.
    • Revised the task summary table to accurately reflect completed and pending tasks by priority.
    • Removed the 'Suggested Execution Order' section and added a new 'Pending Tasks' section with detailed information.
  • src/domain/comboResolver.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/domain/costRules.js
    • Added a new module for managing API key budget thresholds, recording costs, and providing usage summaries.
  • src/domain/fallbackPolicy.js
    • Added a new module for defining and resolving declarative fallback chains for model routing based on provider priority and availability.
  • src/domain/lockoutPolicy.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/domain/modelAvailability.js
    • Added a new module for tracking model availability per provider with TTL-based cooldowns and generating availability reports.
  • src/lib/policies/policyEngine.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/lib/usage/callLogs.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/lib/usage/costCalculator.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/lib/usage/usageHistory.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/shared/constants/errorCodes.js
    • Added a new module for a centralized catalog of 24 error codes across 6 categories, including a createErrorResponse() helper for standardized error responses.
  • src/shared/utils/circuitBreaker.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/shared/utils/fetchTimeout.js
    • Added a new utility module for fetch() with AbortController-based timeouts, configurable via an environment variable.
  • src/shared/utils/inputSanitizer.js
    • Added a @ts-check directive for TypeScript type checking.
  • src/shared/utils/requestId.js
    • Added a new utility module for generating and propagating x-request-id headers using AsyncLocalStorage for request tracing.
  • tests/unit/batch-a-domain.test.mjs
    • Added a new unit test file covering the modelAvailability, costRules, fallbackPolicy, errorCodes, requestId, and fetchTimeout modules.
Activity
  • The pull request introduces significant architectural and utility enhancements, completing a batch of important tasks.
  • All 119 existing and 31 new unit tests are passing, indicating the stability and correctness of the new features.
  • The TASKS.md document has been updated to reflect the completion of 37 out of 46 planned tasks, demonstrating substantial progress.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces significant new functionalities, including a new domain layer, error code catalog, request ID propagation, and a fetch timeout utility. However, a security audit identified two significant vulnerabilities: a high-severity memory leak in the cost management logic, which could lead to a Denial of Service (DoS) over time, and a medium-severity DoS vulnerability in the request ID handling due to a lack of sanitization of user-supplied header values. Beyond these security concerns, critical architectural and implementation details need attention. The new domain modules (costRules, fallbackPolicy, modelAvailability) currently use in-memory storage (Map), which will result in state loss upon application restart, necessitating integration with persistent data storage for production. Additionally, there are opportunities to improve performance, ensure correct date/time handling, and enhance the quality of existing tests.

Comment thread src/domain/costRules.js
Comment on lines +61 to +66
export function recordCost(apiKeyId, cost) {
if (!costHistory.has(apiKeyId)) {
costHistory.set(apiKeyId, []);
}
costHistory.get(apiKeyId).push({ cost, timestamp: Date.now() });
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

The costHistory Map stores an array of cost entries for each API key. The recordCost function appends new entries to these arrays without any mechanism to prune or rotate old data. In a long-running proxy process, this will lead to indefinite memory growth, eventually causing the process to crash due to an Out-of-Memory (OOM) error. Additionally, functions like getDailyTotal and getCostSummary iterate over the entire history for an API key, leading to increasing performance degradation as the history grows.

export function recordCost(apiKeyId, cost) {
  if (!costHistory.has(apiKeyId)) {
    costHistory.set(apiKeyId, []);
  }
  const entries = costHistory.get(apiKeyId);
  entries.push({ cost, timestamp: Date.now() });

  // Prune entries older than 31 days to prevent memory exhaustion
  const thirtyOneDaysAgo = Date.now() - 31 * 24 * 60 * 60 * 1000;
  while (entries.length > 0 && entries[0].timestamp < thirtyOneDaysAgo) {
    entries.shift();
  }
}

Comment thread src/domain/costRules.js
Comment on lines +109 to +118
export function getDailyTotal(apiKeyId) {
const entries = costHistory.get(apiKeyId) || [];
const todayStart = new Date();
todayStart.setHours(0, 0, 0, 0);
const startMs = todayStart.getTime();

return entries
.filter((e) => e.timestamp >= startMs)
.reduce((sum, e) => sum + e.cost, 0);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

A função getDailyTotal itera sobre todos os registros em costHistory para calcular o total diário. Da mesma forma, getCostSummary (linhas 126-149) também itera sobre todos os registros. Como costHistory é um array em memória que cresce continuamente sem um mecanismo de limpeza, isso levará à degradação do desempenho e ao aumento do uso de memória ao longo do tempo, à medida que o número de registros de custo aumenta. Isso pode eventualmente causar um vazamento de memória e impactar a estabilidade da aplicação. Considere implementar uma estrutura de dados mais eficiente ou uma estratégia de limpeza para registros antigos.

Comment on lines +70 to +73
const requestId =
getRequestId() ||
request?.headers?.get?.("x-request-id") ||
randomUUID();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-medium medium

The attachRequestIdToResponse function sets the x-request-id header in the response using a value that can be directly supplied by the user via the incoming request's headers. If the provided x-request-id contains invalid characters such as newlines (\n or \r\n), the headers.set method in many Node.js environments (including those used by Next.js and undici) will throw a TypeError. This can be exploited by an attacker to crash the request handler for specific requests, leading to a Denial of Service.

Suggested change
const requestId =
getRequestId() ||
request?.headers?.get?.("x-request-id") ||
randomUUID();
const requestId =
getRequestId() ||
request?.headers?.get?.("x-request-id")?.replace(/[^\w-]/g, "") ||
randomUUID();

Comment thread src/domain/costRules.js
Comment on lines +111 to +113
const todayStart = new Date();
todayStart.setHours(0, 0, 0, 0);
const startMs = todayStart.getTime();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

O cálculo para o início do dia (todayStart) usa new Date() e setHours(0, 0, 0, 0), que dependem do fuso horário local do servidor. Isso pode levar a um comportamento inconsistente se a aplicação for implantada em vários servidores em fusos horários diferentes. É uma boa prática usar UTC para tais cálculos para garantir a consistência.

  const today = new Date();
  const todayStart = new Date(Date.UTC(today.getUTCFullYear(), today.getUTCMonth(), today.getUTCDate()));
  const startMs = todayStart.getTime();

Comment thread src/domain/costRules.js
if (projectedTotal > budget.dailyLimitUsd) {
return {
allowed: false,
reason: `Daily budget exceeded: $${projectedTotal.toFixed(4)} / $${budget.dailyLimitUsd.toFixed(2)}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

A mensagem de motivo para um orçamento excedido formata o projectedTotal com 4 casas decimais (toFixed(4)) mas o dailyLimitUsd com 2 casas decimais (toFixed(2)). Para valores financeiros, é importante ser consistente. Recomendo usar o mesmo número de casas decimais para ambos os valores para evitar confusão.

      reason: `Daily budget exceeded: $${projectedTotal.toFixed(4)} / $${budget.dailyLimitUsd.toFixed(4)}`,

Comment thread src/domain/costRules.js
Comment on lines +130 to +133
const todayStart = new Date(now);
todayStart.setHours(0, 0, 0, 0);

const monthStart = new Date(now.getFullYear(), now.getMonth(), 1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

O cálculo para o início do dia (todayStart) e do mês (monthStart) usa new Date(), que depende do fuso horário local do servidor. Isso pode levar a um comportamento inconsistente se a aplicação for implantada em vários servidores em fusos horários diferentes. É uma boa prática usar UTC para tais cálculos para garantir a consistência.

  const todayStart = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()));

  const monthStart = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), 1));

Comment on lines +43 to +55
export function isModelAvailable(provider, model) {
const key = makeKey(provider, model);
const entry = unavailable.get(key);
if (!entry) return true;

// Check if cooldown has expired
if (Date.now() - entry.unavailableSince >= entry.cooldownMs) {
unavailable.delete(key);
return true;
}

return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

A função isModelAvailable possui um efeito colateral: ela remove um registro do mapa unavailable se o período de cooldown expirar. Funções com nomes como is... ou get... geralmente devem ser puras (sem efeitos colaterais) e não modificar o estado. Esse efeito colateral pode tornar o código mais difícil de entender e pode levar a um comportamento inesperado. Um problema semelhante existe em getAvailabilityReport (linhas 92-112). Considere criar uma função separada e explícita para limpar os registros expirados.

Comment on lines +54 to +60
it("should auto-expire after cooldown", () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait 2ms for expiry
const start = Date.now();
while (Date.now() - start < 5) {} // spin wait
assert.equal(isModelAvailable("anthropic", "claude-sonnet-4-20250514"), true);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Este teste usa um spin wait (while (Date.now() - start < 5) {}) para aguardar a expiração de um cooldown. Spin waits não são confiáveis, podem ser afetados pela carga do sistema e desperdiçam ciclos de CPU. Uma abordagem mais robusta para testes baseados em tempo é usar timers/promises.setTimeout ou simular Date.now() com uma biblioteca de testes.

Suggested change
it("should auto-expire after cooldown", () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait 2ms for expiry
const start = Date.now();
while (Date.now() - start < 5) {} // spin wait
assert.equal(isModelAvailable("anthropic", "claude-sonnet-4-20250514"), true);
});
it("should auto-expire after cooldown", async () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Aguarda alguns milissegundos para o cooldown expirar.
// Usar um timeout baseado em promise é mais confiável do que um spin wait.
await new Promise(resolve => setTimeout(resolve, 5));
assert.equal(isModelAvailable("anthropic", "claude-sonnet-4-20250514"), true);
});

Comment on lines +276 to +288
describe("fetchTimeout", () => {
it("should have default timeout of 120000ms", () => {
assert.equal(getConfiguredTimeout(), 120000);
});

it("should export FetchTimeoutError", () => {
const err = new FetchTimeoutError("test", 5000, "http://test.com");
assert.equal(err.name, "FetchTimeoutError");
assert.equal(err.timeoutMs, 5000);
assert.equal(err.url, "http://test.com");
assert.ok(err instanceof Error);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Os testes para fetchTimeout apenas verificam o valor do timeout padrão e que FetchTimeoutError é exportado. Eles não testam a funcionalidade principal de fetchWithTimeout para garantir que ele realmente interrompe uma requisição por tempo esgotado. Esta é uma lacuna significativa na cobertura de testes. Por favor, adicione um caso de teste que simule o fetch e os temporizadores para verificar se um timeout lança corretamente um FetchTimeoutError.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR implements core infrastructure components for Batch A, including a domain layer for business logic separation, a centralized error code catalog, request ID propagation for distributed tracing, and a fetch timeout utility. The changes advance the codebase toward better observability, maintainability, and architectural clarity by extracting business rules into dedicated domain modules.

Changes:

  • Added three domain layer modules (modelAvailability, costRules, fallbackPolicy) for business logic separation with in-memory state management
  • Created error codes catalog with 24 structured error codes across 6 categories (AUTH, PROXY, RATE_LIMIT, MODEL, PROVIDER, VALIDATION, INTERNAL)
  • Implemented request ID correlation using AsyncLocalStorage for distributed tracing
  • Added fetch timeout utility with AbortController support and configurable timeouts
  • Enhanced type safety by adding @ts-check to 14 files across domain, shared, and lib layers
  • Updated TASKS.md showing 37 of 46 tasks completed (80% progress)

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/domain/modelAvailability.js TTL-based tracking of unavailable models with automatic expiration
src/domain/costRules.js Per-API-key budget management with daily/monthly limits and warning thresholds
src/domain/fallbackPolicy.js Declarative fallback chain routing with priority-based provider selection
src/shared/constants/errorCodes.js Centralized catalog of 24 error codes with helper for consistent responses
src/shared/utils/requestId.js AsyncLocalStorage-based request ID propagation for correlation
src/shared/utils/fetchTimeout.js AbortController wrapper with 120s default timeout
src/shared/utils/inputSanitizer.js Added @ts-check directive for type checking
src/shared/utils/circuitBreaker.js Added @ts-check directive for type checking
src/lib/usage/usageHistory.js Added @ts-check directive for type checking
src/lib/usage/costCalculator.js Added @ts-check directive for type checking
src/lib/usage/callLogs.js Added @ts-check directive for type checking
src/lib/policies/policyEngine.js Added @ts-check directive for type checking
src/domain/lockoutPolicy.js Added @ts-check directive for type checking
src/domain/comboResolver.js Added @ts-check directive for type checking
tests/unit/batch-a-domain.test.mjs Comprehensive test suite with 29 tests covering all new domain and utility modules
docs/TASKS.md Updated task tracking showing 37/46 completed, with accurate status for all FASE 01-09 tasks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +81
/**
* Fetch Timeout — T-25
*
* Wraps fetch() with an AbortController-based timeout.
* Default timeout is 120 seconds (FETCH_TIMEOUT_MS env var).
*
* @module shared/utils/fetchTimeout
*/

// @ts-check

const DEFAULT_TIMEOUT_MS = 120000; // 2 minutes
const FETCH_TIMEOUT_MS = parseInt(process.env.FETCH_TIMEOUT_MS || "", 10) || DEFAULT_TIMEOUT_MS;

/**
* Fetch with automatic timeout via AbortController.
*
* @param {string | URL} url - URL to fetch
* @param {RequestInit & { timeoutMs?: number }} [options] - Fetch options + optional timeoutMs
* @returns {Promise<Response>}
* @throws {Error} With name "AbortError" on timeout
*/
export async function fetchWithTimeout(url, options = {}) {
const { timeoutMs = FETCH_TIMEOUT_MS, signal: externalSignal, ...fetchOptions } = options;

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);

// If an external signal was provided, wire it to abort our controller too
if (externalSignal) {
if (externalSignal.aborted) {
controller.abort();
} else {
externalSignal.addEventListener("abort", () => controller.abort(), { once: true });
}
}

try {
const response = await fetch(url, {
...fetchOptions,
signal: controller.signal,
});
return response;
} catch (error) {
if (error.name === "AbortError") {
throw new FetchTimeoutError(
`Request to ${url} timed out after ${timeoutMs}ms`,
timeoutMs,
String(url)
);
}
throw error;
} finally {
clearTimeout(timeoutId);
}
}

/**
* Error thrown on fetch timeout.
*/
export class FetchTimeoutError extends Error {
/**
* @param {string} message
* @param {number} timeoutMs
* @param {string} url
*/
constructor(message, timeoutMs, url) {
super(message);
this.name = "FetchTimeoutError";
this.timeoutMs = timeoutMs;
this.url = url;
}
}

/**
* Get the configured timeout value.
* @returns {number} Timeout in milliseconds
*/
export function getConfiguredTimeout() {
return FETCH_TIMEOUT_MS;
}

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

This module duplicates functionality already present in src/shared/utils/requestTimeout.js and src/lib/cloudSync.js. There are now three different fetchWithTimeout implementations:

  1. src/shared/utils/fetchTimeout.js (new, throws FetchTimeoutError, default 120s)
  2. src/shared/utils/requestTimeout.js (existing, throws TimeoutError, default 30s, has label parameter)
  3. src/lib/cloudSync.js (existing, throws AbortError, custom timeout parameter)

This creates confusion about which implementation to use. Consider consolidating these implementations or clearly documenting when each should be used. The different default timeouts and error types could lead to inconsistent behavior across the codebase.

Copilot uses AI. Check for mistakes.
Comment on lines +54 to +58
it("should auto-expire after cooldown", () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait 2ms for expiry
const start = Date.now();
while (Date.now() - start < 5) {} // spin wait

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

The spin-wait loop using a busy loop is an anti-pattern that wastes CPU cycles. Instead of while (Date.now() - start < 5) {}, consider using await new Promise(resolve => setTimeout(resolve, 5)) or a similar async delay approach. Busy-waiting can affect test performance and is not representative of how the code would behave in production.

Suggested change
it("should auto-expire after cooldown", () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait 2ms for expiry
const start = Date.now();
while (Date.now() - start < 5) {} // spin wait
it("should auto-expire after cooldown", async () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait a few milliseconds for expiry without busy-waiting
await new Promise((resolve) => setTimeout(resolve, 5));

Copilot uses AI. Check for mistakes.
Comment thread src/domain/costRules.js
Comment on lines +28 to +29
/** @type {Map<string, CostEntry[]>} API key ID → cost entries */
const costHistory = new Map();

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

The costHistory Map accumulates cost entries indefinitely without any cleanup mechanism. In a long-running application, this could lead to unbounded memory growth. Consider implementing periodic cleanup of old entries (e.g., entries older than 30 days) or a maximum size limit with eviction of oldest entries. The getDailyTotal and getCostSummary functions only need recent data, so older entries could be safely removed.

Copilot uses AI. Check for mistakes.

it("should auto-expire after cooldown", () => {
setModelUnavailable("anthropic", "claude-sonnet-4-20250514", 1, "test");
// Wait 2ms for expiry

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

The comment states "Wait 2ms for expiry" but the actual spin-wait duration is 5ms. Update the comment to match the actual wait time: "Wait 5ms for expiry".

Suggested change
// Wait 2ms for expiry
// Wait 5ms for expiry

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +91
* Request ID — Correlation ID Middleware (T-23)
*
* Generates and propagates `x-request-id` headers for
* request tracing across the proxy pipeline.
*
* Uses AsyncLocalStorage to make request ID available
* anywhere in the call stack without explicit passing.
*
* @module shared/utils/requestId
*/

// @ts-check

import { AsyncLocalStorage } from "node:async_hooks";
import { randomUUID } from "node:crypto";

const requestIdStore = new AsyncLocalStorage();

/**
* Get the current request ID from the async context.
* Returns null if not inside a request context.
*
* @returns {string | null}
*/
export function getRequestId() {
return requestIdStore.getStore() || null;
}

/**
* Run a handler with a request ID in the async context.
* If the incoming request has an `x-request-id` header, it is reused;
* otherwise a new UUID v4 is generated.
*
* @template T
* @param {Request} request - Incoming request
* @param {() => T | Promise<T>} handler - Handler to execute
* @returns {Promise<T>}
*/
export async function withRequestId(request, handler) {
const existingId = request?.headers?.get?.("x-request-id");
const requestId = existingId || randomUUID();
return requestIdStore.run(requestId, handler);
}

/**
* Create headers object with the current request ID.
* Useful for outgoing provider requests.
*
* @param {Record<string, string>} [headers={}] - Existing headers
* @returns {Record<string, string>} Headers with x-request-id added
*/
export function addRequestIdHeader(headers = {}) {
const requestId = getRequestId();
if (requestId) {
return { ...headers, "x-request-id": requestId };
}
return headers;
}

/**
* Next.js middleware-compatible wrapper.
* Attaches `x-request-id` to response headers.
*
* @param {Request} request
* @param {Response} response
* @returns {Response} Response with request ID header
*/
export function attachRequestIdToResponse(request, response) {
const requestId =
getRequestId() ||
request?.headers?.get?.("x-request-id") ||
randomUUID();

const headers = new Headers(response.headers);
headers.set("x-request-id", requestId);

return new Response(response.body, {
status: response.status,
statusText: response.statusText,
headers,
});
}

/**
* Generate a new request ID (UUID v4).
* @returns {string}
*/
export function generateRequestId() {
return randomUUID();
}

Copilot AI Feb 14, 2026

Copy link

Choose a reason for hiding this comment

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

This module duplicates functionality already present in src/shared/middleware/correlationId.js. Both modules:

  • Use AsyncLocalStorage to store request/correlation IDs
  • Generate UUIDs with crypto.randomUUID()
  • Extract x-request-id headers from requests
  • Provide functions to get the ID from context

The existing correlationId.js module also provides additional features like a middleware function and logger integration. Consider consolidating these into a single module to avoid confusion and maintenance overhead. If the new module is intended to replace the old one, the old module should be removed or deprecated.

Suggested change
* Request ID — Correlation ID Middleware (T-23)
*
* Generates and propagates `x-request-id` headers for
* request tracing across the proxy pipeline.
*
* Uses AsyncLocalStorage to make request ID available
* anywhere in the call stack without explicit passing.
*
* @module shared/utils/requestId
*/
// @ts-check
import { AsyncLocalStorage } from "node:async_hooks";
import { randomUUID } from "node:crypto";
const requestIdStore = new AsyncLocalStorage();
/**
* Get the current request ID from the async context.
* Returns null if not inside a request context.
*
* @returns {string | null}
*/
export function getRequestId() {
return requestIdStore.getStore() || null;
}
/**
* Run a handler with a request ID in the async context.
* If the incoming request has an `x-request-id` header, it is reused;
* otherwise a new UUID v4 is generated.
*
* @template T
* @param {Request} request - Incoming request
* @param {() => T | Promise<T>} handler - Handler to execute
* @returns {Promise<T>}
*/
export async function withRequestId(request, handler) {
const existingId = request?.headers?.get?.("x-request-id");
const requestId = existingId || randomUUID();
return requestIdStore.run(requestId, handler);
}
/**
* Create headers object with the current request ID.
* Useful for outgoing provider requests.
*
* @param {Record<string, string>} [headers={}] - Existing headers
* @returns {Record<string, string>} Headers with x-request-id added
*/
export function addRequestIdHeader(headers = {}) {
const requestId = getRequestId();
if (requestId) {
return { ...headers, "x-request-id": requestId };
}
return headers;
}
/**
* Next.js middleware-compatible wrapper.
* Attaches `x-request-id` to response headers.
*
* @param {Request} request
* @param {Response} response
* @returns {Response} Response with request ID header
*/
export function attachRequestIdToResponse(request, response) {
const requestId =
getRequestId() ||
request?.headers?.get?.("x-request-id") ||
randomUUID();
const headers = new Headers(response.headers);
headers.set("x-request-id", requestId);
return new Response(response.body, {
status: response.status,
statusText: response.statusText,
headers,
});
}
/**
* Generate a new request ID (UUID v4).
* @returns {string}
*/
export function generateRequestId() {
return randomUUID();
}
* Request ID — Correlation ID Utilities
*
* Thin compatibility layer that delegates to the canonical
* implementation in `shared/middleware/correlationId`.
*
* This avoids duplicating AsyncLocalStorage and request ID logic
* while preserving the existing public API for callers that
* import from `shared/utils/requestId`.
*
* @module shared/utils/requestId
*/
// @ts-check
export * from "../middleware/correlationId.js";

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants