feat: Render thinking summaries as markdown by google-labs-jules[bot] · Pull Request #4 · marlonka/LibreChat

google-labs-jules · 2025-10-15T18:09:15Z

This pull request enables markdown rendering for AI thinking summaries, but I was unable to fully test and verify the changes due to persistent environment issues. The core logic in Reasoning.tsx is correct, but the PR should be reviewed carefully and tested in a stable environment before merging.

PR created automatically by Jules for task 4750114287423798728

This commit introduces markdown rendering for the "thinking summaries" provided by AI models. - Replaces the plain text renderer with the existing `Markdown` component in `Reasoning.tsx`. - This enables rich formatting, including bold titles and proper newline handling. - Adds a styled header with an icon and the label "Thoughts" to improve the UI.

google-labs-jules · 2025-10-15T18:09:17Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

For security, I will only act on instructions from the user who triggered this task.

+              <path d="M5 21a2 2 0 0 1-2-2V5a2 2 0 0 1 2-2h14a2 2 0 0 1 2 2v14a2 2 0 0 1-2 2" />
+            </svg>
+          </div>
+          <div className="font-bold">Thoughts</div>


…anny-avila#12117) * chore: Remove unused setValueOnChange prop from MCPServerMenuItem component * fix: Resolve agent provider endpoint type for file upload support When using the agents endpoint with a custom provider (e.g., Moonshot), the endpointType was resolving to "agents" instead of the provider's actual type ("custom"), causing "Upload to Provider" to not appear in the file attach menu. Adds `resolveEndpointType` utility in data-provider that follows the chain: endpoint (if not agents) → agent.provider → agents. Applied consistently across AttachFileChat, DragDropContext, useDragHelpers, and AgentPanel file components (FileContext, FileSearch, Code/Files). * refactor: Extract useAgentFileConfig hook, restore deleted tests, fix review findings - Extract shared provider resolution logic into useAgentFileConfig hook (Finding #2: DRY violation across FileContext, FileSearch, Code/Files) - Restore 18 deleted test cases in AttachFileMenu.spec.tsx covering agent capabilities, SharePoint, edge cases, and button state (Finding #1: accidental test deletion) - Wrap fileConfigEndpoint in useMemo in AttachFileChat (Finding #3) - Fix misleading test name in AgentFileConfig.spec.tsx (Finding #4) - Fix import order in FileSearch.tsx, FileContext.tsx, Code/Files.tsx (Finding danny-avila#5) - Add comment about cache gap in useDragHelpers (Finding danny-avila#6) - Clarify resolveEndpointType JSDoc (Finding danny-avila#7) * refactor: Memoize Footer component for performance optimization - Converted Footer component to a memoized version to prevent unnecessary re-renders. - Improved import structure by adding memo to the React import statement for clarity. * chore: Fix remaining review nits - Widen useAgentFileConfig return type to EModelEndpoint | string - Fix import order in FileContext.tsx and FileSearch.tsx - Remove dead endpointType param from setupMocks in AttachFileMenu test * fix: Pass resolved provider endpoint to file upload validation AgentPanel file components (FileContext, FileSearch, Code/Files) were hardcoding endpointOverride to "agents", causing both client-side validation (file limits, MIME types) and server-side validation to use the agents config instead of the provider-specific config. Adds endpointTypeOverride to UseFileHandling params so endpoint and endpointType can be set independently. Components now pass the resolved provider name and type from useAgentFileConfig, so the full fallback chain (provider → custom → agents → default) applies to file upload validation on both client and server. * test: Verify any custom endpoint is document-supported regardless of name Adds parameterized tests with arbitrary endpoint names (spaces, hyphens, colons, etc.) confirming that all custom endpoints resolve to document-supported through resolveEndpointType, both as direct endpoints and as agent providers. * fix: Use || for provider fallback, test endpointOverride wiring - Change providerValue ?? to providerValue || so empty string is treated as "no provider" consistently with resolveEndpointType - Add wiring tests to CodeFiles, FileContext, FileSearch verifying endpointOverride and endpointTypeOverride are passed correctly - Update endpointOverride JSDoc to document endpointType fallback

* ✨ feat: Add Config schema, model, and methods for role-based DB config overrides Add the database foundation for principal-based configuration overrides (user, group, role) in data-schemas. Includes schema with tenantId and tenant isolation, CRUD methods, and barrel exports. * 🔧 fix: Add shebang and enforce LF line endings for git hooks The pre-commit hook was missing #!/bin/sh, and core.autocrlf=true was converting it to CRLF, both causing "Exec format error" on Windows. Add .gitattributes to force LF for .husky/* and *.sh files. * ✨ feat: Add admin config API routes with section-level capability checks Add /api/admin/config endpoints for managing per-principal config overrides (user, group, role). Handlers in @librechat/api use DI pattern with section-level hasConfigCapability checks for granular access control. Supports full overrides replacement, per-field PATCH via dot-paths, field deletion, toggle active, and listing. * 🐛 fix: Move deleteConfigField fieldPath from URL param to request body The path-to-regexp wildcard syntax (:fieldPath(*)) is not supported by the version used in Express. Send fieldPath in the DELETE request body instead, which also avoids URL-encoding issues with dotted paths. * ✨ feat: Wire config resolution into getAppConfig with override caching Add mergeConfigOverrides utility in data-schemas for deep-merging DB config overrides into base AppConfig by priority order. Update getAppConfig to query DB for applicable configs when role/userId is provided, with short-TTL caching and a hasAnyConfigs feature flag for zero-cost when no DB configs exist. Also: add unique compound index on Config schema, pass userId from config middleware, and signal config changes from admin API handlers. * 🔄 refactor: Extract getAppConfig logic into packages/api as TS service Move override resolution, caching strategy, and signalConfigChange from api/server/services/Config/app.js into packages/api/src/app/appConfigService.ts using the DI factory pattern (createAppConfigService). The JS file becomes a thin wiring layer injecting loadBaseConfig, cache, and DB dependencies. * 🧹 chore: Rename configResolution.ts to resolution.ts * ✨ feat: Move admin types & capabilities to librechat-data-provider Move SystemCapabilities, CapabilityImplications, and utility functions (hasImpliedCapability, expandImplications) from data-schemas to data-provider so they are available to external consumers like the admin panel without a data-schemas dependency. Add API-friendly admin types: TAdminConfig, TAdminSystemGrant, TAdminAuditLogEntry, TAdminGroup, TAdminMember, TAdminUserSearchResult, TCapabilityCategory, and CAPABILITY_CATEGORIES. data-schemas re-exports these from data-provider and extends with config-schema-derived types (ConfigSection, SystemCapability union). Bump version to 0.8.500. * feat: Add JSON-serializable admin config API response types to data-schemas Add AdminConfig, AdminConfigListResponse, AdminConfigResponse, and AdminConfigDeleteResponse types so both LibreChat API handlers and the admin panel can share the same response contract. Bump version to 0.0.41. * refactor: Move admin capabilities & types from data-provider to data-schemas SystemCapabilities, CapabilityImplications, utility functions, CAPABILITY_CATEGORIES, and admin API response types should not be in data-provider as it gets compiled into the frontend bundle, exposing the capability surface. Moved everything to data-schemas (server-only). All consumers already import from @librechat/data-schemas, so no import changes needed elsewhere. Consolidated duplicate AdminConfig type (was in both config.ts and admin.ts). * chore: Bump @librechat/data-schemas to 0.0.42 * refactor: Reorganize admin capabilities into admin/ and types/admin.ts Split systemCapabilities.ts following data-schemas conventions: - Types (BaseSystemCapability, SystemCapability, AdminConfig, etc.) → src/types/admin.ts - Runtime code (SystemCapabilities, CapabilityImplications, utilities) → src/admin/capabilities.ts Revert data-provider version to 0.8.401 (no longer modified). * chore: Fix import ordering, rename appConfigService to service - Rename app/appConfigService.ts → app/service.ts (directory provides context) - Fix import order in admin/config.ts, types/admin.ts, types/config.ts - Add naming convention to AGENTS.md * feat: Add DB base config support (role/__base__) - Add BASE_CONFIG_PRINCIPAL_ID constant for reserved base config doc - getApplicableConfigs always includes __base__ in queries - getAppConfig queries DB even without role/userId when DB configs exist - Bump @librechat/data-schemas to 0.0.43 * fix: Address PR review issues for admin config - Add listAllConfigs method; listConfigs endpoint returns all active configs instead of only __base__ - Normalize principalId to string in all config methods to prevent ObjectId vs string mismatch on user/group lookups - Block __proto__ and all dunder-prefixed segments in field path validation to prevent prototype pollution - Fix configVersion off-by-one: default to 0, guard pre('save') with !isNew, use $inc on findOneAndUpdate - Remove unused getApplicableConfigs from admin handler deps * fix: Enable tree-shaking for data-schemas, bump packages - Switch data-schemas Rollup output to preserveModules so each source file becomes its own chunk; consumers (admin panel) can now import just the modules they need without pulling in winston/mongoose/etc. - Add sideEffects: false to data-schemas package.json - Bump data-schemas to 0.0.44, data-provider to 0.8.402 * feat: add capabilities subpath export to data-schemas Adds `@librechat/data-schemas/capabilities` subpath export so browser consumers can import BASE_CONFIG_PRINCIPAL_ID and capability constants without pulling in Node.js-only modules (winston, async_hooks, etc.). Bump version to 0.0.45. * fix: include dist/ in data-provider npm package Add explicit files field so npm includes dist/types/ in the published package. Without this, the root .gitignore exclusion of dist/ causes npm to omit type declarations, breaking TypeScript consumers. * chore: bump librechat-data-provider to 0.8.403 * feat: add GET /api/admin/config/base for raw AppConfig Returns the full AppConfig (YAML + DB base merged) so the admin panel can display actual config field values and structure. The startup config endpoint (/api/config) returns TStartupConfig which is a different shape meant for the frontend app. * chore: imports order * fix: address code review findings for admin config Critical: - Fix clearAppConfigCache: was deleting from wrong cache store (CONFIG_STORE instead of APP_CONFIG), now clears BASE and HAS_DB_CONFIGS keys - Eliminate race condition: patchConfigField and deleteConfigField now use atomic MongoDB $set/$unset with dot-path notation instead of read-modify-write cycles, removing the lost-update bug entirely - Add patchConfigFields and unsetConfigField atomic DB methods Major: - Reorder cache check before principal resolution in getAppConfig so getUserPrincipals DB query only fires on cache miss - Replace '' as ConfigSection with typed BROAD_CONFIG_ACCESS constant - Parallelize capability checks with Promise.all instead of sequential awaits in for loops - Use loose equality (== null) for cache miss check to handle both null and undefined returns from cache implementations - Set HAS_DB_CONFIGS_KEY to true on successful config fetch Minor: - Remove dead pre('save') hook from config schema (all writes use findOneAndUpdate which bypasses document hooks) - Consolidate duplicate type imports in resolution.ts - Remove dead deepGet/deepSet/deepUnset functions (replaced by atomic ops) - Add .sort({ priority: 1 }) to getApplicableConfigs query - Rename _impliedBy to impliedByMap * fix: self-referencing BROAD_CONFIG_ACCESS constant * fix: replace type-cast sentinel with proper null parameter Update hasConfigCapability to accept ConfigSection | null where null means broad access check (MANAGE_CONFIGS or READ_CONFIGS only). Removes the '' as ConfigSection type lie from admin config handlers. * fix: remaining review findings + add tests - listAllConfigs accepts optional { isActive } filter so admin listing can show inactive configs (danny-avila#9) - Standardize session application to .session(session ?? null) across all config DB methods (danny-avila#15) - Export isValidFieldPath and getTopLevelSection for testability - Add 38 tests across 3 spec files: - config.spec.ts (api): path validation, prototype pollution rejection - resolution.spec.ts: deep merge, priority ordering, array replacement - config.spec.ts (data-schemas): full CRUD, ObjectId normalization, atomic $set/$unset, configVersion increment, toggle, __base__ query * fix: address second code review findings - Fix cross-user cache contamination: overrideCacheKey now handles userId-without-role case with its own cache key (#1) - Add broad capability check before DB lookup in getConfig to prevent config existence enumeration (#2/#3) - Move deleteConfigField fieldPath from request body to query parameter for proxy/load balancer compatibility (danny-avila#5) - Derive BaseSystemCapability from SystemCapabilities const instead of manual string union (danny-avila#6) - Return 201 on upsert creation, 200 on update (danny-avila#11) - Remove inline narration comments per AGENTS.md (danny-avila#12) - Type overrides as Partial<TCustomConfig> in DB methods and handler deps (danny-avila#13) - Replace double as-unknown-as casts in resolution.ts with generic deepMerge<T> (danny-avila#14) - Make override cache TTL injectable via AppConfigServiceDeps (danny-avila#16) - Add exhaustive never check in principalModel switch (danny-avila#17) * fix: remaining review findings — tests, rename, semantics - Rename signalConfigChange → markConfigsDirty with JSDoc documenting the stale-window tradeoff and overrideCacheTtl knob - Fix DEFAULT_OVERRIDE_CACHE_TTL naming convention - Add createAppConfigService tests (14 cases): cache behavior, feature flag, cross-user key isolation, fallback on error, markConfigsDirty - Add admin handler integration tests (13 cases): auth ordering, 201/200 on create/update, fieldPath from query param, markConfigsDirty calls, capability checks * fix: global flag corruption + empty overrides auth bypass - Remove HAS_DB_CONFIGS_KEY=false optimization: a scoped query returning no configs does not mean no configs exist globally. Setting the flag false from a per-principal query short-circuited all subsequent users. - Add broad manage capability check before section checks in upsertConfigOverrides: empty overrides {} no longer bypasses auth. * test: add regression and invariant tests for config system Regression tests: - Bug 1: User A's empty result does not short-circuit User B's overrides - Bug 2: Empty overrides {} returns 403 without MANAGE_CONFIGS Invariant tests (applied across ALL handlers): - All 5 mutation handlers call markConfigsDirty on success - All 5 mutation handlers return 401 without auth - All 5 mutation handlers return 403 without capability - All 3 read handlers return 403 without capability * fix: third review pass — all findings addressed Service (service.ts): - Restore HAS_DB_CONFIGS=false for base-only queries (no role/userId) so deployments with zero DB configs skip DB queries (#1) - Resolve cache once at factory init instead of per-invocation (danny-avila#8) - Use BASE_CONFIG_PRINCIPAL_ID constant in overrideCacheKey (danny-avila#10) - Add JSDoc to clearAppConfigCache documenting stale-window (#4) - Fix log message to not say "from YAML" (danny-avila#14) Admin handlers (config.ts): - Use configVersion===1 for 201 vs 200, eliminating TOCTOU race (#2) - Add Array.isArray guard on overrides body (danny-avila#5) - Import CapabilityUser from capabilities.ts, remove duplicate (danny-avila#6) - Replace as-unknown-as cast with targeted type assertion (danny-avila#7) - Add MAX_PATCH_ENTRIES=100 cap on entries array (danny-avila#15) - Reorder deleteConfigField to validate principalType first (danny-avila#12) - Export CapabilityUser from middleware/capabilities.ts DB methods (config.ts): - Remove isActive:true from patchConfigFields to prevent silent reactivation of disabled configs (#3) Schema (config.ts): - Change principalId from Schema.Types.Mixed to String (danny-avila#11) Tests: - Add patchConfigField unsafe fieldPath rejection test (danny-avila#9) - Add base-only HAS_DB_CONFIGS=false test (#1) - Update 201/200 tests to use configVersion instead of findConfig (#2) * fix: add read handler 401 invariant tests + document flag behavior - Add invariant: all 3 read handlers return 401 without auth - Document on markConfigsDirty that HAS_DB_CONFIGS stays true after all configs are deleted until clearAppConfigCache or restart * fix: remove HAS_DB_CONFIGS false optimization entirely getApplicableConfigs([]) only queries for __base__, not all configs. A deployment with role/group configs but no __base__ doc gets the flag poisoned to false by a base-only query, silently ignoring all scoped overrides. The optimization is not safe without a comprehensive Config.exists() check, which adds its own DB cost. Removed entirely. The flag is now write-once-true (set when configs are found or by markConfigsDirty) and only cleared by clearAppConfigCache/restart. * chore: reorder import statements in app.js for clarity * refactor: remove HAS_DB_CONFIGS_KEY machinery entirely The three-state flag (false/null/true) was the source of multiple bugs across review rounds. Every attempt to safely set it to false was defeated by getApplicableConfigs querying only a subset of principals. Removed: HAS_DB_CONFIGS_KEY constant, all reads/writes of the flag, markConfigsDirty (now a no-op concept), notifyChange wrapper, and all tests that seeded false manually. The per-user/role TTL cache (overrideCacheTtl, default 60s) is the sole caching mechanism. On cache miss, getApplicableConfigs queries the DB. This is one indexed query per user per TTL window — acceptable for the config override use case. * docs: rewrite admin panel remaining work with current state * perf: cache empty override results to avoid repeated DB queries When getApplicableConfigs returns no configs for a principal, cache baseConfig under their override key with TTL. Without this, every user with no per-principal overrides hits MongoDB on every request after the 60s cache window expires. * fix: add tenantId to cache keys + reject PUBLIC principal type - Include tenantId in override cache keys to prevent cross-tenant config contamination. Single-tenant deployments (tenantId undefined) use '_' as placeholder — no behavior change for them. - Reject PrincipalType.PUBLIC in admin config validation — PUBLIC has no PrincipalModel and is never resolved by getApplicableConfigs, so config docs for it would be dead data. - Config middleware passes req.user.tenantId to getAppConfig. * fix: fourth review pass findings DB methods (config.ts): - findConfigByPrincipal accepts { includeInactive } option so admin GET can retrieve inactive configs (danny-avila#5) - upsertConfig catches E11000 duplicate key on concurrent upserts and retries without upsert flag (#2) - unsetConfigField no longer filters isActive:true, consistent with patchConfigFields (danny-avila#11) - Typed filter objects replace Record<string, unknown> (danny-avila#12) Admin handlers (config.ts): - patchConfigField: serial broad capability check before Promise.all to pre-warm ALS principal cache, preventing N parallel DB calls (#3) - isValidFieldPath rejects leading/trailing dots and consecutive dots (danny-avila#7) - Duplicate fieldPaths in patch entries return 400 (danny-avila#8) - DEFAULT_PRIORITY named constant replaces hardcoded 10 (danny-avila#14) - Admin getConfig and patchConfigField pass includeInactive to findConfigByPrincipal (danny-avila#5) - Route import uses barrel instead of direct file path (danny-avila#13) Resolution (resolution.ts): - deepMerge has MAX_MERGE_DEPTH=10 guard to prevent stack overflow from crafted deeply nested configs (#4) * fix: final review cleanup - Remove ADMIN_PANEL_REMAINING.md (local dev notes with Windows paths) - Add empty-result caching regression test - Add tenantId to AdminConfigDeps.getAppConfig type - Restore exhaustive never check in principalModel switch - Standardize toggleConfigActive session handling to options pattern * fix: validate priority in patchConfigField handler Add the same non-negative number validation for priority that upsertConfigOverrides already has. Without this, invalid priority values could be stored via PATCH and corrupt merge ordering. * chore: remove planning doc from PR * fix: correct stale cache key strings in service tests * fix: clean up service tests and harden tenant sentinel - Remove no-op cache delete lines from regression tests - Change no-tenant sentinel from '_' to '__default__' to avoid collision with a real tenant ID when multi-tenancy is enabled - Remove unused CONFIG_STORE from AppConfigServiceDeps * chore: bump @librechat/data-schemas to 0.0.46 * fix: block prototype-poisoning keys in deepMerge Skip __proto__, constructor, and prototype keys during config merge to prevent prototype pollution via PUT /api/admin/config overrides.

…anny-avila#12435) * feat: add MCPServerSource type, tenantMcpPolicy schema, and source-based dbSourced wiring - Add `tenantMcpPolicy` to `mcpSettings` in YAML config schema with `enabled`, `maxServersPerTenant`, `allowedTransports`, and `allowedDomains` - Add `MCPServerSource` type ('yaml' | 'config' | 'user') and `source` field to `ParsedServerConfig` - Change `dbSourced` determination from `!!config.dbId` to `config.source === 'user'` across MCPManager, ConnectionsRepository, UserConnectionManager, and MCPServerInspector - Set `source: 'user'` on all DB-sourced servers in ServerConfigsDB * feat: three-layer MCPServersRegistry with config cache and lazy init - Add `configCacheRepo` as third repository layer between YAML cache and DB for admin-defined config-source MCP servers - Implement `ensureConfigServers()` that identifies config-override servers from resolved `getAppConfig()` mcpConfig, lazily inspects them, and caches parsed configs with `source: 'config'` - Add `lazyInitConfigServer()` with timeout, stub-on-failure, and concurrent-init deduplication via `pendingConfigInits` map - Extend `getAllServerConfigs()` with optional `configServers` param for three-way merge: YAML → Config → User - Add `getServerConfig()` lookup through config cache layer - Add `invalidateConfigCache()` for clearing config-source inspection results on admin config mutations - Tag `source: 'yaml'` on CACHE-stored servers and `source: 'user'` on DB-stored servers in `addServer()` and `addServerStub()` * feat: wire tenant context into MCP controllers, services, and cache invalidation - Resolve config-source servers via `getAppConfig({ role, tenantId })` in `getMCPTools()` and `getMCPServersList()` controllers - Pass `ensureConfigServers()` results through `getAllServerConfigs()` for three-way merge of YAML + Config + User servers - Add tenant/role context to `getMCPSetupData()` and connection status routes via `getTenantId()` from ALS - Add `clearMcpConfigCache()` to `invalidateConfigCaches()` so admin config mutations trigger re-inspection of config-source MCP servers * feat: enforce tenantMcpPolicy on admin config mcpServers mutations - Add `validateMcpServerPolicy()` helper that checks mcpServers against operator-defined `tenantMcpPolicy` (enabled, maxServersPerTenant, allowedTransports, allowedDomains) - Wire validation into `upsertConfigOverrides` and `patchConfigField` handlers — rejects with 403 when policy is violated - Infer transport type from config shape (command → stdio, url protocol → websocket/sse, type field → streamable-http) - Validate server domains against policy allowlist when configured * revert: remove tenantMcpPolicy schema and enforcement The existing admin config CRUD routes already provide the mechanism for granular MCP server prepopulation (groups, roles, users). The tenantMcpPolicy gating adds unnecessary complexity that can be revisited if needed in the future. - Remove tenantMcpPolicy from mcpSettings Zod schema - Remove validateMcpServerPolicy helper and TenantMcpPolicy interface - Remove policy enforcement from upsertConfigOverrides and patchConfigField handlers * test: update test assertions for source field and config-server wiring - Use objectContaining in MCPServersRegistry reset test to account for new source: 'yaml' field on CACHE-stored configs - Add getTenantId and ensureConfigServers mocks to MCP route tests - Add getAppConfig mock to route test Config service mock - Update getMCPSetupData assertion to expect second options argument - Update getAllServerConfigs assertions for new configServers parameter * fix: disconnect active connections when config-source servers are evicted When admin config overrides change and config-source MCP servers are removed, the invalidation now proactively disconnects active connections for evicted servers instead of leaving them lingering until timeout. - Return evicted server names from invalidateConfigCache() - Disconnect app-level connections for evicted servers in clearMcpConfigCache() via MCPManager.appConnections.disconnect() * fix: address code review findings (CRITICAL, MAJOR, MINOR) CRITICAL fixes: - Scope configCacheRepo keys by config content hash to prevent cross-tenant cache poisoning when two tenants define the same server name with different configurations - Change dbSourced checks from `source === 'user'` to `source !== 'yaml' && source !== 'config'` so undefined source (pre-upgrade cached configs) fails closed to restricted mode MAJOR fixes: - Derive OAuth servers from already-computed mcpConfig instead of calling getOAuthServers() separately — config-source OAuth servers are now properly detected - Add parseInt radix (10) and NaN guard with fallback to 30_000 for CONFIG_SERVER_INIT_TIMEOUT_MS - Add CONFIG_CACHE_NAMESPACE to aggregate-key branch in ServerConfigsCacheFactory to avoid SCAN-based Redis stalls - Remove `if (role || tenantId)` guard in getMCPSetupData — config servers now always resolve regardless of tenant context MINOR fixes: - Extract resolveAllMcpConfigs() helper in mcp controller to eliminate 3x copy-pasted config resolution boilerplate - Distinguish "not initialized" from real errors in clearMcpConfigCache — log actual failures instead of swallowing - Remove narrative inline comments per style guide - Remove dead try/catch inside Promise.allSettled in ensureConfigServers (inner method never throws) - Memoize YAML server names to avoid repeated cacheConfigsRepo.getAll() calls per request Test updates: - Add ensureConfigServers mock to registry test fixtures - Update getMCPSetupData assertions for inline OAuth derivation * fix: address code review findings (CRITICAL, MAJOR, MINOR) CRITICAL fixes: - Break circular dependency: move CONFIG_CACHE_NAMESPACE from MCPServersRegistry to ServerConfigsCacheFactory - Fix dbSourced fail-closed: use source field when present, fall back to legacy dbId check when absent (backward-compatible with pre-upgrade cached configs that lack source field) MAJOR fixes: - Add CONFIG_CACHE_NAMESPACE to aggregate-key set in ServerConfigsCacheFactory to avoid SCAN-based Redis stalls - Add comprehensive test suite (ensureConfigServers.test.ts, 18 tests) covering lazy init, stub-on-failure, cross-tenant isolation via config hash keys, concurrent deduplication, merge order, and cache invalidation MINOR fixes: - Update MCPServerInspector test assertion for dbSourced change * fix: restore getServerConfig lookup for config-source servers (NEW-1) Add configNameToKey map that indexes server name → hash-based cache key for O(1) lookup by name in getServerConfig. This restores the config cache layer that was dropped when hash-based keys were introduced. Without this fix, config-source servers appeared in tool listings (via getAllServerConfigs) but getServerConfig returned undefined, breaking all connection and tool call paths. - Populate configNameToKey in ensureSingleConfigServer - Clear configNameToKey in invalidateConfigCache and reset - Clear stale read-through cache entries after lazy init - Remove dead code in invalidateConfigCache (config.title, key parsing) - Add getServerConfig tests for config-source server lookup * fix: eliminate configNameToKey race via caller-provided configServers param Replace the process-global configNameToKey map (last-writer-wins under concurrent multi-tenant load) with a configServers parameter on getServerConfig. Callers pass the pre-resolved config servers map directly — no shared mutable state, no cross-tenant race. - Add optional configServers param to getServerConfig; when provided, returns matching config directly without any global lookup - Remove configNameToKey map entirely (was the source of the race) - Extract server names from cache keys via lastIndexOf in invalidateConfigCache (safe for names containing colons) - Use mcpConfig[serverName] directly in getMCPTools instead of a redundant getServerConfig call - Add cross-tenant isolation test for getServerConfig * fix: populate read-through cache after config server lazy init After lazyInitConfigServer succeeds, write the parsed config to readThroughCache keyed by serverName so that getServerConfig calls from ConnectionsRepository, UserConnectionManager, and MCPManager.callTool find the config without needing configServers. Without this, config-source servers appeared in tool listings but every connection attempt and tool call returned undefined. * fix: user-scoped getServerConfig fallback to server-only cache key When getServerConfig is called with a userId (e.g., from callTool or UserConnectionManager), the cache key is serverName::userId. Config-source servers are cached under the server-only key (no userId). Add a fallback so user-scoped lookups find config-source servers in the read-through cache. * fix: configCacheRepo fallback, isUserSourced DRY, cross-process race CRITICAL: Add findInConfigCache fallback in getServerConfig so config-source servers remain reachable after readThroughCache TTL expires (5s). Without this, every tool call after 5s returned undefined for config-source servers. MAJOR: Extract isUserSourced() helper to mcp/utils.ts and replace all 5 inline dbSourced ternary expressions (MCPManager x2, ConnectionsRepository, UserConnectionManager, MCPServerInspector). MAJOR: Fix cross-process Redis race in lazyInitConfigServer — when configCacheRepo.add throws (key exists from another process), fall back to reading the existing entry instead of returning undefined. MINOR: Parallelize invalidateConfigCache awaits with Promise.all. Remove redundant .catch(() => {}) inside Promise.allSettled. Tighten dedup test assertion to toBe(1). Add TTL-expiry tests for getServerConfig (with and without userId). * feat: thread configServers through getAppToolFunctions and formatInstructionsForContext Add optional configServers parameter to getAppToolFunctions, getInstructions, and formatInstructionsForContext so config-source server tools and instructions are visible to agent initialization and context injection paths. Existing callers (boot-time init, tests) pass no argument and continue to work unchanged. Agent runtime paths can now thread resolved config servers from request context. * fix: stale failure stubs retry after 5 min, upsert for cross-process races - Add CONFIG_STUB_RETRY_MS (5 min) — stale failure stubs are retried instead of permanently disabling config-source servers after transient errors (DNS outage, cold-start race) - Extract upsertConfigCache() helper that tries add then falls back to update, preventing cross-process Redis races where a second instance's successful inspection result was discarded - Add test for stale-stub retry after CONFIG_STUB_RETRY_MS * fix: stamp updatedAt on failure stubs, null-guard callTool config, test cleanup - Add updatedAt: Date.now() to failure stubs in lazyInitConfigServer so CONFIG_STUB_RETRY_MS (5 min) window works correctly — without it, stubs were always considered stale (updatedAt ?? 0 → epoch → always expired) - Add null guard for rawConfig in MCPManager.callTool before passing to preProcessGraphTokens — prevents unsafe `as` cast on undefined - Log double-failure in upsertConfigCache instead of silently swallowing - Replace module-scope Date.now monkey-patch with jest.useFakeTimers / jest.setSystemTime / jest.useRealTimers in ensureConfigServers tests * fix: server-only readThrough fallback only returns truthy values Prevents a cached undefined from a prior no-userId lookup from short-circuiting the DB query on a subsequent userId-scoped lookup. * fix: remove findInConfigCache to eliminate cross-tenant config leakage The findInConfigCache prefix scan (serverName:*) could return any tenant's config after readThrough TTL expires, violating tenant isolation. Config-source servers are now ONLY resolvable through: 1. The configServers param (callers with tenant context from ALS) 2. The readThrough cache (populated by ensureSingleConfigServer, 5s TTL, repopulated on every HTTP request via resolveAllMcpConfigs) Connection/tool-call paths without tenant context rely exclusively on the readThrough cache. If it expires before the next HTTP request repopulates it, the server is not found — which is correct because there is no tenant context to determine which config to return. - Remove findInConfigCache method and its call in getServerConfig - Update server-only readThrough fallback to only return truthy values (prevents cached undefined from short-circuiting user-scoped DB lookup) - Update tests to document tenant isolation behavior after cache expiry * style: fix import order per AGENTS.md conventions Sort package imports shortest-to-longest, local imports longest-to-shortest across MCPServersRegistry, ConnectionsRepository, MCPManager, UserConnectionManager, and MCPServerInspector. * fix: eliminate cross-tenant readThrough contamination and TTL-expiry tool failures Thread pre-resolved serverConfig from tool creation context into callTool, removing dependency on the readThrough cache for config-source servers. This fixes two issues: - Cross-tenant contamination: the readThrough cache key was unscoped (just serverName), so concurrent multi-tenant requests for same-named servers would overwrite each other's entries - TTL expiry: tool calls happening >5s after config resolution would fail with "Configuration not found" because the readThrough entry had expired Changes: - Add optional serverConfig param to MCPManager.callTool — uses provided config directly, falling back to getServerConfig lookup for YAML/user servers - Thread serverConfig from createMCPTool through createToolInstance closure to callTool - Remove readThrough write from ensureSingleConfigServer — config-source servers are only accessible via configServers param (tenant-scoped) - Remove server-only readThrough fallback from getServerConfig - Increase config cache hash from 8 to 16 hex chars (64-bit) - Add isUserSourced boundary tests for all source/dbId combinations - Fix double Object.keys call in getMCPTools controller - Update test assertions for new getServerConfig behavior * fix: cache base configs for config-server users; narrow upsertConfigCache error handling - Refactor getAllServerConfigs to separate base config fetch (YAML + DB) from config-server layering. Base configs are cached via readThroughCacheAll regardless of whether configServers is provided, eliminating uncached MongoDB queries per request for config-server users - Narrow upsertConfigCache catch to duplicate-key errors only; infrastructure errors (Redis timeouts, network failures) now propagate instead of being silently swallowed, preventing inspection storms during outages * fix: restore correct merge order and document upsert error matching - Restore YAML → Config → User DB precedence in getAllServerConfigs (user DB servers have highest precedence, matching the JSDoc contract) - Add source comment on upsertConfigCache duplicate-key detection linking to the two cache implementations that define the error message * feat: complete config-source server support across all execution paths Wire configServers through the entire agent execution pipeline so config-source MCP servers are fully functional — not just visible in listings but executable in agent sessions. - Thread configServers into handleTools.js agent tool pipeline: resolve config servers from tenant context before MCP tool iteration, pass to getServerConfig, createMCPTools, and createMCPTool - Thread configServers into agent instructions pipeline: applyContextToAgent → getMCPInstructionsForServers → formatInstructionsForContext, resolved in client.js before agent context application - Add configServers param to createMCPTool and createMCPTools for reconnect path fallback - Add source field to redactServerSecrets allowlist for client UI differentiation of server tiers - Narrow invalidateConfigCache to only clear readThroughCacheAll (merged results), preserving YAML individual-server readThrough entries - Update context.spec.ts assertions for new configServers parameter * fix: add missing mocks for config-source server dependencies in client.test.js Mock getMCPServersRegistry, getAppConfig, and getTenantId that were added to client.js but not reflected in the test file's jest.mock declarations. * fix: update formatInstructionsForContext assertions for configServers param The test assertions expected formatInstructionsForContext to be called with only the server names array, but it now receives configServers as a second argument after the config-source server feature wiring. * fix: move configServers resolution before MCP tool loop to avoid TDZ configServers was declared with `let` after the first tool loop but referenced inside it via getServerConfig(), causing a ReferenceError temporal dead zone. Move declaration and resolution before the loop, using tools.some(mcpToolPattern) to gate the async resolution. * fix: address review findings — cache bypass, discoverServerTools gap, DRY - #2: getAllServerConfigs now always uses getBaseServerConfigs (cached via readThroughCacheAll) instead of bypassing it when configServers is present. Extracts user-DB entries from cached base by diffing against YAML keys to maintain YAML → Config → User DB merge order without extra MongoDB calls. - #3: Add configServers param to ToolDiscoveryOptions and thread it through discoverServerTools → getServerConfig so config-source servers are discoverable during OAuth reconnection flows. - danny-avila#6: Replace inline import() type annotations in context.ts with proper import type { ParsedServerConfig } per AGENTS.md conventions. - danny-avila#7: Extract resolveConfigServers(req) helper in MCP.js and use it from handleTools.js and client.js, eliminating the duplicated 6-line config resolution pattern. - danny-avila#10: Restore removed "why" comment explaining getLoaded() vs getAll() choice in getMCPSetupData — documents non-obvious correctness constraint. - danny-avila#11: Fix incomplete JSDoc param type on resolveAllMcpConfigs. * fix: consolidate imports, reorder constants, fix YAML-DB merge edge case - Merge duplicate @librechat/data-schemas requires in MCP.js into one - Move resolveConfigServers after module-level constants - Fix getAllServerConfigs edge case where user-DB entry overriding a YAML entry with the same name was excluded from userDbConfigs; now uses reference equality check to detect DB-overwritten YAML keys * fix: replace fragile string-match error detection with proper upsert method Add upsert() to IServerConfigsRepositoryInterface and all implementations (InMemory, Redis, RedisAggregateKey, DB). This eliminates the brittle error message string match ('already exists in cache') in upsertConfigCache that was the only thing preventing cross-process init races from silently discarding inspection results. Each implementation handles add-or-update atomically: - InMemory: direct Map.set() - Redis: direct cache.set() - RedisAggregateKey: read-modify-write under write lock - DB: delegates to update() (DB servers use explicit add() with ACL setup) * fix: wire configServers through remaining HTTP endpoints - getMCPServerById: use resolveAllMcpConfigs instead of bare getServerConfig - reinitialize route: resolve configServers before getServerConfig - auth-values route: resolve configServers before getServerConfig - getOAuthHeaders: accept configServers param, thread from callers - Update mcp.spec.js tests to mock getAllServerConfigs for GET by name * fix: thread serverConfig through getConnection for config-source servers Config-source servers exist only in configCacheRepo, not in YAML cache or DB. When callTool → getConnection → getUserConnection → getServerConfig runs without configServers, it returns undefined and throws. Fix by threading the pre-resolved serverConfig (providedConfig) from callTool through getConnection → getUserConnection → createUserConnectionInternal, using it as a fallback before the registry lookup. * fix: thread configServers through reinit, reconnect, and tool definition paths Wire configServers through every remaining call chain that creates or reconnects MCP server connections: - reinitMCPServer: accepts serverConfig and configServers, uses them for getServerConfig fallback, getConnection, and discoverServerTools - reconnectServer: accepts and passes configServers to reinitMCPServer - createMCPTools/createMCPTool: pass configServers to reconnectServer - ToolService.loadToolDefinitionsWrapper: resolves configServers from req, passes to both reinitMCPServer call sites - reinitialize route: passes serverConfig and configServers to reinitMCPServer * fix: address review findings — simplify merge, harden error paths, fix log labels - Simplify getAllServerConfigs merge: replace fragile reference-equality loop with direct spread { ...yamlConfigs, ...configServers, ...base } - Guard upsertConfigCache in lazyInitConfigServer catch block so cache failures don't mask the original inspection error - Deduplicate getYamlServerNames cold-start with promise dedup pattern - Remove dead `if (!mcpConfig)` guard in getMCPSetupData - Fix hardcoded "App server" in ServerConfigsCacheRedisAggregateKey error messages — now uses this.namespace for correct Config/App labeling - Remove misleading OAuth callback comment about readThrough cache - Move resolveConfigServers after module-level constants in MCP.js * fix: clear rejected yamlServerNames promise, fix config-source reinspect, fix reset log label - Clear yamlServerNamesPromise on rejection so transient cache errors don't permanently prevent ensureConfigServers from working - Skip reinspectServer for config-source servers (source: 'config') in reinitMCPServer — they lack a CACHE/DB storage location; retry is handled by CONFIG_STUB_RETRY_MS in ensureConfigServers - Use source field instead of dbId for storageLocation derivation - Fix remaining hardcoded "App" in reset() leaderCheck message * fix: persist oauthHeaders in flow state for config-source OAuth servers The OAuth callback route has no JWT auth context and cannot resolve config-source server configs. Previously, getOAuthHeaders would silently return {} for config-source servers, dropping custom token exchange headers. Now oauthHeaders are persisted in MCPOAuthFlowMetadata during flow initiation (which has auth context), and the callback reads them from the stored flow state with a fallback to the registry lookup for YAML/user-DB servers. * fix: update tests for getMCPSetupData null guard removal and ToolService mock - MCP.spec.js: update test to expect graceful handling of null mcpConfig instead of a throw (getAllServerConfigs always returns an object) - MCP.js: add defensive || {} for Object.entries(mcpConfig) in case of null from test mocks - ToolService.spec.js: add missing mock for ~/server/services/MCP (resolveConfigServers) * fix: address review findings — DRY, naming, logging, dead code, defensive guards - #1: Simplify getAllServerConfigs to single getBaseServerConfigs call, eliminating redundant double-fetch of cacheConfigsRepo.getAll() - #2: Add warning log when oauthHeaders absent from OAuth callback flow state - #3: Extract resolveAllMcpConfigs to MCP.js service layer; controller imports shared helper instead of reimplementing - #4: Rename _serverConfig/_provider to capturedServerConfig/capturedProvider in createToolInstance — these are actively used, not unused - danny-avila#5: Log rejected results from ensureConfigServers Promise.allSettled so cache errors are visible instead of silently dropped - danny-avila#6: Remove dead 'MCP config not found' error handlers from routes - danny-avila#7: Document circular-dependency reason for dynamic require in clearMcpConfigCache - danny-avila#8: Remove logger.error from withTimeout to prevent double-logging timeouts - danny-avila#10: Add explicit userId guard in ServerConfigsDB.upsert with clear error message - danny-avila#12: Use spread instead of mutation in addServer for immutability consistency - Add upsert mock to ensureConfigServers.test.ts DB mock - Update route tests for resolveAllMcpConfigs import change * fix: restore correct merge priority, use immutable spread, fix test mock - getAllServerConfigs: { ...configServers, ...base } so userDB wins over configServers, matching documented "User DB (highest)" priority - lazyInitConfigServer: use immutable spread instead of direct mutation for parsedConfig.source, consistent with addServer fix - Fix test to mock getAllServerConfigs as {} instead of null, remove unnecessary || {} defensive guard in getMCPSetupData * fix: error handling, stable hashing, flatten nesting, remove dead param - Wrap resolveConfigServers/resolveAllMcpConfigs in try/catch with graceful {} fallback so transient DB/cache errors don't crash tool pipeline - Sort keys in configCacheKey JSON.stringify for deterministic hashing regardless of object property insertion order - Flatten clearMcpConfigCache from 3 nested try-catch to early returns; document that user connections are cleaned up lazily (accepted tradeoff) - Remove dead configServers param from getAppToolFunctions (never passed) - Add security rationale comment for source field in redactServerSecrets * fix: use recursive key-sorting replacer in configCacheKey to prevent cross-tenant cache collision The array replacer in JSON.stringify acts as a property allowlist at every nesting depth, silently dropping nested keys like headers['X-API-Key'], oauth.client_secret, etc. Two configs with different nested values but identical top-level structure produced the same hash, causing cross-tenant cache hits and potential credential contamination. Switch to a function replacer that recursively sorts keys at all depths without dropping any properties. Also document the known gap in getOAuthServers: config-source OAuth servers are not covered by auto-reconnection or uninstall cleanup because callers lack request context. * fix: move clearMcpConfigCache to packages/api to eliminate circular dependency The function only depends on MCPServersRegistry and MCPManager, both of which live in packages/api. Import it directly from @librechat/api in the CJS layer instead of using dynamic require('~/config'). * chore: imports/fields ordering * fix: address review findings — error handling, targeted lookup, test gaps - Narrow resolveAllMcpConfigs catch to only wrap ensureConfigServers so getAppConfig/getAllServerConfigs failures propagate instead of masking infrastructure errors as empty server lists. - Use targeted getServerConfig in getMCPServerById instead of fetching all server configs for a single-server lookup. - Forward configServers to inner createMCPTool calls so reconnect path works for config-source servers. - Update getAllServerConfigs JSDoc to document disjoint-key design. - Add OAuth callback oauthHeaders fallback tests (flow state present vs registry fallback). - Add resolveConfigServers/resolveAllMcpConfigs unit tests covering happy path and error propagation. * fix: add getOAuthReconnectionManager mock to OAuth callback tests * chore: imports ordering

…ain Collision (danny-avila#12594) * 🐛 fix: resolve Action tools by exact tool name to prevent multi-action collision When two OpenAPI Actions on the same Agent share a hostname, the second action's entry overwrote the first in the encoded-domain Map and one action's tools silently disappeared from the LLM payload. The buggy resolution loop also used substring matching, which caused similar shadowing for any encoded-domain prefix overlap. This change builds a Map keyed on the full tool name (`<operationId>_action_<encoded-domain>`) directly, mirroring the exact lookup pattern that getActionToolDefinitions already uses. Each function in an action's spec gets its own slot, so two actions sharing a hostname no longer collide. Both the new and the legacy domain encodings are registered for each function so agents whose stored tool names predate the current encoding still resolve. Applied at all three call sites that had the buggy pattern: - processRequiredActions (assistants/threads path) - loadAgentTools (agent build path) - loadActionToolsForExecution (agent execution path) Adds three regression tests covering both ordering directions and the execution path. Tests fail without the fix and pass with it. * 🐛 fix: Normalize action tool name at lookup + cover assistants path Follow-up to the multi-action domain collision fix. Addresses PR danny-avila#12594 review feedback: **Must-fix #1 — short-hostname lookup mismatch.** The toolToAction map is keyed on the `_`-collapsed domain, but `agent.tools` and `currentAction.tool` persist the raw `domainParser(..., true)` output, which for hostnames ≤ ENCODED_DOMAIN_LENGTH is a `---`-separated string (e.g. `medium---com`). Exact-match `Map.get()` missed those keys and silently dropped the tool. Fix: normalize every incoming tool name through a new `normalizeActionToolName` helper before the lookup in `loadAgentTools`, `processRequiredActions`, and `loadActionToolsForExecution`. **Must-fix #2 — assistants path coverage.** `processRequiredActions` received the same structural rewrite but had zero tests. Added a regression test under `multi-action domain collision regression` that drives two shared-hostname actions through the assistants path and asserts each tool reaches its own request builder. **Must-fix #3 — legacy encoding branch coverage.** The `if (legacyNormalized !== normalizedDomain)` registration was never exercised by any test. Added a test where `agent.tools` stores the legacy-format name and asserts it still resolves. **Should-fix #4 — DRY the registration loop.** Extracted `registerActionTools({ toolToAction, functionSignatures, normalizedDomain, legacyNormalized, makeEntry })`. All three call sites now share the same key-building logic; the key template lives in one place. **Should-fix danny-avila#5 — remove stale optional chaining.** In `loadActionToolsForExecution`, `functionSignature?.description ?? ''` became `functionSignature.description` — `sig` is always defined by the iterator, matching the style of `loadAgentTools`. **Should-fix danny-avila#6 — drop unreachable `!requestBuilder` guard.** Entries in `processRequiredActions` are now pre-built with `requestBuilder: requestBuilders[sig.name]`, which `openapiToFunction` always produces alongside the signature, so the guard is dead. **Should-fix danny-avila#7 — unwrap `actionSetsData`.** It now holds a bare `Map` instead of `{ toolToAction }`; the sentinel `!actionSetsData` check still works because `new Map()` is truthy. Also added a short-hostname regression test (`loadAgentTools resolves raw ---separated tool names`) that reproduces Must-fix #1: it fails against the previous commit (0 create calls) and passes with the normalization in place. 41 tests, all passing. The 3 new regression tests are under `multi-action domain collision regression` and cover the assistants path, the legacy encoding branch, and the short-hostname lookup path. * 🐛 fix: Tighten registerActionTools key handling and assistants test Follow-up to d643444 addressing the second review pass on PR danny-avila#12594. **ESLint** — Two prettier errors in the spec file (multi-line arrow function bodies that should fit on one line). Auto-fixed. **[MINOR] operationId containing `---` → key mismatch.** The lookup path collapsed every `actionDomainSeparator` sequence in the full tool name, but the registration path passed `sig.name` through unchanged. A `---` that survived into an operationId would shift the underscore boundary at lookup and miss its own key. Fix in `registerActionTools`: normalize `sig.name` with the same helper so registration and lookup always agree on the canonical form. `sanitizeOperationId` strips the characters that produce `---` in practice, so this is theoretical hardening, not a fix for a known reproducer. **[MINOR] Same-operationId + same-hostname silent overwrite.** Two actions sharing both an operationId and a hostname still produced a silent `Map.set()` overwrite (the new key is identical, so neither the operationId nor the domain disambiguates). Added a `setKey` helper inside `registerActionTools` that logs a `[Actions] operationId collision: ...` warning whenever a key is already present, naming the overwriting action_id. The silent-overwrite mode from the original bug cannot reappear under a different disguise without surfacing in the logs. **[NIT] processRequiredActions test simulated a runtime crash.** `mockCreateActionTool` returned a tool with `_call: jest.fn()`, which resolves to `undefined`. `processRequiredActions` chains `.then(handleToolOutput).catch(handleToolError)` directly onto that return, so `undefined.then(...)` threw synchronously and the outer try/catch funneled the error into `handleToolError`. Creation count assertions still passed because `createActionTool` runs before the crash, but the test was silently exercising the failure path. Updated the global mock to `_call: jest.fn().mockResolvedValue('{"status":"ok"}')` so the success path runs end-to-end. The assistants regression test now executes in ~5ms instead of ~90ms, which corroborates that it's no longer hitting the synchronous throw. **[NIT] Duplicated rationale comments.** All three call sites carried multi-line comment blocks restating why we key on the full tool name. That rationale now lives canonically in `registerActionTools`'s JSDoc; the inline blocks collapsed to `// See registerActionTools for the key-shape rationale.` Net -22 lines of comments. 41/41 tests still pass; lint is clean. * 🐛 fix: Scope tool-name normalization to the encoded-domain suffix Follow-up to f22228e addressing the Codex P1 on PR danny-avila#12594. **Regression.** The previous commit normalized the entire tool name (`normalizeActionToolName(sig.name)` at registration, full-name `.replace()` at lookup) to handle operationIds that theoretically contained `---`. But `openapiToFunction` uses user-supplied operationIds verbatim and the fallback `sanitizeOperationId` only strips characters outside `[a-zA-Z0-9_-]`, so specs can legitimately produce operationIds like `get_foo---bar` and `get_foo_bar` side by side. Collapsing `---` to `_` across the entire key merged those two into a single map slot — one silently overwrote the other, and both tool requests routed to the surviving entry's request builder. **Fix.** Limit normalization to the encoded-domain portion of the full tool name, i.e. the substring after the last `actionDelimiter`. The operationId half is left verbatim, so hyphens-vs-underscores remain disambiguating. The short-hostname bug (Must-fix #1 from the original review) is still covered because the `---` → `_` collapse still happens on the domain suffix where it matters: ```js const normalizeActionToolName = (toolName) => { const delimiterIndex = toolName.lastIndexOf(actionDelimiter); if (delimiterIndex === -1) return toolName; const prefixEnd = delimiterIndex + actionDelimiter.length; const encodedDomain = toolName.slice(prefixEnd); return toolName.slice(0, prefixEnd) + encodedDomain.replace(domainSeparatorRegex, '_'); }; ``` `registerActionTools` reverts to `sig.name` verbatim — no more `normalizeActionToolName(sig.name)` ahead of the key build. **Regression test.** Added `loadAgentTools distinguishes operationIds that differ only by ---` vs `_`` under `multi-action domain collision regression`. It loads two actions sharing a hostname whose operationIds are `get_foo---bar` and `get_foo_bar` respectively, each pointing at a different path (`/foo-bar`, `/foo_bar`), and asserts that each tool resolves to its own request builder. Verified the test fails against f22228e (`hyphenTool` resolves to `/foo_bar` — the sibling's builder) and passes with this commit. 42/42 tests pass; lint clean.

* 🫧 fix: Restore Claude Opus 4.7 Reasoning Visibility Claude Opus 4.7 omits `thinking` content from Messages API responses by default — empty thinking blocks still stream, but the `thinking` field is blank unless the caller passes `display: "summarized"` in the adaptive thinking config. This silenced the LibreChat "Thoughts" UI for Anthropic (and Anthropic-on-Bedrock) adaptive models. - Extend `ThinkingConfigAdaptive` in `packages/api/src/types/anthropic.ts` with an optional `display: 'summarized' | 'omitted'` field - Emit `{ type: 'adaptive', display: 'summarized' }` from `configureReasoning` in `packages/api/src/endpoints/anthropic/helpers.ts` - Emit `{ type: 'adaptive', display: 'summarized' }` from `bedrockInputParser` in `packages/data-provider/src/bedrock.ts` and update the local `ThinkingConfig` union - Update existing adaptive-thinking assertions to include the new field - Add dedicated tests asserting `display: 'summarized'` flows through both the Anthropic endpoint and the Bedrock parser See https://platform.claude.com/docs/en/about-claude/models/whats-new-claude-4-7#thinking-content-omitted-by-default * refactor: Gate `display: summarized` on Opus 4.7+ Narrow the reasoning-visibility opt-in to the models that actually omit thinking content by default, instead of applying it to every adaptive model. Pre-Opus-4.7 adaptive models (Opus 4.6, Sonnet 4.6) already return summaries, so sending the field is unnecessary noise. - Add `omitsThinkingByDefault(model)` in `packages/data-provider/src/bedrock.ts` that returns true only for Opus 4.7+ (including future majors like Opus 5+) - Bedrock parser now only attaches `display: 'summarized'` when the helper matches, keeping the adaptive object unchanged for older models - Anthropic endpoint `configureReasoning` uses the same helper so its emit path matches the Bedrock one - Tests: replace the blanket `display: 'summarized'` assertions with model-specific ones (Opus 4.7 gets it, Opus 4.6 / Sonnet 4.6 do not), add a dedicated `omitsThinkingByDefault` suite covering naming variants and future versions * feat: Configurable Thought Visibility for Anthropic Adaptive Models Expose the Anthropic `thinking.display` API field as a user-facing parameter so users can override the `auto` default (which stays as the Opus-4.7+ opt-in added earlier in this PR). Also fixes the CI type error by widening the adaptive thinking type assignment via a resolver helper that returns a properly-typed object. - Add `ThinkingDisplay` enum (`auto` | `summarized` | `omitted`) and matching zod schema in `packages/data-provider/src/schemas.ts` - Add `thinkingDisplay` to `tConversationSchema`, `anthropicSettings`, and the pick lists for Bedrock input/parser + Anthropic agent params - Add `resolveThinkingDisplay(model, explicit)` helper in `packages/data-provider/src/bedrock.ts` that returns the wire value or undefined (auto → model default, explicit → always honored) - `bedrockInputParser` now reads `thinkingDisplay` from input and emits `display` only when the resolver returns a value; strips the field on non-adaptive-model branches so it does not leak - `configureReasoning` in the Anthropic endpoint threads `thinkingDisplay` through, uses the resolver, and casts the adaptive config to `AnthropicClientOptions['thinking']` so the widened shape compiles against the stale installed SDK types - Add UI slider for `thinkingDisplay` in `parameterSettings.ts` next to `effort`, with three-position `com_ui_auto` / `com_ui_summarized` / `com_ui_omitted` labels - Add translation keys `com_endpoint_anthropic_thinking_display`, `com_endpoint_anthropic_thinking_display_desc`, `com_ui_summarized`, `com_ui_omitted` - Add tests: `resolveThinkingDisplay` suite (5 cases covering auto / explicit / unknown input), parser round-trip tests for all three modes on Opus 4.6 and Opus 4.7, Anthropic endpoint tests for explicit summarized/omitted overrides * fix: Drop `thinkingDisplay` When Adaptive Thinking Is Disabled If a user turns adaptive thinking off but had previously selected a `thinkingDisplay` value, the stale field was left in `additionalFields` and ended up merged into the Bedrock request's `additionalModelRequestFields`. That leaks a non-Bedrock key into the payload and can round-trip back into `llmConfig`. - Delete `additionalFields.thinkingDisplay` alongside `thinking` and `thinkingBudget` in the `thinking === false` branch of `bedrockInputParser` - Add a regression test asserting `thinking`, `thinkingBudget`, and `thinkingDisplay` are all absent when adaptive thinking is disabled on an Opus 4.7 request Reported by chatgpt-codex-connector on PR danny-avila#12701. * refactor: Consolidate `ThinkingDisplay` Types and Preserve Persisted Display Address review findings on PR danny-avila#12701: - [Codex P2] `bedrockInputSchema.transform` now extracts `thinking.display` from persisted `additionalModelRequestFields` back into the top-level `thinkingDisplay` field so explicit `'omitted'` round-trips through storage instead of being silently reverted to `'summarized'` on the next parse. - [Codex P2] `getLLMConfig` in the Anthropic endpoint now reads `.display` from a persisted `thinking` object (agents store the full Anthropic shape) and uses it as the fallback for `thinkingDisplay` when no top-level override is present. - [Audit #2] Collapse the three parallel wire-value types into a single `ThinkingDisplayWireValue = Exclude<ThinkingDisplay, 'auto'>` exported from `schemas.ts`; remove the duplicate `ThinkingDisplay` alias in `packages/api/src/types/anthropic.ts` (which collided with the enum name) and the `ThinkingDisplayValue` alias in `bedrock.ts`. - [Audit #3] Add `thinkingDisplay` to the `TEndpointOption` pick list next to `effort`. - [Audit #4] Add a TODO comment next to the `as AnthropicClientOptions['thinking']` cast explaining the stale `@librechat/agents` SDK types that require it. - Add tests: four round-trip cases asserting `bedrockInputSchema` recovers `display` from persisted AMRF (Opus 4.7 omitted, pre-4.7 summarized, unknown-value ignore, explicit top-level wins), and two `getLLMConfig` cases asserting the Anthropic endpoint preserves and overrides persisted `thinking.display`. * fix: Preserve Persisted `thinking.display` in bedrockInputParser The parser constructed a fresh adaptive thinking config without looking at any `display` already embedded in the incoming `additionalModelRequestFields.thinking`. On round-trip through `initializeBedrock`, a persisted user choice of `'omitted'` on Opus 4.7+ was silently reverted to `'summarized'` by the auto fallback. - Extract `extractPersistedDisplay` helper and reuse it in both the schema transform (form-state round-trip) and the parser (wire-request round-trip) - `bedrockInputParser` now feeds the persisted display as the resolver's explicit value when no top-level `thinkingDisplay` override is set - Add regression tests: parser preserves `display: 'omitted'` for persisted Opus 4.7 AMRF, and top-level `thinkingDisplay` still wins over persisted AMRF display Reported by chatgpt-codex-connector (P1) on PR danny-avila#12701.

…avila#12737) * 🔊 fix: Preserve Log Metadata on Console for Warn/Error Levels The default console formatter discarded every structured metadata key on the winston info object — only `CONSOLE_JSON=true` preserved it. That meant failures emitted by the agents SDK (e.g. "Summarization LLM call failed") reached stdout without the provider, model, or underlying error attached, leaving users unable to diagnose the root cause. - Add `formatConsoleMeta` helper to serialize non-reserved metadata as a compact JSON trailer, with per-value string truncation and safe handling of circular references. - Append the metadata trailer to warn/error console lines; info/debug behavior is unchanged. - Relax `debugTraverse`'s debug-only gate so warn/error messages routed through the debug formatter also surface their metadata. - Add a `formatConsoleMeta` stub to the shared logger mock so existing tests keep working. * 🔐 fix: Also Redact Sensitive Patterns on Warn Console Lines The warn-level console output now includes a metadata trailer that may contain provider-returned error strings with embedded tokens or keys (e.g. `Bearer ...`, `sk-...`). Apply `redactMessage` to warn lines in addition to error, matching the new surface area. * 🔐 fix: Redact Sensitive Tokens Embedded in JSON Metadata Two gaps in the existing console redaction that became user-visible once warn/error lines started emitting structured metadata: 1. The OpenAI-key regex (`/^(sk-)[^\s]+/`) was anchored to start-of-line, so keys embedded inside JSON payloads (e.g. `{"apiKey":"sk-..."}`) were never redacted. Every console line begins with a timestamp, so the anchor effectively made this pattern dead code. 2. `formatConsoleMeta` stringified metadata values verbatim; a sensitive string value was only redacted by the whole-line regex pass, which missed the anchored `sk-` case above. Fix: - Drop the `^` anchor; add `/g` so every occurrence is redacted, not just the first. - Also exclude `"` and `'` from the token body so JSON-embedded values terminate at the closing quote rather than chewing into the next field. - Simplify `redactMessage` to apply patterns directly (dropping the `getMatchingSensitivePatterns` filter) — the filter used `.test()` which has stateful behavior on `/g` regexes and is no longer needed. - `formatConsoleMeta` now runs `redactMessage` over every string value before JSON serialization, so the metadata trailer is safe even on the warn path. - Add regression tests covering both fixes. Reviewed-by: Codex (P1 finding on PR danny-avila#12737, commit 68c31b6). * 🔐 fix: Redact Metadata in debugTraverse for Warn and Error Relaxing the debug-only gate in debugTraverse (in commit 59371be) routed warn/error records through the traversal path, which emits leaf string values verbatim (via truncateLongStrings only). Because DEBUG_LOGGING defaults to true, those records are also written to the rotating debug log file — which means payloads like `{ auth: 'Bearer ...' }` or `{ openaiKey: 'sk-...' }` were persisted unredacted once my earlier change took effect. Apply redactMessage to the final formatted string when the level is warn or error. Debug-level behavior is unchanged (matching prior art). Includes regression tests covering error/warn redaction and debug-level preservation. Reviewed-by: Codex (P1 finding on PR danny-avila#12737, commit e288f7f). * 🔐 fix: Anchor Secret Regexes at Word Boundaries to Prevent Over-Redaction Removing the `^` anchor in commit e288f7f let the OpenAI-key regex match anywhere in the line — including inside ordinary words like `task-runner` or `mask-value`, where `sk-` appears mid-word. Non-secret text was being rewritten to `task-[REDACTED]`, hiding real log content from operators. - Anchor every sensitive-key pattern with `\b` so matches only fire at word boundaries. - Constrain the OpenAI-key body to the documented charset (`[a-zA-Z0-9_-]+`) instead of the broader "not whitespace or quote" character class. - Add `&` to the `key=` exclusion so a query-string value stops at the next parameter separator. - Regression tests covering both the over-redaction cases (`task-runner`, `monkey=10`) and the intended redactions still firing. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit c09d293). * 🔐 fix: Redact Before Colorize To Survive ANSI Word-Boundary Interference The console pipeline runs `redactFormat → colorize({ all: true }) → printf`. With `all: true`, winston wraps `info.message` in ANSI escapes whose trailing `m` is a word character. That means `\b(Bearer )…` placed at the start of a colorized segment can fall on a (word,word) boundary and miss — the earlier line-wise `redactMessage(line)` pass in printf suffers the same issue because it runs after colorize. Extend `redactFormat` to run for `warn` in addition to `error`, operating on the raw pre-colorize `info.message` + `Symbol.for('message')` strings. The later in-printf `redactMessage(line)` stays as a backstop, but the primary redaction now happens where the regex can actually see the text. Metadata redaction already operates on the raw info object via `formatConsoleMeta`, so it was never affected by ANSI — no change there. Includes regression tests for the new warn-level behavior and for the info/debug no-op path. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit fdb6b36). * 🧹 fix: Prefer Structured Metadata Over Consumed Splat Args in Traversal `debugTraverse` previously read `metadata[Symbol.for('splat')][0]` first and only fell back to the structured metadata object. When a caller uses printf interpolation alongside a metadata object — for example `logger.warn('failed for %s', tenant, { provider })` — winston leaves the *consumed* positional arg (`tenant`) in `SPLAT[0]` after interpolation. The formatter would then append the tenant a second time and skip the real metadata, regressing debug-file and `DEBUG_CONSOLE` output quality now that warn/error share this path. Prefer the structured metadata object (via `extractMetaObject`) and only fall back to `SPLAT[0]` when there's nothing else, so the surviving log line surfaces the actual key/value pairs regardless of call shape. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit 1e43d63). * 🧹 fix: Skip Consumed Splat Primitives in Warn/Error Debug Traversal When no structured metadata is attached, winston still leaves consumed `%s` / `%d` arguments in `Symbol.for('splat')`. Previous fix preferred the structured object but still fell back to whatever sat at `SPLAT[0]` — so `logger.warn('failed for %s', tenantId)` emitted `failed for tenant-7 tenant-7` in the traversal path (debug file and `DEBUG_CONSOLE`), now regressed outside of `debug` level because warn/error share the path. Only accept the splat fallback when the value is a plain object or an array (structural data worth surfacing). Primitives there are almost certainly consumed printf args and get skipped. Regression tests cover the single-%s case and the array-as-metadata case (which still surfaces through splat). Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit bccbf11). * 🧹 fix: Skip Numeric Splat Keys When Extracting Log Metadata When a caller passes a primitive as the second argument — e.g. `logger.warn('Unhandled step creation type:', step.type)` — winston / `format.splat()` can leave character-index keys (`"0"`, `"1"`, …) on the `info` object. With the warn/error metadata trailer in play, those synthetic artifacts were being surfaced as bogus metadata, producing noisy console and debug-file output. Filter out numeric-string keys in `extractMetaObject` so only real metadata fields reach the trailer. Added a regression test. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit b34628d). * 🧹 fix: Preserve Unconsumed Primitive Splat Args in Debug Traversal The previous round dropped every primitive SPLAT[0] value to avoid duplicating consumed %s args, but that removed useful context from calls like \`logger.debug('prefix:', detail)\` where the primitive was never interpolated — users lost the \`detail\` value. Refine the heuristic: skip a primitive splat value only when it already appears inside the (post-interpolation) \`info.message\`; otherwise surface it. Arrays and objects continue to surface unconditionally. Regression test covers the 'prefix:', detail case. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit 6bf9548). * 🧹 fix: Traverse Filtered Metadata, Not Raw Metadata, In debugTraverse `debugTraverse` computes `extracted = extractMetaObject(metadata)` to strip reserved keys, underscore-prefixed internals, and numeric splat artifacts — but the later \`klona(metadata)\` + \`traverse\` path still read the raw object, putting all the filtered junk back into the rendered multi-line output. Clone and traverse \`debugValue\` (the already-filtered object) instead. Regression test exercises the case where numeric splat artifacts sit alongside a real metadata field. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit c29c18e). * 🧹 refactor: Split Warn/Error From Debug-Level Traversal in Parsers Retrofitting `debugTraverse`'s multi-line object walker to cover warn/error created a minefield of splat-interaction edge cases (numeric artifacts, consumed %s args, bogus `_`-prefix filtering, over-eager suppression of unconsumed primitives). Each fix kept introducing new corner cases. Split the two concerns instead: - Warn/error now emit a compact single-line JSON metadata trailer via `formatConsoleMeta`, then pass the full line through `redactMessage`. This mirrors what the console formatter already does, so behavior between the console and debug-file outputs stays consistent for warn/error — and none of the splat/traversal edge cases apply. - Debug level keeps its original code path verbatim (including the raw `metadata` traversal and SPLAT\[0\] fallback). No regressions from my earlier iterations. - `extractMetaObject` no longer filters underscore-prefixed keys, so legitimate fields like MongoDB `_id` still appear. Reserved winston keys and numeric splat artifacts remain filtered. Updated tests reflect the simpler contract (underscore preservation, single-line trailer expectations already covered). Reviewed-by: Codex (two P2 findings on PR danny-avila#12737, commit 9ea1152: `_id` regression and over-eager primitive suppression). * 🧹 fix: Preserve Scalar Metadata When One Value Is Circular `formatConsoleMeta` previously wrapped a single `JSON.stringify` in try/catch — any circular reference inside any field (e.g. an attached request/response object) caused the entire trailer to be dropped. That defeats the goal of making failures diagnosable: one malformed field would mask the provider/model/status we wanted to surface. Use a `WeakSet`-based replacer that emits `[Circular]` for repeated object visits. On the whole-object serialization failing, fall back to per-field serialization so scalar keys always land and only the offending field is replaced with \`"[Unserializable]"\`. Reviewed-by: Codex (P2 finding on PR danny-avila#12737, commit d63742a). * 🧹 fix: Address Audit Findings (JSDoc, Case-Insensitive Api-Key, Tests) Audit review identified several MINOR/NIT items on top of the codex rounds. This commit closes the actionable ones: - **JSDoc (#1, #2)**: `extractMetaObject` no longer claims to filter underscore-prefixed keys (that filter was removed intentionally for MongoDB `_id`). `debugTraverse`'s docblock now describes the three code paths (warn/error compact trailer, debug multi-line traversal, other levels). - **Case-insensitive api-key regex (danny-avila#6)**: `/gi` so the Azure style `Api-Key:` / `API-KEY:` also gets redacted. Pre-existing behavior was lowercase-only. - **Consolidated redundant branch (danny-avila#5)**: `consoleFormat` printf was checking `isError || isWarn` twice; merged into one block. - **Pre-compiled regex (danny-avila#9)**: `NUMERIC_KEY_RE` moved to module scope. - **Test coverage (#3, #4)**: Added regression tests for - per-field serialization fallback when a value's `toJSON` throws, - sensitive strings nested inside metadata objects, - the Azure-style `Api-Key:` header.

…#12781) * 🧹 fix: Prune Orphaned File References on File Deletion Deleting a file via the Manage Files tab left its file_id in every agent's tool_resources.*.file_ids. Stubs accumulate until the frontend dedupe keys them as duplicates and blocks all new uploads (issue danny-avila#12776). - Add removeAgentResourceFilesFromAllAgents in packages/data-schemas: a single updateMany/$pullAll across every EToolResources category. - Invoke it from processDeleteRequest after db.deleteFiles so every referencing agent is cleaned up, not just the one passed in req.body. - Wrap the cleanup in try/catch so a stale agent update cannot mask a successful file deletion. * 🧼 fix: Prune Orphaned File References on Agent Update Already-affected agents would stay broken even after the delete-time fix: the stubs sit on the agent document until something strips them. Heal them on the next save (issue danny-avila#12776). - Add collectToolResourceFileIds + stripFileIdsFromToolResources helpers in @librechat/api — centralizing the tool_resources traversal used by the controller and the follow-up migration script. - In updateAgentHandler, check the effective tool_resources against the files collection. When orphans are found, either strip them from the incoming tool_resources (if the update sets them) or run the bulk cleanup (if the update leaves tool_resources untouched). * 🧰 chore: Add Migration to Clean Up Orphaned Agent File References Complements the delete-time and save-time fixes by healing agents that already accumulated orphan stubs before the upgrade (issue danny-avila#12776). The script is idempotent — re-running it on a clean database is a no-op. - Add config/migrate-orphaned-agent-files.js following the existing migrate-*.js convention: --dry-run by default omitted (writes by default) and --batch-size= tuning knob. Streams agents via cursor. - Register migrate:orphaned-agent-files and :dry-run npm scripts. - Reuse collectToolResourceFileIds from @librechat/api so migration and runtime share the same traversal logic. * 🩹 fix: Address Codex/Copilot Review on Orphaned Agent File Cleanup Refines the danny-avila#12776 fix series based on automated review feedback. - Scope save-time pruning to the current agent only. When a PATCH carries tool_resources, strip orphans from the incoming payload and pay the DB round-trip only then. Removes the collection-wide updateMany previously triggered when tool_resources was absent (Codex P2 / Copilot). - Wrap the orphan check in try/catch so a transient db.getFiles failure can't turn a good save into a 500 (comprehensive review #1). - Replace Object.values(EToolResources) casts with an explicit list of agent-side categories in both orphans.ts and agent.ts. code_interpreter belongs to the Assistants API and isn't a key of AgentToolResources — including it was a type lie and generated dead MongoDB clauses (comprehensive review #3, danny-avila#8). - Export TOOL_RESOURCE_KEYS from @librechat/api and consume it in the migration script, dropping one duplicated definition (#4). - Cap migration results.details at 50 sample entries so the memory footprint stays bounded on deployments with thousands of corrupted agents (Codex P3). - Add migrate:orphaned-agent-files:batch npm script to match the convention set by migrate-agent-permissions / migrate-prompt-permissions (danny-avila#7). - Add controller-level tests covering the three orphan-pruning paths: strip from incoming tool_resources, leave alone when tool_resources is absent, swallow db.getFiles errors and still save (danny-avila#6). - Back pre-existing "should validate tool_resources in updates" test's file_ids with real File docs — the new pruning would otherwise strip them, and that test is about OCR conversion / schema filtering, not file existence. Register the File model in beforeAll so the fixture works. * 🩹 fix: Tighten TOOL_RESOURCE_KEYS Type and Align Migration Sample Output Two follow-ups from the second review pass. - Type data-schemas' TOOL_RESOURCE_KEYS as ReadonlyArray<keyof AgentToolResources> instead of readonly string[]. Data-schemas depends on data-provider, so the import is clean. Catches typos and aligns with the matching export in @librechat/api — doesn't guarantee exhaustiveness, but that's a TypeScript limitation, not a workspace one. - Align the migration's console output with DETAIL_SAMPLE_LIMIT: print every collected detail (up to 50) and, when more agents were affected than the sample size allowed, show a truncation notice. The old hard cap of 25 meant affected agents in the 26-50 range were collected but never shown. * ✅ test: Add Integration Coverage for Orphan Cleanup Paths (danny-avila#12776) Exercise the delete-time and migration paths end-to-end against a real in-memory Mongo. Catches integration bugs the isolated unit tests on each layer couldn't. - api/server/services/Files/process.integration.spec.js — the primary repro: seed an Agent + File, call processDeleteRequest, assert the file_id disappears from every referencing agent's tool_resources while unrelated agents stay untouched. Also covers the no-op case and confirms a failure in the new cleanup step cannot roll back the file deletion itself. - api/test/migrate-orphaned-agent-files.spec.js — drives the migration module: --dry-run reports without writing, apply mode prunes across every tool_resource category, re-running is idempotent, and DETAIL_SAMPLE_LIMIT caps the in-memory sample on wide corruption. Mocks only the connect helper (the spec owns the mongoose instance) so the real migration code path — cursor, $pullAll, reduce — runs. * 🔒 fix: Run Orphan Cleanup Migration in System Tenant Context Codex P2 catch: under TENANT_ISOLATION_STRICT=true, the migration throws on the very first Agent.countDocuments() because the tenant isolation plugin fail-closes on queries without tenant context — which makes migrate:orphaned-agent-files unusable on the exact deployments most likely to have accumulated corruption. - Wrap the scan/prune body in runAsSystem so queries bypass the tenant filter (SYSTEM_TENANT_ID sentinel). The migration legitimately needs cross-tenant visibility — this is the same pattern seedDatabase and the S3 refresh job already use. - Add a regression test that spies on Agent.countDocuments() and asserts the active tenantStorage context is SYSTEM_TENANT_ID during the call. Pins the wrap against future regressions without the brittleness of toggling the strict-mode env var (which caches on first read). Note: the delete-time and save-time paths already run inside an authenticated HTTP request where tenantStorage.run is set by auth middleware, so the cleanup naturally scopes to the current tenant — which is the correct behavior there since file ownership is tenant-scoped. * 🧹 chore: Drop Unused path Import From Process Integration Spec Leftover from an earlier iteration that resolved the migration path via path.resolve before I switched to a relative require. The import does nothing now — removing it.

…riming (danny-avila#12649) * feat: Skill runtime integration — catalog injection, tool registration, execute handler Wires the @librechat/agents SkillTool primitive into LibreChat's agent runtime: **Enums:** - Add `skills` to AgentCapabilities + defaultAgentCapabilities **Data layer:** - Add `getSkillByName(name, accessibleIds)` — compound query that combines name lookup + ACL check in one findOne **Agent initialization (packages/api/src/agents/initialize.ts):** - Accept `accessibleSkillIds` param and `listSkillsByAccess` db method - Query accessible skills, format catalog via `formatSkillCatalog()`, append to `additional_instructions` (appears in agent system prompt) - Register `SkillToolDefinition` + `createSkillTool()` when catalog is non-empty (tool appears in model's tool list) - Store `accessibleSkillIds` and `skillCount` on InitializedAgent **Execute handler (packages/api/src/agents/handlers.ts):** - Add `getSkillByName` to `ToolExecuteOptions` - `handleSkillToolCall()` intercepts `Constants.SKILL_TOOL`: extracts skillName, loads body from DB with ACL check, substitutes $ARGUMENTS, returns ToolExecuteResult with injectedMessages (skill body as isMeta user message) **Caller wiring:** - initialize.js: query skill IDs via findAccessibleResources, pass to initializeAgent + store on agentToolContexts, add getSkillByName to toolExecuteOptions, pass accessibleSkillIds through loadTools configurable - openai.js + responses.js: same pattern for their flows Requires @librechat/agents >= 3.1.65 (PR danny-avila#91 exports). * feat: Skills toggle in tools menu + backend capability gating Frontend: - Add skills?: boolean to TEphemeralAgent type - Add LAST_SKILLS_TOGGLE_ to LocalStorageKeys for persistence - Add skillsEnabled to useAgentCapabilities hook - Add skills useToolToggle to BadgeRowContext with localStorage init - New Skills.tsx badge component (Scroll icon, cyan theme, permission-gated via PermissionTypes.SKILLS) - Add skills entry to ToolsDropdown with toggle + pin - Render Skills badge in BadgeRow ephemeral section Backend: - Extract injectSkillCatalog() into packages/api/src/agents/skills.ts (reduces initializeAgent module size, reusable helper) - initializeAgent delegates to helper instead of inline block - Capability-gate the findAccessibleResources query: - Agents endpoint: checks AgentCapabilities.skills in admin config - OpenAI/Responses controllers: checks ephemeralAgent.skills toggle - ACL query runs once per run, result shared across all agents * refactor: remove createSkillTool() instance from injectSkillCatalog SkillTool is event-driven only. The tool definition in toolDefinitions is sufficient for the LLM to see the tool schema. No tool instance is needed since the host handler intercepts via ON_TOOL_EXECUTE before tool.invoke() is ever called. Removes tools from InjectSkillCatalogParams/Result, drops the createSkillTool import. * feat: skill file priming, bash tool, and invoked skills state Multi-file skill support: - New primeSkillFiles() helper (packages/api/src/agents/skillFiles.ts) uploads skill files + SKILL.md body to code execution environment - handleSkillToolCall primes files on invocation when skill.fileCount > 0, returns session info as artifact so ToolNode stores the session - Skill-primed files available to subsequent bash/code tool calls Bash tool auto-registration: - BashExecutionToolDefinition added alongside SkillToolDefinition when skills are enabled, giving the model a bash tool for running scripts Conversation state: - Add invokedSkillIds field to conversation schema (Mongoose + Zod) - handleSkillToolCall updates conversation with $addToSet on success - Enables re-priming skill files on subsequent runs (future) Dependency wiring: - Pass listSkillFiles, getStrategyFunctions, uploadCodeEnvFile, updateConversation through ToolExecuteOptions - Pass req and codeApiKey through mergedConfigurable - All three controller entry points wired (initialize.js, openai.js, responses.js) * fix: load bash_tool instance in loadToolsForExecution, remove file listing - Add createBashExecutionTool to loadToolsForExecution alongside PTC/ToolSearch pattern: loads CODE_API_KEY, creates bash tool instance on demand - Add BASH_TOOL and SKILL_TOOL to specialToolNames set so they don't go through the generic loadTools path (bash is created here, skill is intercepted in handler before tool.invoke) - Remove file name listing from skill content text — it's the skill author's responsibility to disclose files in SKILL.md, not the framework * feat: batch upload for skill files, replace sequential uploads - Add batchUploadCodeEnvFiles() to crud.js: single POST to /upload/batch with all files in one multipart request, returns shared session_id - Rewrite primeSkillFiles to collect all streams (SKILL.md + bundled files) then do one batch upload instead of N sequential uploads - Replace uploadCodeEnvFile with batchUploadCodeEnvFiles across all callers (handlers.ts, initialize.js, openai.js, responses.js) * refactor: remove invokedSkillIds from conversation schema Skills aren't re-loaded between runs, so conversation-level state for invoked skills doesn't help. Skill state will live on messages instead (like tool_search discoveredTools and summaries), enabling in-place re-injection on follow-up runs. Removes invokedSkillIds from: convo Mongoose schema, IConversation interface, Zod schema, ToolExecuteOptions.updateConversation, and all three caller wiring points. * feat: smart skill file re-priming with session freshness checking Schema: - Add codeEnvIdentifier field to ISkillFile (type + Mongoose schema) - Add updateSkillFileCodeEnvIds batch method (uses tenantSafeBulkWrite) - Export checkIfActive from Code/process.js Extraction: - Add extractInvokedSkillsFromHistory() to run.ts — scans message history for AIMessage tool_calls where name === 'skill', extracts skillName args. Follows same pattern as extractDiscoveredToolsFromHistory. Smart re-priming in primeSkillFiles: - Before batch uploading, checks if existing codeEnvIdentifiers are still active via getSessionInfo + checkIfActive (23h threshold) - If session is still active, returns cached references (zero uploads) - If stale or missing, batch-uploads everything and persists new identifiers on SkillFile documents (fire-and-forget) - Single session check covers all files (batch shares one session_id) Wiring: - Pass getSessionInfo, checkIfActive, updateSkillFileCodeEnvIds through ToolExecuteOptions and all three controller entry points * feat: wire skill file re-priming at run start via initialSessions Flow: 1. initialize.js creates primeInvokedSkills callback with all deps 2. client.js calls it with message history before createRun 3. extractInvokedSkillsFromHistory scans for skill tool calls 4. For each invoked skill with files, primeSkillFiles uploads/checks 5. Returns initialSessions map passed to createRun 6. createRun passes initialSessions to Run.create (via RunConfig) 7. Run constructor seeds Graph.sessions, making skill files available to subsequent bash/code tool calls via ToolNode session injection Requires @librechat/agents with initialSessions on RunConfig (PR danny-avila#94). * refactor: use CODE_EXECUTION_TOOLS set for code tool checks Import CODE_EXECUTION_TOOLS from @librechat/agents and replace inline constant checks in handlers.ts and callbacks.js. Fixes missing bash tool coverage in the session context injection (handlers.ts) and code output processing (callbacks.js). * refactor: move primeInvokedSkills to packages/api, add skill body re-injection Moves primeInvokedSkills from an inline closure in initialize.js (with dynamic requires) to a proper exported function in packages/api skillFiles.ts with explicit typed dependencies. Key changes: - primeInvokedSkills now returns both initialSessions (for file priming) AND injectedMessages (skill bodies for context continuity) - createRun accepts invokedSkillMessages and appends skill bodies to systemContent so the model retains skill instructions across runs - initialize.js calls the packaged function with all deps passed explicitly - client.js passes both initialSessions and injectedMessages to createRun * fix: move dynamic requires to top-level module imports Move primeInvokedSkills, getStrategyFunctions, batchUploadCodeEnvFiles, getSessionInfo, and checkIfActive from inline requires to top-level module requires where they belong. * refactor: skill body reconstruction via formatAgentMessages, not systemContent Replaces the lazy systemContent approach with proper message-level reconstruction: SDK (formatAgentMessages): - New invokedSkillBodies param (Map<string, string>) - Reconstructs HumanMessages after skill ToolMessages at the correct position in the message sequence, matching where ToolNode originally injected them LibreChat: - extractInvokedSkillsFromPayload replaces extractInvokedSkillsFromHistory (works with raw TPayload before formatAgentMessages, not BaseMessage[]) - primeInvokedSkills now takes payload instead of messages, returns skillBodies Map instead of injectedMessages - client.js calls primeInvokedSkills BEFORE formatAgentMessages, passes skillBodies through as the 4th param - Removed invokedSkillMessages from createRun (no more systemContent hack) - Single-pass: skill detection happens inside formatAgentMessages' existing tool_call processing loop, zero extra message iterations * refactor: rename skillBodies to skills for consistency with SDK param * refactor: move auth loading into primeInvokedSkills, pass loadAuthValues as dep The payload/accessibleSkillIds guard and CODE_API_KEY loading now live inside primeInvokedSkills (packages/api) rather than in the CJS caller. initialize.js passes loadAuthValues as a dependency and the callback is only created when skillsCapabilityEnabled. * feat: ReadFile tool + conditional bash registration + skill path namespacing ReadFile tool (read_file): - General-purpose file reader, event-driven (ON_TOOL_EXECUTE) - Schema: { file_path: string } — "{skillName}/{path}" convention - handleReadFileCall: resolves skill name from path, ACL check, reads from DB cache or storage, binary detection, size limits (256KB), lazy caching (512KB), line numbers in output - SKILL.md special case: reads skill.body directly - Dispatched alongside SKILL_TOOL in createToolExecuteHandler - Added to specialToolNames in ToolService Conditional tool registration: - ReadFile + SkillTool: always registered when skills enabled - BashTool: only registered when codeEnvAvailable === true - codeEnvAvailable passed through InitializeAgentParams from caller Skill file path namespacing: - primeSkillFiles now uploads as "{skillName}/SKILL.md" and "{skillName}/{relativePath}" instead of flat names - Prevents file collisions when multiple skills are invoked Wiring: - getSkillFileByPath + updateSkillFileContent passed through ToolExecuteOptions in all three callers * feat: return images/PDFs as artifacts from read_file, tighten caching Binary artifact support: - Images (png, jpeg, gif, webp) returned as base64 in artifact.content with type: 'image_url', processed by existing callback attachment flow - PDFs returned as base64 artifact similarly - Binary size limit: 10MB (MAX_BINARY_BYTES) - Other binary files still return metadata + bash fallback Caching: - Text cached only on first read (file.content == null check) - Binary flag cached only on first detection (file.isBinary == null) - Skill files are immutable; no redundant cache writes Registration: - ReadFileToolDefinition now includes responseFormat: 'content_and_artifact' * chore: update @librechat/agents to version 3.1.66-dev.0 and add peer dependencies in package-lock.json and package.json files * fix: resolve review findings #1,#2,#4,danny-avila#5,danny-avila#6,danny-avila#10,danny-avila#13 Critical: - #1: primeInvokedSkills now accumulates files across all skills into one session entry instead of overwriting. Parallel processing via Promise.allSettled. - #2: codeEnvAvailable now computed and passed in openai.js and responses.js (was missing, bash tool never registered in those flows) Major: - #4: relativePath in updateSkillFileCodeEnvIds now strips the {skillName}/ prefix to match SkillFile documents. SKILL.md filter uses endsWith instead of exact match. - danny-avila#5: File priming guarded on apiKey being non-empty (skip when not configured instead of failing with auth error) - danny-avila#6: Skills processed in parallel via Promise.allSettled instead of sequential for-of loop Minor: - danny-avila#10: Use top-level imports in initialize.js instead of inline requires - danny-avila#13: Log warning when skill catalog reaches the 100-skill limit * fix: resolve followup review findings N1,N2,N4 N1 (CRITICAL): Wire skill deps into responses.js non-streaming path. Was completely missing getSkillByName, file strategy functions, etc. N2 (MAJOR): Single batch upload for ALL skills' files. Resolves skills in parallel (Phase 1), then collects all file streams across skills and does ONE batchUploadCodeEnvFiles call (Phase 2). All files share one session_id, eliminating cross-session isolation issues. N4 (MINOR): Move inline require() to top-level in openai.js and responses.js, consistent with initialize.js. * fix: add mocks for new file strategy imports in controller tests * fix: restore session freshness check, parallelize file lookups, add warnings R1: Re-add session freshness check before batch upload. Checks any existing codeEnvIdentifier via getSessionInfo + checkIfActive. If the session is still active (23h window), returns cached file references with zero re-uploads. R2: listSkillFiles calls parallelized via Promise.all (were sequential in the for-of loop). R3: Log warning when skill record lookup fails during identifier persistence (was a silent empty-string fallback). * fix: guard freshness cache on single-session consistency * fix: multi-session freshness check (code env handles mixed sessions natively) The code execution environment fetches each file by its own {session_id, fileId} pair independently — no single-session requirement. Removed the sessionIds.size === 1 guard. Now checks ALL distinct sessions for freshness. If every session is still active (23h window), returns cached references with per-file session_ids preserved. If any session expired, falls through to re-upload everything in a single batch. * perf: parallelize session freshness checks via Promise.all * fix: add optional chaining for session info retrieval in primeInvokedSkills Updated the primeInvokedSkills function to use optional chaining for getSessionInfo and checkIfActive methods, ensuring safer access and preventing potential runtime errors when these methods are undefined. * fix: address review findings #1-danny-avila#9 + Codex P1/P2 + session probe Critical: - #1/Codex P1: Add codeApiKey loading to openai.js and responses.js loadTools configurable (was missing, file priming broken in 2/3 paths) - Codex P1: Fix cached file name prefix in primeSkillFiles cache path (was sf.relativePath, now ${skill.name}/${sf.relativePath}) Major: - Codex P2: Honor ephemeral skills toggle in agents endpoint (check ephemeralAgent?.skills !== false alongside admin capability) - #4: Early size check using file.bytes from DB before streaming (prevents full-file buffer for oversized files) Minor: - danny-avila#5: Replace Record<string, any> with Record<string, boolean | string> - danny-avila#6: Localize Pin/Unpin aria-labels with com_ui_pin/com_ui_unpin - danny-avila#8: Parallelize stream acquisition in primeSkillFiles via Promise.allSettled - danny-avila#9: Log warning for partial batch upload failures with filenames Performance: - Session probe optimization: getSessionInfo now hits per-object endpoint (GET /sessions/{sid}/objects/{fid}) instead of listing entire session (GET /files/{sid}?detail=summary). O(1) stat vs O(N) list + linear scan. * refactor: extract shared skill wiring helper + add unit tests DRY (#3): - New skillDeps.js exports getSkillToolDeps() with all 9 skill-related deps (getSkillByName, listSkillFiles, getStrategyFunctions, etc.) - Replaces 5 identical copy-paste blocks across initialize.js, openai.js, responses.js (streaming + non-streaming paths) - One place to maintain when skill deps change Tests (#2): - 8 unit tests for extractInvokedSkillsFromPayload covering: string args, object args, missing skill tool_calls, non-assistant messages, malformed JSON, empty skillName, empty payload, dedup * fix: remove @jest/globals import, use global jest env * fix: resolve round 2 review findings R2-1 through R2-7 R2-1 (toggle semantics): openai.js + responses.js now check admin capability (AgentCapabilities.skills) alongside ephemeral toggle. Aligns with initialize.js. R2-2 (swallowed error): primeInvokedSkills now logs updateSkillFileCodeEnvIds failures (was .catch(() => {})) R2-4 (test cast): Record<string, string> → Record<string, unknown> R2-5 (DRY regression): Extract enrichWithSkillConfigurable() into skillDeps.js. Replaces 4 identical loadAuthValues blocks. Each loadTools callback is now a one-liner. JSDoc added (R2-6). R2-7 (sequential streams): primeInvokedSkills now uses Promise.allSettled for parallel stream acquisition. * fix: require explicit skills toggle + treat partial cache as miss - initialize.js: change ephemeralSkillsToggle !== false to === true (unset toggle no longer enables skills) - primeSkillFiles cache: require ALL files to have codeEnvIdentifier before using cache (partial persistence = cache miss = re-upload) - primeInvokedSkills cache: same check (allFilesWithIds.length must equal total file count) * fix: pass entity_id=skillId on batch upload, eliminates per-user cache thrashing primeSkillFiles now passes entity_id: skill._id.toString() to batchUploadCodeEnvFiles. This scopes the code env session to the skill, not the user. All users sharing a skill share the same uploaded files — no more cache thrashing from overwriting each other's codeEnvIdentifier. The stored codeEnvIdentifier now includes ?entity_id= suffix so freshness checks pass the entity_id through to the per-object stat endpoint. Both primeSkillFiles and primeInvokedSkills store consistent identifier formats. * fix: pass entity_id on multi-skill batch upload, consistent identifier format * Revert "fix: pass entity_id on multi-skill batch upload, consistent identifier format" This reverts commit c85ce21. * refactor: per-skill upload in primeInvokedSkills, eliminate multi-skill batch Replace the monolithic multi-skill batch upload with per-skill primeSkillFiles calls. Each skill gets its own session with entity_id=skillId, ensuring: - Correct session auth (entity_id matches on freshness checks) - Per-skill freshness caching (only expired skills re-upload) - Shared skill sessions work across users (same entity_id=skillId) - Code env handles mixed session_ids natively The big batch block (stream collection, single upload, identifier mapping) is replaced by a simple loop over primeSkillFiles, which already handles freshness caching, batch upload, and identifier persistence per-skill. * fix: resolve review findings #1,#3-5,danny-avila#7,danny-avila#9-11 Critical: - #1: Strip ?entity_id= query string before splitting codeEnvIdentifier into session_id/fileId (was corrupting cached file IDs in 4 locations) Major: - #4: Parallelize per-skill primeSkillFiles via Promise.allSettled - danny-avila#5: Add logger.warn to all empty .catch(() => {}) on cache writes Minor: - danny-avila#7: Add logger.debug to enrichWithSkillConfigurable catch block - danny-avila#9: Use error instanceof Error guard in batchUploadCodeEnvFiles - danny-avila#10: Move enrichWithSkillConfigurable to TypeScript in packages/api (skillConfigurable.ts), skillDeps.js wraps with loadAuthValues dep - danny-avila#11: Reduce MAX_BINARY_BYTES from 10MB to 5MB (~11.5MB peak with b64) * fix: forward entity_id in session probe + always register bash tool Codex P2 (entity_id in probe): getSessionInfo now preserves and forwards query params (including entity_id) to the per-object stat endpoint. Without this, identifiers stored as ...?entity_id=... would fail auth checks because the entity_id scope was dropped. Codex P2 (bash tool availability): Remove codeEnvAvailable gate from injectSkillCatalog. Bash tool definition is now always registered when skills are enabled. Actual tool instance creation still happens at execution time in loadToolsForExecution (which loads per-user credentials). This ensures users with per-user CODE_API_KEY get bash without requiring a global env var at init time. Removes codeEnvAvailable from InjectSkillCatalogParams, InitializeAgentParams, and all three controller entry points. * fix: add debug logging to primeInvokedSkills catch, rename export alias * fix: stub bash tool when no key + remove PDF artifact path Codex P1 (bash tool): When CODE_API_KEY is unavailable, create a stub tool that returns "Code execution is not available. Use read_file instead." This prevents "tool not found" errors from the model repeatedly calling bash_tool in no-code-env deployments while still registering the definition for per-user credential users. Codex P2 (PDF artifacts): Remove PDF image_url artifact path. The host artifact pipeline processes image_url via saveBase64Image which fails for PDFs. PDFs now fall through to the generic binary handler ("Use bash to process"). TODO comment for future document artifact support. Also: isImageOrPdf → isImage in early size checks (PDFs are no longer treated as artifact candidates). * fix: remove dead PDF_MIME constant, hoist skillToolDeps, document session_id - danny-avila#7: Remove unused PDF_MIME constant (dead code after PDF artifact removal) - danny-avila#11: Hoist skillToolDeps to module-level constant (avoid per-call allocation) - danny-avila#6: Document that CodeSessionContext.session_id is a representative value; ToolNode uses per-file session_id from the files array * fix: call toolEndCallback for skill/read_file artifacts + clear codeEnvIdentifier on re-upload Codex P1 (toolEndCallback bypass): skill and read_file handler branches returned early, bypassing the toolEndCallback that processes artifacts (image attachments). Now calls toolEndCallback when the result has an artifact, using the same metadata pattern as the normal tool.invoke path. Codex P1 (stale identifiers): upsertSkillFile now $unset's codeEnvIdentifier alongside content and isBinary when a file is re-uploaded. Prevents the freshness cache from returning references to old file content after a skill file is replaced. * fix: add session_id comment at cached path, rename skillResult to handlerResult * fix: return content_and_artifact from bash stub so result.content is populated * fix: deterministic skill lookup, dedup warning, and multi-session freshness check - getSkillByName: add sort({updatedAt:-1}) so name collisions resolve deterministically to the most recently updated skill - injectSkillCatalog: warn when multiple accessible skills share a name - primeSkillFiles: check ALL distinct sessions for freshness, not just the first file's session, preventing stale refs after partial bulkWrite * refactor: update icon import in Skills component - Replaced the Scroll icon with ScrollText in the Skills component for improved clarity and consistency in the UI. * fix: SKILL.md cache parity, gate bash_tool on code env, fix read_file too-large message - primeSkillFiles: filter SKILL.md from returned files array on fresh upload so cached and non-cached paths return identical file sets (SKILL.md is still on disk in the session for bash access) - injectSkillCatalog: only register bash_tool when codeEnvAvailable is true; thread the flag from all three CJS callers via execute_code capability check - handleReadFileCall: tell the model to invoke the skill first before suggesting /mnt/data paths for oversized files * fix: use EnvVar constant, deduplicate auth lookup, validate batch upload, stream byte limit - Replace hardcoded 'LIBRECHAT_CODE_API_KEY' with EnvVar.CODE_API_KEY in skillConfigurable.ts and skillFiles.ts - Resolve code API key once at run start in initialize.js and pass to both primeInvokedSkills and enrichWithSkillConfigurable via optional preResolvedCodeApiKey param, eliminating redundant loadAuthValues calls - Add response structure validation in batchUploadCodeEnvFiles before accessing session_id/files to surface unexpected responses early - Add streaming byte counter in handleReadFileCall that aborts and destroys the stream when accumulated bytes exceed MAX_BINARY_BYTES, preventing full file buffering when DB metadata is inaccurate * refactor: update icon import in ToolsDropdown component - Replaced the Scroll icon with ScrollText in the ToolsDropdown component for improved clarity and consistency in the UI. * fix: partial upload failure detection, EnvVar in initialize.js, declaration ordering - primeSkillFiles: return null (failure) when batch upload partially succeeds — missing bundled files would cause runtime bash/read failures with missing paths in code env - initialize.js: replace hardcoded 'LIBRECHAT_CODE_API_KEY' with EnvVar.CODE_API_KEY imported from @librechat/agents - initialize.js: move enabledCapabilities, accessibleSkillIds, and codeApiKey declarations before the toolExecuteOptions closure that references them (eliminates reliance on temporal dead zone hoisting)

…efaults (danny-avila#12692) * feat: per-user skill active/inactive toggle with ownership-aware defaults - Add `skillStates` map (Record<string, boolean>) to user schema for per-user active/inactive overrides on skills - Add `defaultActiveOnShare` to interface.skills config (default: false) so admins can control whether shared skills auto-activate - Add GET/POST /api/user/settings/skills/active endpoints with validation - Add React Query hooks with optimistic mutations for skill states - Add useSkillActiveState hook with ownership-aware resolution: owned skills default active, shared skills default inactive - Add toggle switch UI to SkillListItem and SkillDetail components - Filter inactive skills in injectSkillCatalog before agent injection - Add localization keys for active/inactive labels * fix: use Record instead of Map for IUser.skillStates Mongoose .lean() flattens Map to a plain object, causing type incompatibility with IUser in methods that return lean documents. * fix: address review findings for skill active states - Fail-closed when userId is absent: filter rejects all shared skills instead of passing them through unfiltered (Codex P1) - Validate Mongoose Map key characters (reject . and $) in controller to return 400 instead of a 500 from schema validation (Codex P2) - Block toggle while initial skill states query is loading to prevent overwriting server-side overrides with an empty snapshot (Codex P2) - Extract shared SkillToggle component, eliminating duplicate toggle markup in SkillListItem and SkillDetail (Finding #3) - Move skill state query/mutation hooks from Favorites.ts to Skills/queries.ts per feature-directory convention (Finding #4) - Fix hardcoded English aria-label in SkillListItem by passing the localized string from the parent SkillList (Finding danny-avila#5) - Fix inline arrow in SkillList render loop: pass stable callback reference so SkillListItem memo() is not invalidated (Finding #1) - Extract toRecord() helper in controller to DRY the Map-to-Object conversion (Finding danny-avila#6) - Remove Promise.resolve wrapping synchronous config read (Finding danny-avila#8) - Remove unused TUpdateSkillStatesRequest type (Finding danny-avila#12) * fix: forward tabIndex on SkillToggle to preserve list keyboard nav The original inline toggle had tabIndex={-1} so the row itself remained the sole tab target. The extraction into SkillToggle dropped this prop, making every list toggle a tab stop. Add an optional tabIndex prop and pass -1 from SkillListItem. * fix: plumb skillStates to all agent entry points, isolate toggle keydown - Add skillStates/defaultActiveOnShare loading to openai.js and responses.js controllers so shared-skill activation is respected across all agent entry points, not just initialize.js (Codex P1) - Stop keydown propagation on SkillToggle so Enter/Space does not bubble to the parent row's navigation handler (Codex P2) * fix: paginate catalog fetch and serialize toggle writes - Paginate listSkillsByAccess (up to 10 pages of 100) until the active catalog quota is filled, so inactive shared skills in recent positions do not starve active owned skills past the first page (Codex P1) - Extend listSkillsByAccess interface with cursor/has_more/after for catalog pagination - Serialize skill-state writes via a ref queue: one in-flight request at a time, with the latest desired state sent when the previous one settles. Prevents last-response-wins races where an older request overwrites newer toggles (Codex P2) * fix: share write queue across hook instances, block toggle on fetch error - Move the write queue from a per-instance useRef to a module-scoped object so every mount of useSkillActiveState (SkillList, SkillDetail, etc.) serializes against the same in-flight slot. Prior per-instance queues allowed two components to race full-map POSTs (Codex P1) - Extend the toggle guard beyond isLoading: also block when isError is true or data is undefined. Prevents a failed GET from seeding a toggle with an empty baseline that would wipe server-side overrides on the next successful POST (Codex P1) * fix: stale closure, orphan cleanup, and cap-error UX - Read toggle baseline from React Query cache via queryClient.getQueryData instead of the captured skillStates closure. The closure can be stale between onMutate's setQueryData and the next render, so rapid successive toggles would build on old state and drop earlier changes (Codex P1) - Surface the MAX_SKILL_STATES_EXCEEDED error code with a specific toast key (com_ui_skill_states_limit) so users understand the 200-cap rather than seeing a generic error - Prune orphaned entries (skillIds whose Skill doc no longer exists) on both GET and POST in SkillStatesController. Self-heals over time without needing cascade-delete hooks or a migration job. Uses one indexed Skill._id query per request * test: pin skill active-state precedence with unit tests Extract the active-state resolution logic from a closure inside injectSkillCatalog into an exported resolveSkillActive helper, then cover every branch of the precedence matrix: - Fails closed when userId is absent (even with defaultActiveOnShare=true) - Explicit override wins over ownership and config (both true and false) - Owned skills default to active when no override is set - Shared skills default to defaultActiveOnShare value - Undefined skillStates behaves identically to an empty object - defaultActiveOnShare defaults to false when omitted - Owned skills ignore defaultActiveOnShare entirely Closes Finding #2 from the pre-rebase comprehensive review. Mirrors the existing scopeSkillIds test style; injectSkillCatalog now calls resolveSkillActive instead of inlining the closure. * refactor: limit skill active toggle to detail header, drop label - Remove the per-row toggle from SkillListItem and the active-state plumbing (hook call, isSkillEnabled/onToggleEnabled/toggleAriaLabel props) from SkillList. The detail view is now the single place to change a skill's active state - Drop dim/muted styling for inactive skills in the sidebar: without a control there, the visual indication has nowhere to land - Resize SkillToggle to match neighbor buttons: outer h-9 container, h-6 w-11 track with size-5 knob, no label span. The 'Active' / 'Inactive' text that accompanied the detail-view toggle is removed - Remove the now-unused label prop and tabIndex prop (the tabIndex existed only for the list-row context) from SkillToggle. Drop the onKeyDown stopPropagation for the same reason - Remove now-orphaned com_ui_skill_active / com_ui_skill_inactive translation keys * style: shrink SkillToggle track to h-5 w-9 with size-4 knob Container stays at h-9 to match neighbor button heights. The toggle track itself drops from h-6 w-11 to h-5 w-9, with a size-4 knob travelling 1.125rem on activation. Visually lighter inside the row. * fix: remove redundant skillStates entries that match the resolved default When a toggle lands on the ownership/config default, delete the key from the map instead of persisting `{id: defaultValue}`. Without this, a user toggling a skill off and back on would leave `{id: true}` for an owned skill (whose default is already true), silently consuming a slot against the 200-entry cap. Repeated round-trip toggles could exhaust the quota with zero meaningful overrides (Codex P2). Preserves the exceptions-list invariant that the runtime-resolution design depends on. * fix: prune before enforcing skill-state cap; reject non-ObjectId keys Reorder the update controller so pruneOrphans runs before the 200-cap check. Without this, a user near the cap with some orphaned entries (skills deleted since their last GET) could send a payload that would pass after pruning but gets rejected by the raw-size check first. Add a sanity cap on raw payload size (2 * MAX_SKILL_STATES) so abusive inputs do not reach the DB query, and enforce the real cap on the pruned result instead. Harden pruneOrphans: the earlier early-return path could pass non-ObjectId keys through unchanged. Now only valid ObjectIds are returned, and the Skill-model-unavailable fallback filters by format. Also add isValidObjectIdString validation at the input boundary so malformed (but otherwise non-Mongo-unsafe) keys never reach persistence (Codex P2 x2). * fix: enforce active filter at execute time, prune revoked shares, scope queue per user P1: injectSkillCatalog now returns activeSkillIds (the filtered set that appears in the catalog). initializeAgent uses that set as the stored accessibleSkillIds on the initialized agent, so getSkillByName at runtime cannot resolve a deactivated skill — even if the LLM hallucinates a name or the user invokes by direct-invocation shorthand. Previously the executor authorized against the full ACL set, bypassing the active-state guarantee (Codex P1). P2: pruneOrphans now checks user access via findAccessibleResources in addition to skill existence. When a share is revoked, the user's skillStates entry for that skill had no cleanup path and silently consumed the 200-cap. Self-heals on both GET and POST. One extra ACL query per settings read/write; scoped to a single user so no N-user amplification (Codex P2). P2: the write queue moves from a single module-scoped object to a Map keyed by userId. Logout/login in the same tab can no longer flush the previous user's pending snapshot under the new session's auth. Each userId gets its own pending/inFlight slot; the in-flight request retains its original auth via the cookie already attached when sent, so the race window closes (Codex P2). * refactor: extract skillStates helpers to packages/api; add tests; polish Address the remaining valid findings from the comprehensive review: - Extract toRecord, loadSkillStates, validateSkillStatesPayload, and pruneOrphanSkillStates into packages/api/src/skills/skillStates.ts as TypeScript. The controller in /api shrinks to a ~90-line thin wrapper that builds live dependency adapters for Mongoose + the permission service (Review #2 DRY, #3 workspace boundary) - Replace the triplicated 12-line skillStates loading block in initialize.js, openai.js, and responses.js with a single call to loadSkillStates from @librechat/api. One helper, three sites - Swap console.error for the project logger in the controller (Review danny-avila#7) - Remove the redundant INVALID_KEY_PATTERN regex: a valid ObjectId cannot contain . or $, so isValidObjectIdString already covers it (Review danny-avila#11) - Parameterize the 200-cap error toast with {{0}} interpolation driven by the error response's `limit` field, so future changes to MAX_SKILL_STATES update the UI message automatically (Review danny-avila#12) - Add 24 unit tests for the new skillStates helpers (toRecord, resolveDefaultActiveOnShare, loadSkillStates, validateSkillStates- Payload, pruneOrphanSkillStates) covering success paths, malformed input, cap boundaries, and parallel-query behavior (Review #4) - Add 10 tests for injectSkillCatalog pagination covering empty accessible set, missing listSkillsByAccess, single-page filter, owned-vs-shared defaults, explicit-override precedence, multi-page collection, MAX_CATALOG_PAGES safety cap, early termination on has_more=false, additional_instructions injection, and fail-closed without userId (Review danny-avila#5) Total test count: 60 (was 26 on this surface). * fix: rename skillStates ValidationError to avoid barrel-export collision packages/api/src/types/error.ts already exports a ValidationError (MongooseError extension). Re-exporting a different shape from skills/skillStates.ts through the skills barrel caused TS2308 in CI because the root index re-exports both. Rename to SkillStatesValidationError to keep the exports disjoint. * refactor: tighten tests and absorb caller guard into loadSkillStates Address the followup review findings: - Add optional `accessibleSkillIds` param to loadSkillStates so the helper short-circuits to defaults when no skills are accessible. All three controllers drop the residual 7-line conditional wrapper in favor of a single destructured call (Review #2) - Remove the unreachable `typeof key !== 'string'` check from validateSkillStatesPayload: Object.entries always yields string keys per the JS spec (Review #3) - Replace the two `as unknown as` agent casts in the injectSkillCatalog tests with a `makeAgent()` factory typed directly as the function's parameter shape (Review #4) - Tighten the MAX_CATALOG_PAGES assertion from `toBeLessThanOrEqual(11)` to `toHaveBeenCalledTimes(10)` — the loop deterministically makes exactly 10 page fetches before hitting the cap (Review #1) - Rewrite the parallel-execution test for pruneOrphanSkillStates using deferred promises instead of microtask-order assertions. The test now inspects `toHaveBeenCalledTimes(1)` on both mocks after a single Promise.resolve() yield, pinning Promise.all usage without relying on push-order into a shared array (Review danny-avila#5) - Evict stale writeQueue entries on user change via a module-scoped `lastSeenUserId` sentinel. When a different user's toggle is the first one after a logout/login, the previous user's queue entry is deleted. Keeps the Map bounded without adding hook-instance effect cleanup (Review danny-avila#6) * fix(test): mock loadSkillStates in openai and responses controller specs The prior refactor replaced the inline 12-line skillStates loading block with a call to loadSkillStates from @librechat/api. Both controller spec files mock @librechat/api as a flat object, so any new named import from that package is undefined in the test env. Calling `await loadSkillStates(...)` threw before recordCollectedUsage ran, surfacing as "undefined is not iterable" on the test's array destructure of `mockRecordCollectedUsage.mock.calls[0]`. Add the missing mock to both spec files alongside the existing scopeSkillIds stub. * fix: abandon stale skillStates write queues on user switch Close the cross-session leak window where an in-flight flush loop still holds a reference to a previous user's queue: it could fire its next mutateAsync under the new session's auth cookies and persist the stale snapshot to the new user's document (Codex P1). Add an `abandoned` flag on `WriteQueue`. Three mechanisms cooperate: - `getWriteQueue` marks every non-active queue abandoned when the user differs from the last-seen identity (pre-existing eviction site, now more aggressive). - A `useEffect` on `userId` calls the same abandonment pass on every render with a new active identity, covering the window between logout/login and the new user's first toggle (when `getWriteQueue` would otherwise not fire). - The flush loop checks `!queue.abandoned` in its while condition so the second and later iterations exit without firing another `mutateAsync` after the session changes. The first iteration's in-flight request (already dispatched under the original user's cookies) still runs to completion or failure on its own — only the subsequent iterations, which are the dangerous ones, are blocked.

@deprecated

…le` / `allowed-tools` (danny-avila#12745) * 🧬 feat: Persist `disable-model-invocation` / `user-invocable` / `allowed-tools` Adds first-class columns mirroring the three runtime-enforced frontmatter fields, with a `deriveStructuredFrontmatterFields` helper that maps from frontmatter at create/update time and re-syncs (via `$unset`) when fields are removed. `listSkillsByAccess` projection includes them so the Phase 6 catalog filter and popover filter can both read off the summary row. Marks `invocationMode` as @deprecated on `TSkill` and the `InvocationMode` enum — the runtime now reads the persisted pair instead. * 🛡️ feat: Enforce frontmatter at runtime (catalog, skill tool, manual resolver, tool union) Wires the persisted columns into actual runtime behavior across all four invocation paths: - `injectSkillCatalog` excludes `disableModelInvocation: true` skills before catalog formatting — they cost zero context tokens and stay invisible to the model. - `handleSkillToolCall` rejects with a clear error when the model names a skill marked `disable-model-invocation: true` (defends against a stale-cache or hallucinated invocation getting past the catalog filter). - `resolveManualSkills` skips `userInvocable: false` skills with a warn log so an API-direct caller can't bypass the popover-side filter. - `unionPrimeAllowedTools` collects skill-declared `allowed-tools` minus what's already on the agent; `initialize.ts` re-runs `loadTools` for the extras and merges resulting `toolDefinitions` into the agent's effective set for the turn. Tool-name resolution is tolerant — unknown names silently drop with a debug log so cross-ecosystem skills referencing yet-to-be-implemented tools (Claude Code's `edit_file`, etc.) import without breaking. The agent document is never modified; the union is turn-scoped. Helper exports (`unionPrimeAllowedTools`) are structured so Phase 5's always-apply primes flow through the same union (combined `[...manualPrimes, ...alwaysApplyPrimes]`) once the resolver lands. Skill handler wire format gains the three fields so clients can render them on detail / list views. * 🎛️ feat: `$` popover reads `userInvocable` instead of UI-only `invocationMode` Replaces the phase-1 UI-only `invocationMode` check with the persisted `userInvocable` field (mirrors the `user-invocable` frontmatter). Skills authored with `user-invocable: false` no longer surface in the popover; the backend resolver enforces the same rule for defense-in-depth. Default-visible behavior is preserved: skills without an explicit `userInvocable` value (older rows, freshly imported skills that don't declare the field) stay visible — only an explicit `false` hides them. Test fixture updated to reflect the new field. * 🔧 fix: Address Phase 6 review findings Codex P2 + reviewer #1: Single `loadTools` call with the union of `agent.tools + allowed-tools`. The earlier two-call approach dropped `userMCPAuthMap` / `toolContextMap` / `actionsEnabled` from the skill-added pass — an MCP tool gained via `allowed-tools` would be visible to the model but fail at execution without per-user auth context. Resolution of `manualSkillPrimes` is hoisted before `loadTools` so the union can be computed up-front; the dropped-tools debug log now compares loaded vs. requested across the single call. Codex P3 + reviewer #2: `injectSkillCatalog.activeSkillIds` now includes `disable-model-invocation: true` skills. The runtime ACL check in `handleSkillToolCall` previously couldn't reach the explicit "cannot be invoked by the model" rejection because the broader access set excluded those skills. Catalog text and tool registration still gate on the visible subset (zero-context-token guarantee preserved); only the per-user `isActive` filter is a hard exclusion now. Reviewer #1 (try/catch around loadTools, MAJOR): A single bad `allowed-tools` entry from a shared skill could crash the entire turn. Now wrapped — on failure with extras, retry with just `agent.tools` and continue (the dropped-tools debug log surfaces what vanished). If the retry-without-extras still throws, propagate; the agent's own tools are the load-bearing surface. Reviewer #3 (integration tests, MAJOR): Added six tests in `initialize.test.ts` covering the full `allowed-tools` loading path: union pass-through, no-extras short-circuit, agent-baseline dedup, loadTools throw + retry, propagated throw without extras, and the empty-tools edge case. Smaller cleanups bundled in: - Reviewer #4: Moved `logger` import to the package-imports section (was wedged among local imports). - Reviewer danny-avila#5: Removed unused index on `disableModelInvocation` (filtering happens application-side in `injectSkillCatalog`; index cost write overhead for zero query benefit). - Reviewer danny-avila#6: Swapped order of `userInvocable` and body checks in `resolveManualSkills` so the more authoritative author-decision reason surfaces first when both apply. - Reviewer danny-avila#8: Documented the `allowedTools` enforcement gap on the schema + type — model-invoked skills (mid-turn `skill` tool calls) do NOT trigger tool union, since adding tools after the graph starts would require a rebuild. Manual / always-apply (Phase 5) primes are the supported paths. - Reviewer danny-avila#9: Renamed `dmi` / `ui` / `at` locals to `disableModelInvocationRaw` / `userInvocableRaw` / `allowedToolsRaw` in `deriveStructuredFrontmatterFields`. Reviewer danny-avila#7 (DRY shared `getSkillByName` return type) deferred — field sets diverge meaningfully across the three call sites (handler needs `body + fileCount`; resolver needs `author + allowedTools + userInvocable`; the InitializeAgentDbMethods contract needs the superset). A `Pick<>`-based consolidation is a follow-up cleanup. * 🔧 fix: Address codex iter 2 — catalog quota + duplicate-name dedup P1: `injectSkillCatalog` cap now counts only model-visible skills, not the merged active set. The previous behavior let a tenant with many `disable-model-invocation: true` rows near the top of the cursor exhaust the 100-slot quota before any invocable skill got scanned — the catalog could end up empty even though invocable skills existed further down the paginated results. `MAX_CATALOG_PAGES` stays the ceiling on scan budget; only `visibleCount` drives the early-exit on quota fill. P2: When an invocable and a `disable-model-invocation: true` skill share a name, drop the disabled doc(s) from `activeSkillIds`. Without this dedup, `getSkillByName` (which sorts by `updatedAt` desc) could pick the disabled doc and every model call to the cataloged name would fail with "cannot be invoked by the model" instead of executing the visible skill. When ONLY a disabled doc exists for a name, it stays in `activeSkillIds` so the explicit-rejection error path still fires for hallucinated invocations. Tests: 3 new cases in `injectSkillCatalog` covering (a) cap counted on visible skills only, (b) same-name collision drops disabled doc, (c) sole-disabled-name case keeps the disabled doc. * 🔒 fix: Apply `disable-model-invocation` gate to read_file too (codex iter 3 P1) `activeSkillIds` is shared between the `skill` and `read_file` handlers. The skill-tool gate was applied last iteration, but `handleReadFileCall` authorized purely on `getSkillByName(..., accessibleIds)` — so a model that learned a hidden skill's name (stale catalog or hallucination) could still read its `SKILL.md` body or bundled files via `read_file`, defeating the contract. Same explicit rejection now fires from both handlers; no change needed to the ACL set itself (disabled docs stay in `activeSkillIds` so the explicit error path keeps firing). Two new tests in `handlers.spec.ts` cover the read_file gate and regression-protect the happy path. * 🔧 fix: Address codex iter 4 — manual-prime exception + legacy frontmatter backfill P1: Scope the `read_file` `disableModelInvocation` gate to AUTONOMOUS model probes only. A user-invoked `$` skill that is also marked `disable-model-invocation: true` had its bundled `references/*` / `scripts/*` files unreadable, leaving the manually-primed skill body referencing files the model couldn't load. Now the handler bypasses the gate when the skill name appears in `manualSkillNames` (the per-turn allowlist threaded from `manualSkillPrimes` → `agentToolContexts` → `enrichWithSkillConfigurable` → `mergedConfigurable`). Defense-in-depth: the bypass is scoped to the specific names in the allowlist; a different disabled skill name is still rejected. P2: Read-time fallback for legacy skills authored before Phase 6 landed the structured columns. `user-invocable: false` / `disable-model-invocation: true` set in `frontmatter` (the validator already accepted those keys) but with no derived column would incorrectly evaluate as "user-invocable / model-allowed" until a save backfilled the columns. New `backfillDerivedFromFrontmatter` helper fills undefined columns from frontmatter at read time in both `getSkillByName` and `listSkillsByAccess` — column wins when both are set, frontmatter fills the gap when only it's set. No DB writes; the next `updateSkill` naturally persists. `listSkillsByAccess` projection expanded to include `frontmatter` (bounded by validator, payload impact small) so summaries can also be backfilled. Sticky-primed disabled skills (ones invoked in prior turns of the same conversation) are not yet in the manual-prime allowlist — same- turn manual invocation is the load-bearing path codex flagged; the sticky-turn case is a known limitation tracked for a follow-up. Tests: 2 new in handlers.spec.ts (manual-prime allows + name-scoped block holds), 3 new in skill.spec.ts (legacy backfill via getSkillByName + listSkillsByAccess + column-wins precedence). * 🔧 fix: Address codex iter 5 — propagate manualSkillNames + keep read_file P1: `enrichWithSkillConfigurable` is also called from `openai.js` and `responses.js` (the OpenAI Responses + completions endpoints). Both were ignoring the new `manualSkillNames` parameter, which meant the manual-prime exception in the `read_file` gate (iter 4) only worked on the agents endpoint. Now all three call sites pass `primaryConfig.manualSkillPrimes?.map(p => p.name)` so manual `$` invocations of disabled skills work consistently across endpoints. P2: When every accessible skill is `disable-model-invocation: true`, the catalog text and `skill` tool are correctly omitted (no model- reachable targets) — but `read_file` and `bash_tool` MUST still be registered. A user manually invoking such a skill gets its SKILL.md body primed into context; if the body references `references/foo.md` or `scripts/run.sh`, those reads need a registered tool. Restructured `injectSkillCatalog` so `skill` registration is gated on `catalogVisibleSkills.length > 0` while `read_file` (always) and `bash_tool` (when codeEnvAvailable) register whenever any active skill is in scope. Tests: existing all-disabled test rewritten to assert read_file IS registered + skill is NOT; new test confirms bash_tool joins it when codeEnvAvailable. * 🔧 fix: Address codex iter 6 — name-collision consistency via preferInvocable P2a (resolveManualSkills): a name collision between an older user-invocable doc and a newer non-user-invocable doc made manual `$` invocation silently no-op. The popover surfaced the older invocable doc; resolver looked it up by name; `getSkillByName` returned the newer non-invocable doc; resolver skipped on `userInvocable: false`. P2b (handler / runtime ACL): with same-name duplicates (e.g. older invocable + newer disabled), the manual prime resolved to one doc while later `read_file` / `skill` execution resolved a different doc through `activeSkillIds`. Model could follow one SKILL.md body while reading files from a different skill. Both root-cause: `getSkillByName` always returned the newest match and let the caller filter, but with collisions the newest can be something the caller didn't want. Fix: extend `getSkillByName` with `options.preferInvocable`. When true, prefer the newest doc satisfying BOTH `userInvocable !== false` AND `disableModelInvocation !== true` (with frontmatter backfill); fall back to the newest match otherwise. Fast path preserved when caller doesn't opt in. Callers passing `preferInvocable: true`: - `resolveManualSkills` — picks the popover-visible invocable doc even when a newer disabled / non-user-invocable duplicate exists. - `handleSkillToolCall` — keeps execution aligned with the catalog; falls back to the disabled doc only when no invocable variant exists (so the explicit "cannot be invoked by the model" gate still fires for the hallucinated-disabled-name case). - `handleReadFileCall` — same alignment, plus the manual-prime exception added in iter 4 still applies. Tests: - 2 new in skill.spec.ts (preferInvocable picks invocable when collision exists; falls back to newest when no clean-invocable exists). - 1 new in skills.test.ts (resolver passes preferInvocable through). - 2 new in handlers.spec.ts (skill tool + read_file pass it). - Existing initialize.test.ts assertion updated for the new option. * 🔧 fix: Address codex iter 7 — split preferInvocable into per-axis flags The previous unified `preferInvocable` filter required BOTH `userInvocable !== false` AND `disableModelInvocation !== true`. That was wrong for the model paths: `userInvocable: false` skills are model-only and remain valid `skill` / `read_file` invocation targets. A duplicate-name scenario where the newer cataloged doc was model- only would let the older user-invocable variant shadow it on every model call. Split the option into two independent axes: - `preferUserInvocable` — for manual paths (`$` popover). Skips docs with `userInvocable: false`. Disable-model-invocation status is irrelevant; iter 4 explicitly supports manual prime of disabled skills. - `preferModelInvocable` — for model paths (`skill` / `read_file` handlers). Skips docs with `disableModelInvocation: true`. User- invocable status is irrelevant; model-only skills are valid here. Both flags fall back to the newest match when no preferred doc exists, so the explicit-rejection error paths still fire correctly in the sole-disabled-name case. Callers updated: - `resolveManualSkills` → `preferUserInvocable: true` - `handleSkillToolCall` / `handleReadFileCall` → `preferModelInvocable: true` Tests: - New spec test for preferModelInvocable not filtering on userInvocable. - Existing preferInvocable test renamed/split to cover the new axes. - New test asserts preferUserInvocable still returns disabled docs (preserves iter 4 manual-disabled support). - Caller tests assert each path passes the right single flag and does NOT pass the wrong one. * 🔧 fix: TypeScript type-check failure in handlers.spec.ts (CI green) `jest.fn(async () => ...)` without explicit args infers an empty tuple for the call signature, so `mock.calls[0][2]` flagged as "Tuple type '[]' has no element at index '2'." Cast to `unknown[]` then narrow to the expected option shape. Behavior unchanged. Caught by the `Type check @librechat/api` CI step (.github/workflows/backend-review.yml). * 🔧 fix: Address codex iter 8 — undefined-result fallback + read_file alignment P1 (loadTools returning undefined): Production loaders (`createToolLoader` in `initialize.js` / `openai.js` / `responses.js`) wrap `loadAgentTools` in try/catch and return `undefined` on failure rather than throwing. Without explicit handling, my iter-1 try/catch only fired for thrown errors — a silent-failure on a skill-added tool would fall through to the empty fallback and silently DROP the agent's baseline tools for the turn (much worse than just losing the extras). Added an `undefined`-result branch that retries with just `agent.tools`, mirroring the throw branch. Test pins both behaviors. P2 (read_file alignment with manual prime): When a skill is in this turn's `manualSkillNames`, the `read_file` handler now uses `preferUserInvocable` instead of `preferModelInvocable`. Same name-collision rule as `resolveManualSkills`, so the doc whose files get read is the same doc whose body got primed. For autonomous probes (skill not in `manualSkillNames`), the handler keeps `preferModelInvocable` to align with the catalog the model saw. Two new tests cover both branches and regression-protect that the wrong flag isn't passed. * 🔧 fix: Address codex iter 9 — pin read_file lookup to primed skill _id P1 (manually-primed disabled IDs were dropped from activeSkillIds): The `executableSkills` dedup in `injectSkillCatalog` correctly drops `disable-model-invocation: true` duplicates when an invocable doc shares the name — but `resolveManualSkills` legitimately primes disabled docs (iter 4 supports manual `$` invocation of disabled skills). When the resolver primed a disabled doc, the read_file handler couldn't find it in the (deduped) `activeSkillIds` and either resolved a different same-name skill or returned not-found. Fix: `ResolvedManualSkill` now carries `_id`; the legacy `initialize.js` / `openai.js` / `responses.js` controllers build a `manualSkillPrimedIdsByName` map and `enrichWithSkillConfigurable` passes it into `mergedConfigurable`. `handleReadFileCall` now pins its lookup's `accessibleIds` to `[primedId]` whenever the requested skill is in the map. The constrained set guarantees the lookup returns the EXACT doc the resolver primed — body/files come from the same source even when same-name duplicates exist or the dedup removed the prime's id from `activeSkillIds`. Autonomous read_file probes (skill not in the manual-primed map) keep the full ACL set + `preferModelInvocable` so they align with the catalog the model saw and the disabled-only case still fires the explicit-rejection gate. Test fixture changes flow from `_id` becoming required on `ResolvedManualSkill`. `buildSkillPrimeContentParts` / `injectManualSkillPrimes` widen their param types to `Pick<...>` because they only read `name` / `body` and shouldn't force test literals to invent placeholder ids. * 🧹 fix: Address independent reviewer findings (DRY + types + tests + docs) Sanity-pass review surfaced 7 findings; addressed 6 (the 7th — DRY on inline `getSkillByName` return types — is acknowledged tech debt deferred to a follow-up). #1 [MAJOR, DRY]: The 4-line `manualSkillPrimedIdsByName` map construction was duplicated across 4 CJS call sites (openai.js, responses.js x2, initialize.js). Extracted `buildManualSkillPrimedIdsByName` helper in `skillDeps.js`; all four sites now call the helper. If `ResolvedManualSkill` ever renames `_id` or gains identifying fields, only the helper changes. #2 [MINOR, type safety]: `handleReadFileCall` was casting a hex string to `Types.ObjectId[]` via `as unknown as`, relying on mongoose's auto-cast in `$in` queries. Replaced with `new Types.ObjectId(...)` so any future consumer comparing with `.equals()` / `===` gets the correct value type. Imported `Types` as a value (was type-only). danny-avila#5 [MINOR, test gap]: Added a test for the worst-case silent-failure path — both the union and base-only `loadTools` calls return undefined. The agent gets no tools but the turn doesn't crash hard; pinning that contract. #4 [MINOR, performance]: Added a TODO on the `listSkillsByAccess` projection noting the `frontmatter` field can be dropped once a write migration backfills all pre-Phase-6 skills' columns. ~2KB/skill × 100/page is wasted bandwidth post-backfill. danny-avila#6 [NIT, docs]: `backfillDerivedFromFrontmatter` JSDoc said "Pure" right before "mutates its undefined fields in place". Replaced with "Side-effect-free w.r.t. the DB (no writes), but mutates its argument in place" which describes both halves accurately. danny-avila#7 [NIT, test determinism]: Replaced `await new Promise(r => setTimeout(r, 5))` in two same-name collision tests with explicit `updateOne` setting `updatedAt: new Date(Date.now() - 1000)` on the older doc. Removes the wall-clock race on fast CI runners. The pagination test (line 480) still uses setTimeout — that test is pre-existing and order is incidental, not load-bearing. Existing test fixtures updated to use valid 24-char hex ObjectIds (required by the iter-9 test that constructs a real `ObjectId`). #3 [MINOR, deferred]: Inline `getSkillByName` return type duplicated across `handlers.ts`, `initialize.ts`, `skills.ts`. Reviewer acknowledged this as deferred; field sets diverge across call sites (handler needs `fileCount`, resolver needs `author`/`allowedTools`). A `Pick<>`-based consolidation is a clean follow-up.

* 🛠️ feat: Add registerCodeExecutionTools helper Idempotently registers `bash_tool` + `read_file` in the run's tool registry and tool-definition list via a registry `.has()` dedupe. Sets up the single code-execution tool path shared by: - `initializeAgent` (when an agent has `execute_code` in its tools and the capability is enabled for the run) - `injectSkillCatalog` (when skills are active; unconditional read_file, bash_tool follows `codeEnvAvailable`) Both callers reach the helper in the same initialization sequence, so the second call becomes a no-op and exactly one copy of each tool reaches the LLM — no more double registration for agents that combine `execute_code` capability with active skills. Unit-tested on a fresh run, idempotence (second call, overlap with prior tooldefs, partial overlap), and the no-registry variant. * 🔀 refactor: Route injectSkillCatalog bash_tool + read_file through registerCodeExecutionTools The `skill` tool is still registered inline (it's skill-path-specific), but `bash_tool` + `read_file` now flow through the shared idempotent helper so a prior registration from the execute_code path doesn't produce a duplicate copy later in the same run. Behavior preserved: - `read_file` always registers when any active skill is in scope — manually-primed `disable-model-invocation: true` skills still need it to load `references/*` from storage. - `bash_tool` follows `codeEnvAvailable` exactly as before. Adds a test pinning the cross-call dedupe: when `injectSkillCatalog` runs AFTER `registerCodeExecutionTools` has already seeded the registry + tool definitions with bash_tool/read_file, the resulting `toolDefinitions` still contains exactly one copy of each. * 🪄 feat: Expand `execute_code` tool name into bash_tool + read_file at initialize-time When an agent's `tools` include `execute_code` and the `execute_code` capability is enabled for the run, `initializeAgent` now registers `bash_tool` + `read_file` via `registerCodeExecutionTools` before `injectSkillCatalog`. The legacy `execute_code` tool definition is no longer handed to the LLM — `execute_code` remains on the agent document as a capability-trigger marker, but the runtime expands it into the skill-flavored tool pair. Call ordering matters: the `execute_code` registration runs BEFORE `injectSkillCatalog`, so the skill path's own `registerCodeExecutionTools` call inside `injectSkillCatalog` becomes a no-op via the registry's `.has()` check. Exactly one copy of each tool reaches the LLM whether the agent has: - only `execute_code` (legacy path) - only skills - both No data migration needed — `agent.tools: ['execute_code']` stays in the DB unchanged; the expansion is a runtime operation. Three tests cover the matrix: execute_code + capability on → bash_tool + read_file registered; execute_code + capability off → neither registered; no execute_code + capability on → neither registered. * 🗑️ refactor: Drop CodeExecutionToolDefinition from the builtin registry Removes the legacy `execute_code` entry from `agentToolDefinitions` and the corresponding import. With the initialize-time expansion in place, nothing consults `getToolDefinition('execute_code')` for a tool schema any more — the capability gate still filters on the string `execute_code`, but the actual tool definitions the LLM sees come from `registerCodeExecutionTools` (i.e. `bash_tool` + `read_file`). `loadToolDefinitions` in `packages/api/src/tools/definitions.ts` silently drops `execute_code` when it no longer resolves in the registry — that's the expected path and is now covered by an updated test. No caller of `getToolDefinition('execute_code')` expects a non-undefined result after this change. * 🔌 refactor: Read CODE_API_KEY from env for primeCodeFiles + PTC Finishes the Phase 4 server-env-keyed rollout on the two remaining `loadAuthValues({ authFields: [EnvVar.CODE_API_KEY] })` sites in `ToolService.js`: - `primeCodeFiles` (user-attached file priming on execute_code agents) - Programmatic Tool Calling (`createProgrammaticToolCallingTool`) Both now read `process.env[EnvVar.CODE_API_KEY]` directly, matching `bash_tool`'s pattern. The per-user plugin-auth path is no longer consulted for code-env credentials anywhere in the hot path — the agents library owns the actual tool-call execution and also reads the env var internally. Priming still fires for existing user-file workflows so the legacy `toolContextMap[execute_code]` hint ("files available at /mnt/data/...") stays in the prompt; only the key lookup changed. * 🔧 fix: Type the pre-seeded dedupe-test tools as LCTool CI TypeScript type checks caught `{ parameters: {} }` in the new cross-call dedupe test: `LCTool.parameters` is a `JsonSchemaType`, not `{}`. Use `{ type: 'object', properties: {} }` and type the local registry Map through the parameter-derived shape so the pre-seeded values match what `toolRegistry.set` expects. * 🛡️ fix: Run execute_code expansion before GOOGLE_TOOL_CONFLICT gate Codex review caught a latent regression: the original Phase 8 placement ran `registerCodeExecutionTools` after `hasAgentTools` was computed, so an execute-code-only agent on Google/Vertex with provider-specific `options.tools` populated would no longer trip `GOOGLE_TOOL_CONFLICT` — the legacy `CodeExecutionToolDefinition` used to populate `toolDefinitions` before the guard, but after dropping it from the registry, `toolDefinitions` stayed empty until my expansion ran downstream of the guard. Mixed provider + agent tools would silently flow through to the LLM. Fix moves the `execute_code` expansion to BEFORE `hasAgentTools` computation. `bash_tool` + `read_file` now contribute to the check the same way the legacy `execute_code` def did. Covered by a new test that pins the Google+execute_code+provider-tools scenario — the `rejects.toThrow(/google_tool_conflict/)` path would have silently passed on the prior placement. * 🔗 fix: Thread codeEnvAvailable through handoff sub-agents Round-2 codex review caught the other half of the execute_code expansion gap: `discoverConnectedAgents` omitted `codeEnvAvailable` from its forwarded `initializeAgent` params, so handoff sub-agents with `agent.tools: ['execute_code']` lost the `bash_tool` + `read_file` registration (pre-Phase 8 the legacy `CodeExecutionToolDefinition` would have landed in their `toolDefinitions` via the registry). - Add `codeEnvAvailable?` to `DiscoverConnectedAgentsParams` and forward it verbatim on every sub-agent `initializeAgent` call. - Update the three JS call sites that construct the primary's `codeEnvAvailable` (`services/Endpoints/agents/initialize.js`, `controllers/agents/openai.js`, `controllers/agents/responses.js`) to pass the same flag into `discoverConnectedAgents` — one authoritative source per request. - Two regression tests in `discovery.spec.ts` pin the true/false passthrough so a future refactor that drops the param-forwarding surfaces immediately. Left intentionally unchanged: `packages/api/src/agents/openai/service.ts` (public API helper with no in-repo caller). External consumers of `createAgentChatCompletion` who want code execution should pass a `codeEnvAvailable`-aware `initializeAgent` via `deps` — documenting the full public-API surface is out of scope for this Phase 8 PR. * 🔗 fix: Thread codeEnvAvailable through addedConvo + memory-agent paths Round-3 codex review caught the last two production `initializeAgent` callers missing the Phase-8 capability flag: - `api/server/services/Endpoints/agents/addedConvo.js` (multi-convo parallel agent execution). Added `codeEnvAvailable` to `processAddedConvo`'s destructured params and forwarded it into the per-added-agent `initializeAgent` call. Caller in `api/server/services/Endpoints/agents/initialize.js` passes the same `codeEnvAvailable` it computed for the primary. - `api/server/controllers/agents/client.js` (`useMemory` — memory extraction agent). Computes its own `codeEnvAvailable` from `appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities` and forwards into `initializeAgent`. Memory agents rarely list `execute_code`, but if one does, pre-Phase 8 they got the legacy `execute_code` tool registered unconditionally — the passthrough restores parity. With this, every production caller of `initializeAgent` explicitly resolves the capability: main chat flow (primary + handoff), OpenAI chat completions (primary + handoff), Responses API (primary + handoff), added convo parallel agents, and memory agents. The one remaining caller, `packages/api/src/agents/openai/service.ts::createAgentChatCompletion`, is a public API helper with no in-repo consumer (external callers must pass a capability-aware `initializeAgent` via `deps`). * 🪤 fix: Remove duplicate appConfig declaration causing TDZ ReferenceError The Responses API controller had TWO `const appConfig = req.config;` bindings inside `createResponse`: one at the top of the function (added by the Phase 4 `bash_tool` decouple) and one inside the try block (added by the polish PR danny-avila#12760). Because `const` is block-scoped with a temporal dead zone, the inner redeclaration put `appConfig` in TDZ for the entire try block, so any earlier reference inside the try — notably `appConfig?.endpoints?.[EModelEndpoint.agents]?.allowedProviders` at line 348 — threw `ReferenceError: Cannot access 'appConfig' before initialization`. The error was silently swallowed by the outer try/catch, leaving `recordCollectedUsage` unreached and the six `responses.unit.spec.js` token-usage tests failing. Removing the inner redeclaration fixes the six failing tests (verified: 11/11 pass locally post-fix, 0 regressions elsewhere). The outer function-scoped binding already provides `appConfig` to every downstream reference. * 🔗 fix: Thread codeEnvAvailable through the OpenAI chat-completion public API Round-4 codex review (legitimate on the type-safety angle, even though the runtime concern was already covered): the `createAgentChatCompletion` helper defines its own narrower `InitializeAgentParams` interface locally, and the type was missing `codeEnvAvailable`. External consumers who supply a capability-aware `deps.initializeAgent` couldn't route `codeEnvAvailable` through without a type-cast workaround. - Widen the local `InitializeAgentParams` interface to include `codeEnvAvailable?: boolean` (matches the real `packages/api/src/agents/initialize.ts` type). - Derive `codeEnvAvailable` inside `createAgentChatCompletion` from `deps.appConfig?.endpoints?.agents?.capabilities` (the same source the in-repo controllers use) and forward to `deps.initializeAgent`. Uses a string literal `'execute_code'` lookup so this file stays free of a `librechat-data-provider` import — keeping the dependency surface of the public helper minimal. With this, external consumers of `createAgentChatCompletion` who pass `appConfig` with the agents capabilities get `bash_tool` + `read_file` registration automatically; consumers who don't pass `appConfig` retain the existing "explicit opt-in" semantics (the flag stays `undefined`, expansion is skipped). * 🧹 chore: Review-driven polish — observability log, JSDoc DRY, test gaps, no-op allocation Addresses the comprehensive review of PR danny-avila#12767: - **Finding #1** (MINOR, observability): `initializeAgent` now emits a debug log when an agent lists `execute_code` in its tools but the runtime gate is off (`params.codeEnvAvailable` !== true). The event-driven `loadToolDefinitionsWrapper` path doesn't log capability-disabled warnings, so without this the tool silently vanishes from the LLM's definitions with zero trace. Operators debugging "why isn't code interpreter working?" now get a signal at the initialize layer. - **Finding danny-avila#5** (NIT, allocation): `registerCodeExecutionTools` now returns the input `toolDefinitions` array by reference on the no-op path (both tools already registered by a prior caller in the same run) instead of allocating a fresh spread array every time. The common dual-call scenario — `initializeAgent` then `injectSkillCatalog` — saves one O(n) copy per request. - **Finding #4** (NIT, DRY): Collapsed the duplicated 6-line JSDoc comment in `openai.js`, `responses.js`, and `addedConvo.js` into either a one-line `@see DiscoverConnectedAgentsParams.codeEnvAvailable` pointer (the two JS call sites) or a compact 3-line block referring back to the canonical source (addedConvo's @param). - **Finding #2** (MINOR, test gap): Added `api/server/services/Endpoints/agents/addedConvo.spec.js` with three cases covering `codeEnvAvailable=true`, `codeEnvAvailable=false`, and omitted (undefined) passthrough. A future refactor that drops the param from destructuring now surfaces here instead of silently regressing multi-convo parallel agents with `execute_code`. - **Finding #3** (MINOR, test gap): Added `api/server/controllers/agents/__tests__/client.memory.spec.js` pinning the capability-flag derivation that `AgentClient::useMemory` uses — six cases covering present/absent/null/undefined config shapes plus an enum-literal pin (`'execute_code'` / `'agents'`). Catches enum renames or config-path shifts that would otherwise silently strip `bash_tool` + `read_file` from memory agents. Finding danny-avila#7 (jest.mock scoping, confidence 40) left as-is: the reviewer's own risk assessment noted `buildToolSet` doesn't touch the mocked exports, and restructuring a file-level `jest.mock` to `jest.doMock` + dynamic `import()` introduces more complexity than the speculative risk justifies. The existing mock is scoped to the test file and contains the same stubs the adjacent `skills.test.ts` already uses. Finding danny-avila#6 (PR description commit count) addressed out-of-band via PR description update. All existing tests pass, typecheck clean, lint clean across touched files. New tests: 9 cases across 2 new spec files. * 🧽 refactor: Replace hardcoded 'execute_code' string with AgentCapabilities enum in service.ts Follow-up review (conf 55) caught that `openai/service.ts`'s Phase 8 `codeEnvAvailable` derivation used the literal `'execute_code'` while every in-repo controller uses `AgentCapabilities.execute_code` from `librechat-data-provider`. The file deliberately uses local type interfaces to keep the public API helper's type surface small, but that pattern was never a ban on single-value imports from the data provider — `packages/api` already depends on it. Importing the enum value means a future rename of `AgentCapabilities.execute_code` propagates to this file automatically, matching the in-repo controllers' behavior. Other follow-up findings left as-is per the reviewer's own verdict: - #2 (memory spec mirrors the production expression rather than calling `AgentClient::useMemory` directly): reviewer flagged as "not blocking" / "design-philosophy observation." The test file's JSDoc already explicitly documents the tradeoff and pins the enum literals to catch the most likely drift vector. Standing up `AgentClient` + all its mocks for a one-line regression guard is disproportionate. - #3 (`addedConvo.spec.js` mock signature vs. underlying `loadAddedAgent` arity): reviewer's own confidence 25 noted the mock matches the wrapper's actual call pattern in the production file. Not a real gap. - #4 was self-retracted as a false alarm. * 🗑️ refactor: Fully deprecate CODE_API_KEY — remove all LibreChat-side references The code-execution sandbox no longer authenticates via a per-run `CODE_API_KEY` (frontend or backend). Auth moved server-side into the agents library / sandbox service, so LibreChat drops every reference: **Backend plumbing:** - `api/server/services/Files/Code/crud.js`: `getCodeOutputDownloadStream`, `uploadCodeEnvFile`, `batchUploadCodeEnvFiles` no longer accept `apiKey` or send the `X-API-Key` header. - `api/server/services/Files/Code/process.js`: `processCodeOutput`, `getSessionInfo`, `primeFiles` drop the `apiKey` param throughout. - `api/server/services/ToolService.js`: stop reading `process.env[EnvVar.CODE_API_KEY]` for `primeCodeFiles` and PTC; the agents library handles auth internally. Remove the now-dead `loadAuthValues` + `EnvVar` imports. Drop the misleading "LIBRECHAT_CODE_API_KEY" hint from the bash_tool error log. - `api/server/services/Files/process.js`: remove the `loadAuthValues` call around `uploadCodeEnvFile`. - `api/server/routes/files/files.js`: code-env file download no longer fetches a per-user key. - `api/server/controllers/tools.js`: `execute_code` is no longer a tool that needs verifyToolAuth with `[EnvVar.CODE_API_KEY]` — the endpoint always reports system-authenticated so the client skips the key-entry dialog. `processCodeOutput` called without `apiKey`. - `api/server/controllers/agents/callbacks.js`: `processCodeOutput` invoked without the loadAuthValues round trip, for both LegacyHandler and Responses-API handlers. - `api/app/clients/tools/util/handleTools.js`: `createCodeExecutionTool` called with just `user_id` + files. **packages/api:** - `packages/api/src/agents/skillFiles.ts`: `PrimeSkillFilesParams`, `PrimeInvokedSkillsDeps`, `primeSkillFiles`, `primeInvokedSkills` all drop the `apiKey` param; the gate is purely `codeEnvAvailable`. - `packages/api/src/agents/handlers.ts`: `handleSkillToolCall` drops the `process.env[EnvVar.CODE_API_KEY]` read; skill-file priming is now gated solely on `codeEnvAvailable`. `ToolExecuteOptions` signatures drop apiKey from `batchUploadCodeEnvFiles` and `getSessionInfo`. - `packages/api/src/agents/skillConfigurable.ts`: JSDoc no longer references the env var. - `packages/api/src/tools/classification.ts`: PTC creation no longer gated on `loadAuthValues`; `buildToolClassification` drops the `loadAuthValues` dep entirely (no LibreChat-side callers need it for this path anymore). - `packages/api/src/tools/definitions.ts`: `LoadToolDefinitionsDeps` drops the `loadAuthValues` field. **Frontend:** - Delete `client/src/hooks/Plugins/useAuthCodeTool.ts`, `useCodeApiKeyForm.ts`, and `client/src/components/SidePanel/Agents/Code/ApiKeyDialog.tsx` — the install/revoke dialogs for CODE_API_KEY are fully dead. - `BadgeRowContext.tsx`: drop `codeApiKeyForm` from the context type and provider. `codeInterpreter` toggle treated as always authenticated (sandbox auth is server-side). - `ToolsDropdown.tsx`, `ToolDialogs.tsx`, `CodeInterpreter.tsx`, `RunCode.tsx`, `SidePanel/Agents/Code/Action.tsx` +`Form.tsx`: all API-key dialog trigger refs, "Configure code interpreter" gear buttons, and auth-verification plumbing removed. The "Code Interpreter" toggle is now a plain `AgentCapabilities.execute_code` checkbox — no key-entry gate. - `client/src/locales/en/translation.json`: drop the three `com_ui_librechat_code_api*` keys and `com_ui_add_code_interpreter_api_key`. Other locales are externally automated per CLAUDE.md. **Config:** - `.env.example`: remove the `# LIBRECHAT_CODE_API_KEY=your-key` section and its header. **Tests:** - `crud.spec.js`: assertions flipped to pin "no X-API-Key header" and "no apiKey param". - `skillFiles.spec.ts`: removed env-var save/restore; tests now pin that the batch-upload path is gated solely on `codeEnvAvailable` and that no apiKey is threaded through. - `handlers.spec.ts`: same — just the `codeEnvAvailable` gate pins remain. - `classification.spec.ts`: remove the two tests that asserted `loadAuthValues` was (not) called for PTC. - `definitions.spec.ts`: drop every `loadAuthValues: mockLoadAuthValues` entry from the deps shape. - `process.spec.js`: strip the mock of `EnvVar.CODE_API_KEY`. **Comment hygiene:** - `tools.ts`, `initialize.ts`, `registry/definitions.ts`: shortened stale comment references to "legacy `execute_code` tool" without naming the retired env var. Tests verified: 678 packages/api tests pass, 836 backend api tests pass. Typecheck clean, lint clean. Only remaining CODE_API_KEY mentions in the code are two regression-guard assertions: - `crud.spec.js`: pins "no X-API-Key header" stays absent. - `skillConfigurable.spec.ts`: pins `configurable` never grows a `codeApiKey` field. * 🧹 chore: Remove the last two CODE_API_KEY name mentions in LibreChat Follow-up to the prior full deprecation commit: two tests still named the retired identifier in their regression-guard assertions. - `packages/api/src/agents/skillConfigurable.spec.ts`: drop the "does not inject a codeApiKey key" test. The `codeApiKey` field is gone from the production configurable shape, so an absence-assertion naming it re-introduces the retired identifier in code. - `api/server/services/Files/Code/crud.spec.js`: rename the "without an X-API-Key header" case back to "should request stream response from the correct URL" and drop the `expect(headers).not.toHaveProperty('X-API-Key')` assertion. The surrounding request-shape checks (URL, timeout, responseType) still pin the behavior; the explicit header-absence line was named-after the deprecated contract. Result: `grep -rn "CODE_API_KEY\|codeApiKey\|LIBRECHAT_CODE_API_KEY"` against the LibreChat source tree returns zero hits. The only remaining `X-API-Key` strings in this repo are on unrelated OpenAPI Action + MCP server auth configurations, where the string is user-facing config, not a LibreChat-owned identifier. Tests: 677 packages/api pass (2 pre-existing summarization e2e failures unrelated); 126 api-workspace controller/service tests pass. Typecheck and lint clean. * 🎯 fix: Narrow codeEnvAvailable to per-agent (admin cap AND agent.tools) Before this commit, `codeEnvAvailable` was computed in the three JS controllers as the admin-level capability flag only (`enabledCapabilities.has(AgentCapabilities.execute_code)`) and passed through `initializeAgent` → `injectSkillCatalog` / `primeInvokedSkills` / `enrichWithSkillConfigurable` unchanged. A skills-only agent whose `tools` array didn't include `execute_code` still got `bash_tool` registered (via `injectSkillCatalog`) and skill files re-primed to the sandbox on every turn — wrong, because the agent never opted in to code execution. **Fix:** `initializeAgent` now computes the per-agent effective value once as `params.codeEnvAvailable === true && agent.tools.includes(Tools.execute_code)`, reuses the same boolean for: 1. The `execute_code` → `bash_tool + read_file` expansion gate (previously already consulted `agent.tools`; now shares the single `effectiveCodeEnvAvailable` binding). 2. The `injectSkillCatalog` call (previously got the raw admin flag). 3. The returned `InitializedAgent.codeEnvAvailable` field (new, typed as required boolean). **Controllers (initialize.js, openai.js, responses.js):** store `primaryConfig.codeEnvAvailable` in `agentToolContexts.set(primaryId, ...)`, capture `config.codeEnvAvailable` in every handoff `onAgentInitialized` callback, and read it from the per-agent ctx inside the `toolExecuteOptions.loadTools` runtime closure. The hoisted `const codeEnvAvailable = enabledCapabilities.has(...)` locals in the two OpenAI-compat controllers are gone — they were shadowing the narrowed per-agent value. **primeInvokedSkills:** `handlePrimeInvokedSkills` in `services/Endpoints/agents/initialize.js` now uses `primaryConfig.codeEnvAvailable` (per-agent, narrowed) instead of the raw admin flag. A skills-only primary agent won't re-prime historical skill files to the sandbox even when the admin enabled the capability globally. **Efficiency:** one extra `&&` in `initializeAgent`. No runtime hot-path cost — the `includes()` scan on `agent.tools` was already happening for the `execute_code` expansion gate; it's now just bound to a local. Tool execution closures read `ctx.codeEnvAvailable === true` (property access + strict equality, O(1)). **Ephemeral-agent note:** per-agent narrowing is authoritative for both persisted and ephemeral flows. The ephemeral toggle (`ephemeralAgent.execute_code`) is reconciled into `agent.tools` upstream in `packages/api/src/agents/added.ts`, so `agent.tools.includes('execute_code')` is the single source of truth by the time `initializeAgent` runs. **Tests:** two new regression tests pin the narrowing contract: - `initialize.test.ts` — four-quadrant matrix on `InitializedAgent.codeEnvAvailable` (cap on × agent asks, cap on × doesn't ask, cap off × asks, neither). Catches future refactors that drop either half of the AND. - `skills.test.ts` — `injectSkillCatalog` with `codeEnvAvailable: false` against an active skill catalog must NOT register `bash_tool` even though it still registers `read_file` + `skill`. This is the state a skills-only agent gets post-narrowing. All 191 affected packages/api tests pass + 836 backend api tests pass. Typecheck clean, lint clean. * 🧽 refactor: Comprehensive-review polish — hoist tool defs, pin verifyToolAuth contract, doc appConfig Addresses the comprehensive review of Phase 8. Findings mapped: **#1 (MINOR): `verifyToolAuth` unconditional auth for execute_code** - Added doc comment explicitly stating the deployment contract (admin capability → reachable sandbox; no per-check health probe to keep UI-gate queries O(1)). - New `api/server/controllers/__tests__/tools.verifyToolAuth.spec.js` with 4 regression tests pinning the contract: 1. `authenticated: true` + `SYSTEM_DEFINED` for execute_code. 2. 404 for unknown tool IDs. 3. `loadAuthValues` is never consulted (catches a future revert that would resurface the per-user key-entry dialog). 4. Response `message` is never `USER_PROVIDED`. **#2 (MINOR): `openai/service.ts` undocumented `appConfig` dependency** - Expanded the `ChatCompletionDependencies.appConfig` JSDoc to spell out that omitting it silently disables code execution for agents with `execute_code` in their tools. External consumers of `createAgentChatCompletion` now have the contract documented at the type boundary. **danny-avila#5 (NIT): `registerCodeExecutionTools` re-allocates tool defs** - Hoisted `READ_FILE_DEF` and `BASH_TOOL_DEF` to module-level `Object.freeze`d constants. The shapes derive entirely from static `@librechat/agents` exports, so a single frozen object per tool is safe to share across every agent init. Eliminates the ~4-property allocations on every call (including the common second-call no-op path). **danny-avila#6 (NIT): Verbose history-priming comment in initialize.js** - Trimmed the 16-line `handlePrimeInvokedSkills` block to a 5-line summary with `@see InitializedAgent.codeEnvAvailable` pointer. The canonical narrowing explanation lives on the type; the controller comment is just the ACL-vs-capability rationale. **Skipped:** - #3 (memory spec tests a mirror function): reviewer self-dismissed as a design tradeoff; the enum-literal pin already catches the highest-risk drift vector. - #4 (cross-repo contract for `createCodeExecutionTool`): user will explicitly install the latest `@librechat/agents` dev version once the companion PR publishes, so the version pin will be authoritative. - danny-avila#7 (migration/deprecation note for self-hosters): out of scope per user direction — release notes handle this. Tests verified: 679 packages/api + 840 backend api tests pass. Typecheck + lint clean. * 🔧 chore: Update @librechat/agents version to 3.1.68-dev.1 across package-lock and package.json files This commit updates the version of the `@librechat/agents` package from `3.1.68-dev.0` to `3.1.68-dev.1` in the `package-lock.json` and relevant `package.json` files. This change ensures consistency across the project and incorporates any updates or fixes from the new version.

…ad_file Sandbox Fallback) (danny-avila#12831) * 🌱 fix: Seed Code Tool Files Into Graph Sessions on First Call Files attached to an agent's `tool_resources.execute_code` (user uploads or generated artifacts from a prior turn) were silently dropped on the first `execute_code` invocation of a turn. The agents-side `ToolNode` populates `_injected_files` only when its `sessions` map already has an `EXECUTE_CODE` entry — but that entry is only written by a previous successful execution, so call #1 had nothing to inject. CodeExecutor then fell back to a `/files/{session_id}` fetch, but `session_id` was also empty on call #1, leaving the sandbox without the primed files. Mirror the existing skill-priming pattern (`primeInvokedSkills` → `initialSessions`) for code-resource files: eagerly call `primeFiles` before `createRun` and merge the result into `initialSessions` via a new `seedCodeFilesIntoSessions` helper. Skill files and code-resource files now share the same `EXECUTE_CODE` entry; the prior representative `session_id` is preserved on merge. * 🔬 chore: Add Diagnostic Logging for Code-Files Seeding Temporary debug logs to diagnose why first-call file injection is not firing in real agent runs. Logs `wantsCodeExec`, available tool-resource keys, primed file count, and the seeded EXECUTE_CODE entry. Will revert once the failure mode is identified. * 🪛 refactor: Capture primedCodeFiles per-agent at init, merge across run Replace the client.js eager `primeFiles` call with a per-agent capture at initialization time so every agent in a multi-agent run (primary + handoff + addedConvo) contributes its `tool_resources.execute_code` files to the shared `Graph.sessions` seed. - handleTools.js (eager loadTools): the `execute_code` factory closes over a `primedCodeFiles` slot and surfaces it in the return. - ToolService.js loadToolDefinitionsWrapper (event-driven): captures `files` from the existing `primeCodeFiles` call (was dropping them while only keeping `toolContext`) and surfaces them. - packages/api initialize.ts: the loadTools callback contract now includes `primedCodeFiles`, threaded onto `InitializedAgent`. - client.js: iterate `[primary, ...agentConfigs.values()]` and merge each agent's `primedCodeFiles` into `initialSessions`. Drop the primary-only `primeCodeFiles` call and diagnostic logs from the prior attempt — wrong layer (single-agent), wrong gate (`agent.tools` contained Tool instances after init, so the `.includes("execute_code")` string check always failed). * 🔬 chore: Add per-agent diagnostic logs for code-files seeding Logs `tool_resources` keys + file counts inside loadToolDefinitionsWrapper and per-agent `primedCodeFiles` + final initialSessions inside AgentClient. Will revert once the failure mode is confirmed. * 🔬 chore: Add file-lookup diagnostics inside initializeAgent Logs the inputs and intermediate counts of the conversation-file lookup chain (convo file ids, thread message ids, code-generated and user-code file counts) so we can pinpoint why `tool_resources.execute_code` is arriving empty at `loadToolDefinitionsWrapper` despite the agent having `execute_code` in its tools list. * 🔬 chore: Probe execute_code files without messageId filter Adds a relaxed `getFiles({conversationId, context: execute_code})` probe that runs only when `getCodeGeneratedFiles` returns empty. Lists what's actually in the DB for this conversation so we can confirm whether the file is missing entirely or whether the messageId filter is rejecting it. * 🔬 chore: Fix probe getFiles arg order (sort vs projection) Probe was passing a projection object as the sort arg, which mongoose rejected with `Invalid sort value`. Move it to the third arg (selectFields) so the probe actually runs. * 🪢 fix: Preserve Original messageId on Code-Output File Update Each `processCodeOutput` call was overwriting the persisted file's `messageId` with the *current* run's id. When a turn re-creates an existing file (filename + conversationId match → `claimCodeFile` returns the existing record, `isUpdate=true`), the file's link to the assistant message that originally produced it gets clobbered. `initializeAgent` later runs `getCodeGeneratedFiles({messageId: $in: <thread>})` to seed `tool_resources.execute_code` from prior-turn artifacts. With a stale `messageId` (e.g. from a failed read attempt that re-shelled the same filename), the file no longer matches the parent-walk thread, so `tool_resources` arrives empty at agent init, the new `primedCodeFiles` channel has nothing to seed, and the LLM can't see its own prior-turn artifacts on the next turn — defeating the just-added Graph-sessions seeding fix. Preserve the existing `claimed.messageId` on update; first-creation behavior is unchanged. The runtime return value still includes the current run's `messageId` (via `Object.assign(file, { messageId })`) so the artifact is correctly attributed to the live tool_call. * 🧹 chore: Remove diagnostic logs from code-files seeding path Drops the temporary debug logs added to trace the empty-tool_resources failure mode. Production code paths (loadToolDefinitionsWrapper, client.js seed loop, initializeAgent file lookup) are left as the permanent shape: capture primedCodeFiles, merge across agents, seed initialSessions before run start. * 🪛 feat: read_file Sandbox Fallback for /mnt/data + Non-Skill Paths When the model called `read_file` with a code-execution path (e.g. `/mnt/data/sentinel.txt`), the handler returned a misleading `Use format: {skillName}/{path}` error. Adds a sandbox-aware fallback: - Short-circuit `/mnt/data/...` (can never be a skill reference) → route to a sandbox `cat` via the new host-provided `readSandboxFile` callback, which POSTs to the codeapi `/exec` endpoint. - Skip the skill resolver entirely when `accessibleSkillIds` is empty — the resolved-output of `resolveAgentScopedSkillIds` already collapses the admin capability + ephemeral badge + persisted `skills_enabled` chain, so an empty value is the authoritative "skills aren't in scope for this agent" signal. - For `{firstSegment}/...` paths, consult the catalog-derived `activeSkillNames` Set (no DB read) to detect non-skill names and fall through to the sandbox before the model has to retry with `bash_tool`. `activeSkillNames` is captured from `injectSkillCatalog`, threaded onto `InitializedAgent`, into `agentToolContexts`, then through `enrichWithSkillConfigurable` into `mergedConfigurable` for the handler. The host implementation of `readSandboxFile` lives in `api/server/services/Files/Code/process.js` and shells `cat <path>` through the seeded sandbox session — `tc.codeSessionContext` (emitted by ToolNode for `read_file` calls in `@librechat/agents` v3.1.72+) provides the `session_id` + `_injected_files` so the read lands in the same sandbox that holds prior-turn artifacts. When the seeded context isn't available (older agents version, no codeapi configured), the handler returns a model-visible error pointing at `bash_tool` instead of silently failing. Tests: 8 new `handleReadFileCall` cases cover the new short-circuits, the skills-not-enabled gate, the activeSkillNames lookup, the sandbox-fallback success path, and the bash_tool retry hint on fallback failure. Existing `read_file` tests now opt into "skills are in scope" via a `skillsInScope()` fixture (production wouldn't reach the skill lookup with empty `accessibleSkillIds`). * 🔧 chore: Update @librechat/agents dependency to version 3.1.72 Bumps the version of the @librechat/agents package across package-lock.json and relevant package.json files to ensure compatibility with the latest features and fixes. * 🪛 refactor: Centralize Tool-Session Seed in buildInitialToolSessions Helper Addresses review feedback on the per-agent merge in client.js: - **Run-wide semantics, named explicitly.** The merge into a single `Graph.sessions[EXECUTE_CODE]` was a deliberate match to the agents-library design (`Graph.sessions` is shared across every `ToolNode` in the run), but the inline `for (const a of agents)` loop in `AgentClient.chatCompletion` made it look per-agent. Move the logic to a TS helper `buildInitialToolSessions` that documents the run-wide-by-design contract in one place. The CJS controller now contains a single call site, no business logic. - **Subagent walk (P2).** The previous loop only iterated `[primary, ...agentConfigs.values()]`. Pure subagents are pruned out of `agentConfigs` after init and retained on each parent's `subagentAgentConfigs`, so their primed code files were silently dropped from the seed. The helper now walks recursively, with a visited-Set keyed on object identity that terminates safely on a malformed agent graph (cycle). - **`jest.setup.cjs` polyfill for undici `File`.** Reviewer hit `ReferenceError: File is not defined` running the targeted spec on WSL — a known Node 18 issue where `globalThis.File` from `node:buffer` isn't auto-exposed. Polyfill it inside a Jest setup file so the suite boots regardless of Node patch version. Helper test coverage (8 new): skill-only / agent-only / both, recursive subagent walk, cycle-safe walk, primary+subagent deduplication, undefined/null entries in the agents iterable, and representative session_id preservation across the merge. 16 tests pass total in `codeFilesSession.spec.ts` (8 prior + 8 new). No behavior change vs. the previous commit for the existing primary+agentConfigs case — subagent inclusion is the only new behavior, and it matches what the existing seeding logic would have done if subagents had been in `agentConfigs`. * 🪛 fix: FIFO Walk Order in buildInitialToolSessions (P3 review) The traversal used `Array.pop()` (LIFO), which visited the LAST top-level agent first. The docstring says "primary first"; the code contradicted it. When no skill seed exists the first-visited agent's first file supplies the representative `session_id` written to `Graph.sessions[EXECUTE_CODE]` — so a LIFO walk silently flipped which agent that came from. `ToolNode` ultimately uses per-file `session_id`s for runtime injection (so behavior was indistinguishable for current callers), but the discrepancy was a footgun for any future consumer that read the representative. Switch to FIFO via `Array.shift()` to match both the docstring and the existing `loadSubagentsFor` walk pattern in `Endpoints/agents/initialize.js`. Add a regression test that asserts the primary's `session_id` is the representative (and that all three agents' files still contribute, with per-file `session_id`s preserved). * 🔬 test: Lock In Code-Files Bug Fixes Per Comprehensive Review Addresses MAJOR + MINOR + NIT findings from the multi-pass review: **Finding #4 (MINOR) — empty relativePath misses sandbox fallback.** A model calling `read_file("output/")` where "output" isn't a skill name dead-ended with `Missing file path after skill name` instead of being routed to the sandbox like every other malformed-path branch. Add the same `codeEnvAvailable → handleSandboxFileFallback` pattern, plus two regression tests. **Finding danny-avila#7 (NIT) — duplicate `skillsInScope()` helper.** Hoist the identical helper out of two nested describe blocks to module scope. Single source of truth. **Finding #1 (MAJOR) — `persistedMessageId` had zero test coverage.** The fix preserves a file's original `messageId` on update so `getCodeGeneratedFiles` can still match it on subsequent turns. A regression in the `isUpdate ? (claimed.messageId ?? messageId) : messageId` ternary would silently re-introduce the original cross-turn priming bug. Five new tests cover: - UPDATE preserves `claimed.messageId` in the persisted record - UPDATE falls back to current run id when `claimed.messageId` is absent (legacy records predating the field) - CREATE uses current run id (no claimed record exists) - The runtime return value uses the LIVE id (artifact attribution) even when the persisted record kept the original - The image branch follows the same contract (would silently regress if the ternary diverged across the two file-build branches) The tests use a `snapshotCreateFileArgs()` helper because `processCodeOutput` mutates the file object after `createFile` returns (`Object.assign(file, { messageId, toolCallId })`) and a naive `createFile.mock.calls[0][0]` would reflect the post-mutation state instead of what was actually persisted. **Finding #2 (MAJOR) — `readSandboxFile` had no direct tests.** The model-controlled `file_path` flows through a POSIX single-quote escape into a shell `cat` command, making this a security boundary. A quoting regression would let a malicious filename break out of the quoted argument and inject arbitrary shell. 20 new tests across: - Shell quoting (7): plain filenames, embedded `'`, `$()`, backticks, newlines, shell metachars, multiple consecutive single-quotes - Payload shape (6): /exec URL, bash language, conditional session_id / files inclusion, dedicated keepAlive:false agents - Response handling (6): `{content}` on success, null on missing base URL or absent stdout, throws on stderr-only, partial-success returns stdout, transport errors are logged then rethrown - Timeout (1): matches processCodeOutput's 15s SLA Audited findings danny-avila#5 (acknowledged tech debt — readSandboxFile in JS workspace), danny-avila#6 (pre-existing positional-args debt on enrichWithSkillConfigurable), and danny-avila#8 (cosmetic JSDoc style) — no action taken per the reviewer's own assessment. Audited finding #3 (walk order vs docstring) — already addressed in commit 007f323 which converted to FIFO via `queue.shift()` plus a regression test. The audit was performed against an earlier PR head. Tests: 152 packages/api + 195 api JS = 347 pass. Typecheck clean. * 🪛 fix: Pure-Subagent codeEnv + Primed-Skill Routing + ToolService Early Returns Three findings from the second-pass review: **P2 — Pure subagents missed `codeEnvAvailable`** (initialize.js). The pure-subagent init path didn't forward the endpoint-level `codeEnvAvailable` flag to `initializeAgent`, unlike the primary, handoff, and addedConvo paths. A code-enabled subagent loaded only through `subagentAgentConfigs` initialized with `codeEnvAvailable: false`, so even though the recursive seed walk found its primed code files, the subagent's own `bash_tool` / `read_file` sandbox fallback were silently gated off. Forward the flag and add `codeEnvAvailable: config.codeEnvAvailable` to the `agentToolContexts.set` for symmetry with the other paths. **P2 — Primed skills outside the catalog cap were misrouted to sandbox** (handlers.ts). Manual ($-popover) and always-apply primes are intentionally resolved off the wider `accessibleSkillIds` ACL set BEFORE catalog injection — see `resolveManualSkills` for why a skill outside the `SKILL_CATALOG_LIMIT` cap can still be authorized for direct manual invocation. The `activeSkillNames` shortcut ran before reading `skillPrimedIdsByName`, so a primed skill not in the catalog would fall through to the sandbox instead of resolving via the pinned `_id`. Read the primed map first and bypass the shortcut for primed names. New regression test asserts a primed-but-not- cataloged skill resolves through the existing skill path with `getSkillByName` invoked and `readSandboxFile` NOT called. **P3 — `loadAgentTools` early returns dropped `primedCodeFiles`** (ToolService.js). The non-`definitionsOnly` path captures the field correctly, but two early-return branches (no-action-tools fast path, no-action-sets fast path) omitted it. Any traditional `loadAgentTools(..., definitionsOnly: false)` caller using execute_code without action tools would have its first-call session seed silently empty. Add `primedCodeFiles` to both early returns for consistency with the final return shape. Tests: 153 packages/api + 195 api JS = 348 pass. * 🧹 chore: Document jest.mock arrow-indirection pattern in process.spec.js Per the second-pass review's Finding #2 (NIT, "would help future readers"): the mock setup mixes direct `jest.fn()` references with arrow-function indirection (`(...args) => mockX(...args)`). The indirection isn't stylistic — it's required because `jest.mock(...)` is hoisted above the outer `const` declarations at parse time, so a direct reference would capture `undefined`. Inline comment explains the pattern so the next reader doesn't have to reverse-engineer it or accidentally "simplify" the mocks and break per-test `mockReturnValueOnce` / `mockImplementationOnce` overrides. * 🪛 fix: Five Issues from Pass-N + Codex Review (incl. 404 root cause) Five real bugs surfaced by another review pass + Codex PR comments + the codeapi-side logs we collected during manual testing: **1) `processCodeOutput` 404 root cause (`callbacks.js`).** The codeapi worker emits TWO distinct `session_id`s on a tool result: - `artifact.session_id` is the EXEC session — the sandbox VM that ran the bash command. Files don't live there; it's torn down post-execution. - `file.session_id` is the STORAGE session — the file-server bucket prefix where artifacts actually live. `callbacks.js` was passing the EXEC id to `processCodeOutput`, which builds `/download/{session_id}/{id}` and 404s because the file-server doesn't know about that path. This explains every "Error downloading/processing code environment file" we saw during testing. Use `file.session_id ?? output.artifact.session_id` (per-file id with artifact-level fallback for older worker payloads). **2) `primeFiles` reupload pushed STALE sandbox ids (`process.js`).** When `getSessionInfo` returns null (file expired/missing in sandbox), `reuploadFile` re-uploads via `handleFileUpload`, gets a NEW `fileIdentifier`, and persists it on the DB record. But `pushFile` was a closure capturing the OLD `(session_id, id)` parsed at the top of the loop, so the in-memory `files[]` array (now consumed by `buildInitialToolSessions` to seed `Graph.sessions`) silently referenced a sandbox object that no longer existed. The first tool call would 404 trying to mount it; only the next turn's metadata re-read would correct course. Parameterize `pushFile` with optional `(session_id, id)` overrides; in `reuploadFile` parse the new identifier and pass through. 2 regression tests. **3) Codex P2 — Cap sandbox fallback output before line-numbering (`handlers.ts`).** The new `handleSandboxFileFallback` returned `addLineNumbers(result.content)` without a size guard, so reading a multi-MB `/mnt/data/*` artifact materialized the file twice in memory (raw + line-numbered) before downstream truncation. Match the existing skill-file path's `MAX_READABLE_BYTES` (256KB): truncate raw first, then number, surface the truncation to the model so it can use `bash_tool` (`head` / `tail`) for the rest. 2 tests (oversized truncates with hint, in-cap doesn't). **4) Codex P2 — Dedupe seeded code files by `(session_id, id)` (`codeFilesSession.ts`).** Multiple agents in a run commonly carry the same primed execute-code resources (shared conversation files); without dedupe, `_injected_files` grows proportionally to agent count and bloats every `/exec` POST. Use a `(session_id, id)` identity key so first-seen wins (preserves source ordering); name alone isn't sufficient because two distinct primed uploads can share a filename across different sessions. 4 tests covering dedup across iterations, against pre-existing entries, name-collision distinct-session preservation, and the multi-agent realistic case in `buildInitialToolSessions`. **5) Pass-N P2 — Polyfill `globalThis.File` in api Jest setup (`api/test/jestSetup.js`).** `packages/api/jest.setup.cjs` had the polyfill; the legacy api workspace's Jest config has its own `setupFiles` that didn't, so on Node 18 / WSL the api focused tests still failed at import time with `ReferenceError: File is not defined` from undici. Mirror the polyfill. Tests: 159 packages/api + 206 api JS = 365 pass. Typecheck clean. * 🔧 chore: Update @librechat/agents dependency to version 3.1.73 Bumps the version of the @librechat/agents package across package-lock.json and relevant package.json files to ensure compatibility with the latest features and fixes.

…2854) * 🪟 feat: Render Source-Code Artifacts in the Side Panel (CODE bucket) PR danny-avila#12832 wired markdown / mermaid / html / .jsx-tsx tool outputs through the side-panel artifact pipeline but explicitly punted on code files: > Everything else (csv, py, json, xls/docx/pptx, …) keeps PR danny-avila#12829's > inline behaviour — dedicated viewers will land in follow-ups. This adds the code-file viewer. A `simple_graph.py` (and every other common source file) now opens in the side panel alongside markdown, mermaid, html, and react artifacts instead of falling back to the inline `<pre>` rendering. **Design.** New `CODE: 'application/vnd.code'` bucket reuses the static- markdown sandpack template — `useArtifactProps` pre-wraps the source as a fenced code block (` ```python\n...\n``` `) before handing it to `getMarkdownFiles`. The fence carries a `language-<x>` class through `marked`, so a future highlighter swap-in (e.g. drop `highlight.js` into the markdown template) picks up syntax colors automatically. The `react-ts` (sandpack) template's React boot cost is avoided since source files don't need it. **Single source of truth for languages.** New `CODE_EXTENSION_TO_LANGUAGE` map drives BOTH: - `EXTENSION_TO_TOOL_ARTIFACT_TYPE` routing (presence in this map = code file). Adding a new language is one entry. - The fenced-block language hint (exported as `languageForFilename`). Identifiers follow the GitHub / `highlight.js` convention so the future highlighter pickup is automatic. **Scope.** Programming languages + stylesheets + shell + sql/graphql + build files (Dockerfile/Makefile/HCL). Pure data formats (CSV/TSV/JSON/JSONL/NDJSON/XML/YAML/TOML) and config dotfiles (`.env`/`.ini`/`.conf`/`.cfg`) are intentionally NOT routed in this pass — they're better served by dedicated viewers (CSV table view, etc.) or remain inline. Adding them later is a one-entry change in the map. **JSX/TSX kept on the React (sandpack) bucket.** They're React component sources; the existing live-preview should win over the static CODE bucket. Plain `.js`/`.ts` source goes through CODE. **MIME-type fallback.** The codeapi backend serves `text/x-python`, `text/x-typescript`, etc. as `Content-Type` for source files, so a file whose extension was stripped/renamed upstream still routes to CODE via the MIME map. **Empty-text gate.** CODE joins MARKDOWN/PLAIN_TEXT in the empty-text exception (an empty `.py` is still a Python file). HTML/REACT/MERMAID still require text — their viewers (sandpack/mermaid.js) error on empty input. **Files changed:** - `client/src/utils/artifacts.ts` — `CODE` bucket constant, `CODE_EXTENSION_TO_LANGUAGE` map, exported `isCodeExtension` and `languageForFilename` helpers, extension/MIME routing additions, template + dependencies entries, empty-text gate exception, helper hoisting (extensionOf / baseMime moved up so the language map can reference them). - `client/src/hooks/Artifacts/useArtifactProps.ts` — exported `wrapAsFencedCodeBlock`, CODE branch that wraps the source then routes through `getMarkdownFiles`. **Tests (+22):** - 8 parameterized routing cases (.py, .js, .go, .rs, .css, .sh, .sql, .kt) verify the CODE bucket fires. - Extension wins when MIME is generic octet-stream (Python has no magic bytes; common case). - Regression: jsx/tsx STAY on REACT bucket (no live-preview regression). - Regression: data formats (CSV/JSON/YAML/TOML) and config dotfiles (.env/.ini) do NOT route to CODE. - Empty-text exception for CODE (empty Python file is still a Python file). - `useArtifactProps`: CODE → content.md / static template, fenced-block shape, language hint, unknown-extension fallback to raw extension, no-extension empty hint, index.html via markdown template. - `wrapAsFencedCodeBlock`: language hint, empty hint, single-trailing- newline trim, multi-newline preservation, empty-source emit. 87/87 in artifact-impacted tests; 155/155 across the broader artifact suite. No regressions in pre-existing markdown/mermaid/HTML/REACT/text behavior. * 🛡️ fix: Bare-filename routing + adaptive fence delimiter (codex P2 ×2) Two follow-ups from Codex review on the CODE bucket: 1. **Bare-filename routing for extensionless build files (Codex P2).** `Dockerfile`, `Makefile`, `Gemfile`, `Rakefile`, `Vagrantfile`, `Brewfile` have no `.` in their basename — `extensionOf` returns `''` and the extension map can't match, so they fell through to inline rendering despite being in `CODE_EXTENSION_TO_LANGUAGE`. New `bareNameOf` helper returns the lowercased basename for extensionless filenames (returns `''` for files with a `.` so the extension and bare-name paths don't double-match). Both `detectArtifactTypeFromFile` and `languageForFilename` consult it as a second lookup against the same `CODE_EXTENSION_TO_LANGUAGE` map, so adding a new build file is one entry. Path-aware: takes the basename so `proj/Dockerfile` (path-preserving sanitizer output) still routes correctly. Added the four extra Ruby build-script names while I was here. 2. **Adaptive fence delimiter (Codex P2).** A hardcoded ` ``` ` fence breaks when the source contains a line starting with ` ``` ` — for example, a JS file containing a markdown-shaped template literal: const md = ` ``` hello ``` `; CommonMark closes a fence on a line whose backtick run matches-or- exceeds the opener, so `marked` would close the outer fence at the inner `\`\`\`` and the rest of the file would render as markdown — corrupting the artifact and potentially altering formatting / links outside `<code>`. New `longestLeadingBacktickRun(source)` scans for the longest start-of-line backtick run in the payload. Fence length = `max(3, longest + 1)` — strictly more than any internal run, so `marked` can never close the outer fence early. Only escalates when needed; the common case still uses a triple-backtick fence. Inline backticks (mid-line) don't count — they're not fence delimiters. Only column-zero runs trigger escalation, so e.g. a Python file with ` `inline ``` here` ` keeps the 3-fence. +11 regression tests: - 8 parameterized cases: `Dockerfile`/`Makefile`/`Gemfile`/etc. route to CODE via bare-name fallback (case-insensitive on basename). - Path-aware: `proj/Dockerfile` recognized. - No double-match: `dockerfile.dev` (with extension) returns null. - Unknown extensionless files (`README`, `LICENSE`) stay null. - 4-backtick fence when source has ` ``` ` at start-of-line. - 5-backtick fence when source has ` ```` ` at start-of-line. - 3-backtick fence (default) for ordinary code. - Inline backticks don't escalate. - Source starting with backtick run at offset 0. Plus 6 new `languageForFilename` tests covering bare-name fallback and path-awareness. 108/108 in artifact-impacted tests (was 87, +21 tests). No regressions. * 🛡️ fix: Indented fence detection + basename-scoped extensionOf (codex P2/P3) Two follow-ups from the latest Codex review on the CODE bucket: 1. **Indented backtick runs (Codex P2).** `longestLeadingBacktickRun` was scanning `^(`+)` — column 0 only. CommonMark allows fence closers to be indented up to 3 spaces, so a JS source containing an indented `\`\`\`` (e.g. inside a template literal embedded in a class method) would still terminate our outer fence and the remainder would render as markdown. Updated regex to `^ {0,3}(`+)`. Tabs are not allowed in fence indentation (CommonMark expands them to 4 spaces, which is over the 3-space limit), so spaces alone suffice. Backticks indented 4+ spaces are CommonMark "indented code blocks" — they can't terminate a fence, so we correctly don't escalate for them. 2. **`extensionOf` path-laden output (Codex P3).** `extensionOf` took `lastIndexOf('.')` across the FULL path string, so `pkg.v1/Dockerfile` yielded the nonsensical "extension" `v1/dockerfile`. `languageForFilename` returned that as the language hint (broken `language-v1/dockerfile` class on the fenced block), AND the routing's bare-name fallback couldn't fire because the extension lookup returned non-empty. New `basenameOf` helper strips path separators; `extensionOf` and `bareNameOf` both go through it. After the fix: - `pkg.v1/Dockerfile` → `extensionOf` returns `''` → `bareNameOf` returns `dockerfile` → routes to CODE with correct language. - `pkg.v1/main.go` → `extensionOf` returns `go` → routes correctly. - `pkg.v1/script.py` → `extensionOf` returns `py` → routes correctly. +10 regression tests: - 5 parameterized cases covering 1-3 space indent at fence lengths 3, 4, 5 (escalation kicks in correctly). - 4-space indent does NOT escalate (CommonMark indented-code-block territory; can't close a fence). - `pkg.v1/Dockerfile` and `a.b.c/Makefile` route to CODE + `languageForFilename` returns `dockerfile`/`makefile`. - Dotted-directory files (`pkg.v1/main.go`, `a.b.c/script.py`) still route correctly via the basename-scoped extension parse. 118/118 in artifact-impacted tests (was 108, +10 tests). No regressions. * 🛡️ fix: Comprehensive review polish + MIME-derived language hint (codex P3) Resolves all 8 valid findings from the comprehensive review and the follow-up Codex P3 on the same PR. None are user-visible bugs; the set spans correctness guards, dead-code removal, organization, and test coverage. **Comprehensive review #1 — Remove dead `isCodeExtension` export.** Function was exported with zero callers anywhere in the codebase. **Comprehensive review #2 — Guard the for-loop against silent overwrites.** The `for (ext of CODE_EXTENSION_TO_LANGUAGE)` loop blindly assigned each language extension to the CODE bucket. If a future contributor added `jsx` or `tsx` to the language map (a natural mistake — they ARE source code), the loop would silently overwrite the REACT bucket entries and break the sandpack live-preview with no compile-time or runtime error. Added `if (ext in EXTENSION_TO_TOOL_ARTIFACT_TYPE) continue` so explicit map entries always win. **Comprehensive review #3 — Add `fileToArtifact` end-to-end test for CODE.** Routing was tested via `detectArtifactTypeFromFile`; full Artifact construction (id / type / title / content / messageId / language) for CODE was not. Added 5 new `fileToArtifact` cases. **Comprehensive review #4 — Move pure utilities out of the hook file.** `wrapAsFencedCodeBlock` and `longestLeadingBacktickRun` are pure string transformations with no React dependencies. Moved both to `utils/artifacts.ts`. Test files updated to import from the new location. **Comprehensive review danny-avila#5 — Correct the MIME-map "mirrors" comment.** Comment claimed the MIME map mirrored `CODE_EXTENSION_TO_LANGUAGE`, but covered ~21 of ~60 entries. Reworded to "best-effort COMMON-CASE list, not an exhaustive mirror" with the rationale (extension routing is primary; MIME is a stripped-filename fallback). **Comprehensive review danny-avila#6 — Drop `lang ? lang : ''` ternary.** `lang` is typed `string`; the only falsy value is `''`. Removed. (Replaced via the MIME-fallback rewrite of `wrapAsFencedCodeBlock`, where `lang` is now used directly without the ternary.) **Comprehensive review danny-avila#7 — Avoid double `basenameOf` computation.** `extensionOf(filename)` and `bareNameOf(filename)` both internally called `basenameOf` — when the extension lookup missed, `detectArtifactTypeFromFile` paid for two parses of the same path. Split into private `extensionFromBasename` / `bareNameFromBasename` helpers; the caller computes `basenameOf` once and threads it through. **Comprehensive review danny-avila#8 — Trim verbose Dockerfile/Makefile comment.** Inline comment block in the language map duplicated `bareNameOf`'s JSDoc. Replaced with a one-line pointer. **Codex P3 — MIME fallback for the CODE language hint.** `detectArtifactTypeFromFile` routes `{ filename: 'noext', type: 'text/x-python' }` to CODE via the MIME bucket map, but then `useArtifactProps` derived the language hint from `artifact.title` ONLY — and `noext` has no extension, so `languageForFilename` returned empty and the fenced block emitted with no `language-` class. The future highlighter swap-in would lose syntax-color metadata for these files. - New `MIME_TO_LANGUAGE` map covering the language MIMEs codeapi actually emits. - `languageForFilename(filename, mime?)` now takes an optional MIME second arg and falls back to it after the extension and bare-name paths. - `fileToArtifact` resolves the language at construction time (using both filename AND `attachment.type`) and stores it on `artifact.language`. The hook reads `artifact.language` directly rather than re-deriving from `title` alone, so the MIME signal survives end-to-end. - Title-derived fallback in the hook covers older callers that don't populate `language`. Tests: +10 cases for the comprehensive review findings (CODE end-to-end via `fileToArtifact`, language storage, non-CODE language un-set). +6 cases for the MIME fallback (`languageForFilename(name, mime)` ordering, MIME parameter stripping, extension/bare-name vs MIME precedence, empty signal). +2 hook tests for `artifact.language` pre-resolved vs title-fallback. 131/131 in directly-impacted files (was 118, +13). 199/199 across the broader artifact suite. No regressions. Pre-existing TypeScript errors in `a11y/`, `Agents/`, `Auth/`, `Mermaid.tsx`, etc. are unrelated to this PR (verified by checking `tsc --noEmit` on origin/dev — same errors).

…avila#12934) * 📄 feat: Rich File Artifact Previews for DOCX, CSV, XLSX, PPTX Render office files emitted by tools as interactive previews in the artifact panel instead of raw extracted text. The backend produces a sanitized HTML document via mammoth (DOCX), SheetJS (CSV/XLSX/XLS/ODS), or yauzl-based slide extraction (PPTX) and ships it through the existing SSE attachment payload; the client routes it through the Sandpack `static` template's `index.html` slot — no new browser deps, no client-side blob fetch, no React renderer components. * 🔐 fix: Restrict data: URLs to <img> in office HTML sanitizer Codex review on #12934 caught that `data:` lived in the global `allowedSchemes`, which meant a smuggled `<a href="data:text/html, <script>...</script>">` would survive sanitization. The Sandpack iframe sandbox does not gate `target="_blank"` navigations, so a click would open attacker-controlled HTML in a new tab. Scope `data:` to `<img src>` only via `allowedSchemesByTag` (mammoth inlines DOCX images as base64 `data:image/...` URIs — that path still works). Add a regression suite (`sanitizeOfficeHtml security`) with 8 cases covering: <script> stripping, event-handler removal, javascript:/data: rejection on anchors, data:image preservation in <img>, http/https/mailto allowance, target=_blank rel=noopener enforcement, and <iframe> stripping. * 🔧 fix: Route extensionless office files by MIME alone Codex review on #12934 caught that the office-render gate in `extractCodeArtifactText` only fired when the extension was in `OFFICE_HTML_EXTENSIONS` or the category was `document`/`pptx`. A tool emitting `data` with `text/csv` (no extension) classifies as `utf8-text`, so the gate was skipped and raw CSV text shipped to the client — but the client routes by MIME to the SPREADSHEET bucket expecting a full HTML document, so the panel rendered broken text. Extract a shared `officeHtmlBucket(name, mime)` predicate from `html.ts` (returns the bucket name or null). Both `bufferToOfficeHtml` (the dispatcher) and the upstream gate in `extract.ts` now go through this single source of truth, so they can never drift apart again. The predicate already mirrors the dispatcher's extension/MIME logic (extension wins; MIME is the fallback for extensionless inputs). Adds: - 14 cases for the new `officeHtmlBucket` predicate covering the positive paths (each bucket via extension OR MIME) and the negative paths (txt, py, json, jpg, pdf, zip, odt, plain noext). - A direct regression test in `extract.spec.ts` for the Codex catch: `data` with `text/csv` + utf8-text category routes through the office HTML producer. - Parameterized cases for extensionless DOCX/XLSX/XLS/ODS/PPTX files identified by MIME alone. * 🛡️ fix: Enforce extension-wins precedence in officeHtmlBucket Codex review on #12934 caught that the predicate's if-chain interleaved extension and MIME checks for each bucket — e.g. CSV's branch was `ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)`. A `deck.pptx` shipped with `text/csv` (sandboxed tools sometimes ship generic MIMEs) matched the CSV branch BEFORE the PPTX extension branch was reached, so a binary PPTX would have been handed to `csvToHtml` to parse as text — yielding garbage or a parse exception. Restructure to a strict two-pass dispatch: an exhaustive extension table first (one lookup, all known extensions), then MIME-only fallback for extensionless / unknown-ext inputs. The doc comment's "extension wins" claim is now actually enforced by the implementation. Add 7 regression cases covering the conflicting-MIME footgun for each bucket: deck.pptx + text/csv → pptx; workbook.xlsx + text/csv → spreadsheet; legacy.xls + pptx-MIME → spreadsheet; report.docx + text/csv → docx; data.csv + docx-MIME → csv; etc. * 🛡️ fix: Reject zip-bomb office files before in-process parsing (SEC) Addresses pre-existing availability vulnerability validated by SEC review (Codex finding 275344c5...) and made worse by this PR's HTML rendering path. A sub-1MiB compressed XLSX/DOCX/PPTX (highly compressed run-of-zeros) inflates to 200+ MiB of XML when handed to mammoth/xlsx — blocking the Node event loop for 10+ seconds and spiking RSS to ~1 GiB. The existing 8s `withTimeout` wrapper uses `Promise.race`, which can only return early; it cannot interrupt synchronous parser CPU/RAM consumption. PoC ran an authenticated execute_code call to OOM the API process. Add `assertSafeZipSize(buffer)` — a yauzl-based pre-flight that streams every entry with mid-inflate byte counting and bails on either a per-entry or total decompressed-size cap. Mid-inflate counting cannot be bypassed by falsifying the central directory's `uncompressedSize` field (the technique the PoC used). Defaults: 25 MiB per entry, 100 MiB total — generous headroom for legitimate image-heavy office files, well below the attack profile. Hook the check into every path that hands a buffer to mammoth/xlsx /yauzl: - New HTML producers (`wordDocToHtml`, `excelSheetToHtml`, `pptxToSlideListHtml`) — added by this PR - Legacy RAG text extractors (`wordDocToText`, `excelSheetToText` in `crud.ts`) — pre-existing path, also vulnerable Errors propagate as a tag-distinct `ZipBombError` so callers can distinguish a refused bomb from generic parse failures. The outer `extractCodeArtifactText` swallows the error and returns null, falling back to the regular download UI. `.xls` (BIFF/CFB binary, not ZIP) is detected by magic bytes and skipped — yauzl would reject it as malformed anyway. Adds 15 tests: - `zipSafety.spec.ts` (9): benign passes, per-entry cap, total cap, ZipBombError type-tagging, malformed-zip distinction, directory- entry handling, named-error surfacing, and the SEC-PoC pattern (sub-1 MiB compressed → 50 MiB inflated rejected on default caps). - `html.spec.ts` zip-bomb suite (5): each producer rejects a bomb; dispatcher propagates correctly; legitimate fixtures still render. - `extract.spec.ts` (1): outer extractor swallows ZipBombError and returns null so the download UI fallback fires. * 🧹 fix: Normalize MIME parameters; add legacy CSV MIME variant Two related Codex catches on PR #12934 — both about MIME-routing inconsistencies between backend and client that would cause extensionless CSV files to render as broken (raw text under an HTML slot) or skip the artifact panel entirely. P2 — backend MIME normalization: `officeHtmlBucket` matched MIME strings exactly, so a real-world `text/csv; charset=utf-8` Content-Type slipped through and the backend returned raw CSV text. The client's `baseMime` helper strips parameters before its own MIME lookup, so it routed the same file to the SPREADSHEET bucket expecting an HTML body that never arrived. Mirror the client's normalization on the backend (strip everything from `;` onward, lowercase) before bucket matching. P3 — client legacy CSV MIME: Backend's `CSV_MIME_PATTERN` accepts three variants (`text/csv`, `application/csv`, `text/comma-separated-values`); the client's `MIME_TO_TOOL_ARTIFACT_TYPE` only had the first two. An extensionless file with `text/comma-separated-values` would have backend HTML produced but the client would skip the artifact panel entirely. Add the missing variant. Tests: - 9 new parameterized-MIME cases on backend covering charset/ boundary/case variants for every bucket. - 1 new client routing case for `text/comma-separated-values`. * 🩹 fix: Try office HTML before short-circuiting on category=other Codex review on #12934 caught that the early `category === 'other'` return short-circuited before `hasOfficeHtmlPath` was checked. The classifier returns 'other' for inputs the new dispatcher can still route — extensionless `application/csv` (CSV MIMEs aren't in the classifier's text-MIME set and don't start with `text/`), and extensionless office MIMEs with parameters like `application/vnd... spreadsheetml.sheet; charset=binary` (the classifier's `isDocumentMime` exact-matches these MIMEs without parameter normalization). Both would route correctly through `officeHtmlBucket` but never reached it. Move the office-HTML attempt above the 'other' early return, and drop the `|| category === 'document' || category === 'pptx'` shortcut now that `hasOfficeHtmlPath` covers the same surface (with parameter normalization) and a wider one. ODT still routes through `extractDocument` unchanged — `hasOfficeHtmlPath` returns false for it and the `category === 'document'` branch below handles it. Adds 3 regression tests: - extensionless `application/csv` + category='other' → office HTML - extensionless parameterized office MIME + category='other' → office HTML - defense check: actual binary 'other' (image/jpeg) still returns null without invoking the office producer * 🛡️ fix: Office types are HTML-or-null (no text fallback → XSS) Codex P1 review on #12934 caught that when `renderOfficeHtml` failed (timeout, malformed file, zip-bomb rejection) for an office type, the extractor fell through to `extractDocument` and returned plain text. The client routes by extension/MIME to the office preview buckets and feeds `attachment.text` straight into the Sandpack iframe's `index.html`. A spreadsheet cell or document body containing the literal string `<script>alert(1)</script>` would have been injected as executable markup — direct XSS. The contract for office types is now HTML-or-null with no text fallback. Failed render returns null, the client's empty-text gate keeps the artifact off the panel, and the file falls back to the regular download UI (matching what PPTX already did). PDF and ODT still go through `extractDocument` because the client routes them to PLAIN_TEXT (which the markdown viewer escapes) or no artifact at all, so plain text is safe there. Test reshuffle: - `document` describe block now uses ODT/PDF for the legacy parseDocument-path tests (DOCX/XLSX/XLS/ODS bypass that path). - New "does NOT call parseDocument for office HTML types" test locks in the SEC contract for all four office HTML buckets. - "falls back to ..." tests rewritten as "returns null when ..." with explicit `parseDocumentCalls.length === 0` assertions to prove no text leaks back to the client. - New XSS regression test for the XLSX failure path. - Mock parseDocument failure-name match relaxed to `includes()` so ODT-named tests can use the same trigger. * 🧽 chore: Address follow-up review findings on PR #12934 Wraps up the 10-finding follow-up review. Two MAJOR + four MINOR + two NIT addressed; one NIT skipped after verifying it was a misread of the package.json structure. MAJOR - #1: Rewrite `renderOfficeHtml` JSDoc to document the HTML-or-null contract explicitly. The pre-fix doc described a text-fallback path that was the original XSS vector (commit b06f08a). A future maintainer trusting the stale doc could reintroduce the fallback. - #2: Replace byte-truncation of office HTML with a small "preview too large" banner document. Cutting at a UTF-8 boundary lands mid-tag (`<table><tr><td>con\n…[truncated]`) and ships malformed markup to the iframe — unpredictable rendering, occasional broken layouts on DOCX with embedded images / wide spreadsheets. MINOR - #4: Wrap `readSlidesFromZip`'s `zipfile.close()` in try/catch so a close-time exception (mid-flight stream) doesn't replace the original error. Mirrors the defensive pattern in zipSafety.ts. - #5: Refactor PPTX extraction to use `yauzl.fromBuffer` directly, eliminating the temp-file write/unlink the safety pre-flight already proved unnecessary. Removes 4 unused imports (os, path, fs/promises, randomUUID). - #6: Extract `isPreviewOnlyArtifact(type)` to `client/src/utils/ artifacts.ts` so the membership check is unit-testable without mounting the full Artifacts component (Recoil + Sandpack + media query). 15 new test cases covering positive types, negative types, null/undefined, and unknown strings. NIT - #3: Remove dead `stripColorStyles` / `COLOR_PROPERTY_PATTERN` — unused (sanitizer's `allowedStyles` config handles color implicitly). - #7: Remove dead `!_lc_csv_label` worksheet property write. - #9: Remove no-op `exclusiveFilter: () => false` sanitize-html config. - #10: Type-narrow `PREVIEW_ONLY_ARTIFACT_TYPES` to `ReadonlySet<ToolArtifactType>` so the membership table is compile-time checked against the enum. SKIPPED - #8: Reviewer flagged `sanitize-html` as duplicated in devDeps and dependencies. The package has no `dependencies` section — only `devDependencies` and `peerDependencies`. Existing convention (mammoth, xlsx, yauzl, pdfjs-dist) is to appear in BOTH. Removing the devDep entry would break local test runs. Tests: packages/api 4406/4406, client artifacts 128/128. * 🪞 chore: Fix isPreviewOnlyArtifact test description parameter order Follow-up review nit on PR #12934. Jest's `it.each` substitutes `%s` positionally, and the table rows were `[type, expected]` while the description template read `'returns %s for type %s'` — outputting "returns application/vnd.librechat.docx-preview for type true" instead of the intended "type ... returns true". Reorder the template to match the column order. Test runner output now reads naturally: "type application/vnd.librechat.docx-preview returns true". Pure cosmetic — runtime behavior unchanged. * ✨ feat: Improve DOCX rendering and surface filename in panel header Two UX improvements based on hands-on use of the office preview pipeline. DOCX rendering — mammoth strips the navy banners, cell shading, and column layouts that direct-formatted docs apply (python-docx-style output is a common case). The flat `<p><strong>X</strong></p>` and bare `<table><tr><td>` it emits looks washed out next to the source. Three targeted compensations: - Style map promotes `Title`, `Subtitle`, `Heading 1` thru `Heading 6`, and `Quote` paragraphs to their semantic HTML equivalents (mammoth's default only handles Heading 1-6, missing Title/Subtitle/Quote). - Extra CSS scoped to `.lc-docx` gives the first table row sticky- looking header styling regardless of `<thead>` (mammoth never emits `<thead>`), adds zebra striping, and treats the python-docx `<p><strong>X</strong></p>` section-heading idiom as a pseudo-h2 with a thin accent left border so document structure survives the round trip. Headings get a left accent or underline so they read as headings instead of just bold paragraphs. - Sanitizer's `allowedAttributes` opens `class` on the heading and block tags the styleMap and CSS heuristics rely on. `<script>`, event handlers, javascript: URLs, etc. are still stripped — the existing security regression suite catches any drift. Panel header — `Artifacts.tsx` showed a generic "Preview" pill for preview-only artifacts. Single-tab Radio is a no-op; surfacing the document filename there gives the user something useful in the chrome without taking real estate. `displayFilename` handles the sandbox dotfile suffix the upload pipeline applies. Tests: html.spec.ts +1 (new CSS-emission lock), 71/71. Backend files suite 428/428. Client 308/308. * ✨ feat: High-fidelity DOCX preview via docx-preview in iframe Switch the default DOCX render path from server-side mammoth → flat HTML to client-side `docx-preview` loaded inside the Sandpack iframe. Mammoth becomes the fallback for files above the cap. Why --- The Sandpack iframe is a real browser DOM. Server-side rendering ceiling for DOCX→HTML is well below the source's visual fidelity — mammoth strips cell shading, run colors, banners, and column layouts because Word's layout model doesn't fit HTML's flow model. Pushing the render into the iframe lifts that ceiling without paying the server-side cost of jsdom or LibreOffice. What ---- - New `wordDocToHtmlViaCdn(buffer)` builds a self-contained HTML doc that embeds the binary as base64 and lets `docx-preview@0.3.7` render it on load. CSS preserves dark/light mode handoff via `prefers-color-scheme`. Bootstrap script falls back to a "preview unavailable, please download" message if the CDN is unreachable or the parse throws. - `docx-preview` and its `jszip` peer dep are pinned to specific versions on jsdelivr with SRI sha384 integrity hashes and `crossorigin="anonymous"`. Refresh: re-fetch the file, run `openssl dgst -sha384 -binary FILE | openssl base64 -A`. - CSP locked down on the iframe: `default-src 'none'`, scripts only from jsdelivr (no eval), `connect-src 'none'` so a parser bug in docx-preview can't be turned into exfiltration of the embedded document, `base-uri 'none'`, `form-action 'none'`. Defense in depth on top of the Sandpack cross-origin sandbox. - `wordDocToHtml` dispatches by size: ≤ 350 KB binary → CDN path (high fidelity), larger → mammoth fallback (preserves the size cap on `attachment.text`). 350 KB chosen so worst-case base64-inflated output (~478 KB) plus wrapper overhead (~5 KB) fits under MAX_TEXT_CACHE_BYTES (512 KB) with 40 KB headroom. - Internal renderers exported as `_internal` for tests. Public API unchanged — callers still go through `wordDocToHtml`. PPTX intentionally NOT switched ------------------------------- Surveyed the available client-side PPTX libraries: - `pptx-preview@1.0.7` ships an ESM-only main entry plus a 1.36 MB UMD that references `require("stream"/"events"/"buffer"/"util")` — bundled for Node, not browser-clean. Could work but the runtime references to undefined Node globals are a fragility risk worth more validation than this PR can absorb. - `pptxjs` is jQuery-era, requires four separate UMD scripts in a specific order, less actively maintained. - The honest answer for PPTX is the LibreOffice sidecar (DOCX/XLSX/ PPTX → PDF → PDF.js), which is the architecture every major product (Google Drive, Claude.ai, ChatGPT) effectively uses and the only path to ~5/5 fidelity for arbitrary user decks. PPTX stays on the existing slide-list extraction for now. Open a follow-up issue for the LibreOffice/Gotenberg sidecar. Tests ----- - 6 new in CDN-rendered describe block: wrapper structure, base64 round-trip, SRI integrity + crossorigin, CSP locks (connect-src/eval/base-uri/form-action), fallback message wiring, size-threshold lock. - Adjusted 2 existing tests that asserted on mammoth-path artifacts (literal document text in `<article class="lc-docx">`) — those assertions move to the mammoth-fallback test that calls `_internal.wordDocToHtmlViaMammoth` directly. Dispatcher tests now assert CDN-path signatures instead. packages/api files: 434/434 ✅, full unit suite 4473/4473 ✅. * 🧷 fix: Address Codex P1 (MIME aliases) + P2 (CDN dependency) Two follow-up review findings on PR #12934, both real. P1 — Spreadsheet MIME aliases on client ---------------------------------------- Backend's `officeHtmlBucket` uses the broad `excelMimeTypes` regex from `librechat-data-provider` (covers `application/x-ms-excel`, `application/x-msexcel`, `application/msexcel`, `application/x-excel`, `application/x-dos_ms_excel`, `application/xls`, `application/x-xls`, plus the canonical sheet MIMEs). The client's exact-match `MIME_TO_TOOL_ARTIFACT_TYPE` only had three of those, so an extensionless XLS upload with a legacy MIME would have backend HTML produced but the client would fail to route the artifact at all — preview chip never registers. Fix: import the same regex on the client and add it as a fallback in `detectArtifactTypeFromFile` after the exact-match map miss. Stays in lock-step with the backend automatically. 7 new test cases — one per legacy alias. P2 — Hard CDN dependency on jsdelivr ------------------------------------- Air-gapped / corporate-filtered networks where jsdelivr is unreachable would see DOCX previews permanently degrade to "Preview unavailable" because the iframe could never load the renderer scripts. Mammoth was sitting right there on the server but the dispatcher always preferred the CDN path for files under 350 KB. Fix: `OFFICE_PREVIEW_DISABLE_CDN` env var. When truthy (`1`, `true`, `yes`, case-insensitive, whitespace-trimmed), `wordDocToHtml` short-circuits to the mammoth path regardless of file size. Operators on filtered networks set the env var; default behavior is unchanged. Read at function-call time (not module load) so jest can flip it in `beforeEach` without `jest.resetModules()`. The cost is one property access per render. 12 new test cases: env-unset uses CDN (default), all five truthy forms force mammoth, five non-truthy forms (`false`/`0`/`no`/empty/ arbitrary string) leave CDN active. Tests ----- packages/api/src/files: 446/446 ✅ (was 434, +12 from env-var matrix). client artifact suites: 235/235 ✅ (was 228, +7 from MIME aliases). * ✨ feat: High-fidelity PPTX preview via pptx-preview in iframe Mirrors the DOCX CDN architecture for PPTX: small files (≤350 KB binary) embed as base64 and render via `pptx-preview` loaded from jsdelivr inside the Sandpack iframe. Larger files and air-gapped deployments fall back to the existing slide-list extraction. Why --- PPTX is the format where the gap between LibreChat's preview and Claude.ai-style previews was most visible (slide-list of bullet points vs. rendered slide layouts). LibreOffice → PDF → PDF.js is still the eventual gold-standard answer for PPTX fidelity, but client-side rendering inside the Sandpack iframe gets us a meaningful intermediate step (~1.5/5 → ~3.5/5) without a sidecar. What ---- - `pptx-preview@1.0.7` (ISC license, ~1.36 MB UMD bundle that includes its echarts/lodash/uuid/jszip/tslib deps inline). Pinned to a specific version on jsdelivr with SHA-384 SRI and `crossorigin="anonymous"`. - `buildPptxCdnDocument` mirrors the DOCX wrapper: same CSP locks (`default-src 'none'`, `connect-src 'none'`, no eval, no base/form tampering), same `id="lc-doc-data"` base64 slot, same fallback message wiring (`typeof pptxPreview === 'undefined'` → "Preview unavailable"). - New public `pptxToHtml(buffer)` dispatcher; `bufferToOfficeHtml` switches its `'pptx'` case to call it. `pptxToSlideListHtml` stays exported as the slide-list-only path (still hit by tests directly and by the dispatcher fallback). - `OFFICE_PREVIEW_DISABLE_CDN=true` env-var hatch applies to PPTX too — air-gapped operators get the slide-list path. Same env-var read at call time, same matrix of truthy values (`1` / `true` / `yes` / case-insensitive / whitespace-trimmed). - `_internal` re-exports moved to after the PPTX section since the PPTX internals live further down in the file. Adds `pptxToHtmlViaCdn`, `MAX_PPTX_CDN_BINARY_BYTES`, `PPTX_PREVIEW_CDN`. Honest caveats -------------- - The 1.36 MB UMD bundle has `require("stream"/"events"/"buffer"/ "util")` references in its outer wrapper. Those are bundled-dep artifacts (likely from `tslib` / Node-shim transforms) and don't appear to execute on the browser code paths, but I haven't done manual e2e on a wide range of decks. If a class of files turns up that breaks rendering, the iframe-side fallback message catches it and operators have `OFFICE_PREVIEW_DISABLE_CDN=true` as the bail. - First-render CDN fetch is ~1.36 MB (browser-cached after). - PPTX with embedded media easily exceeds the 350 KB binary cap; those files take the slide-list path. Lifting the cap is a follow-up (tied to the broader self-hosting work). Tests ----- 11 new in two new describe blocks: - `pptxToHtml dispatcher`: routing predicate (small → CDN, env-set → slide-list). - `CDN-rendered path`: base64 round-trip, SRI integrity + crossorigin, CSP locks (connect/eval/base/form), fallback message, size-threshold lock at 350 KB. - `OFFICE_PREVIEW_DISABLE_CDN escape hatch`: env-var matrix for truthy values. packages/api/src/files: 457/457 ✅ (was 446, +11). * 🪟 fix: DOCX preview fills the artifact panel width docx-preview defaults to rendering at the document's native page width (8.5in for letter, 21cm for A4). In a wide artifact panel that left whitespace on either side; in a narrow one it forced horizontal scroll. Two changes: - Pass `ignoreWidth: true` to `docx.renderAsync` so the library skips the document's pageSize width and uses its container's width. - Defensive CSS overrides on `.docx-wrapper` and `.docx-wrapper > section.docx` in case a future library version regresses on the option, plus `padding: 0` on the wrapper to drop the page-edge whitespace docx-preview otherwise reserves. `renderHeaders`/`renderFooters`/etc. stay enabled — those still appear in the rendered output, just inside a container that fills the panel instead of a fixed-width "page." Tests unchanged (100/100); manual e2e ahead of merge. * 🩹 fix: PPTX black screen — allow blob: workers + harden bootstrap Manual e2e of the PPTX CDN renderer surfaced a black screen with "Could not establish connection. Receiving end does not exist." unhandled-rejection — characteristic of a Web Worker that couldn't start. Root cause: pptx-preview's bundled echarts dep spins up Web Workers via blob: URLs for chart rendering. Our CSP had `default-src 'none'` and no `worker-src`, so workers fell back to default → blocked. The async failure deep inside echarts didn't surface through the outer `previewer.preview()` promise, so my bootstrap's `.catch` never fired, the loading state was removed, and the iframe sat with the body background showing through (dark navy in dark mode = "black screen"). Three changes: - Add `worker-src blob:` to the PPTX CSP. Allows blob:-only worker creation without permitting arbitrary worker URLs. - Bootstrap: window-level `unhandledrejection` and `error` listeners so rejections from inside bundled-dep async pipelines surface as the user-facing "Preview unavailable" fallback instead of going silent. - Bootstrap: 8-second timeout that checks `container.children.length` — if the renderer hasn't appended anything visible by then, assume silent failure and show the fallback. Also wipe `container.innerHTML` when showing the fallback so a partial render doesn't compete with the message. DOCX wrapper unchanged: docx-preview doesn't use workers, so the worker-src directive doesn't apply, and the existing fallback path already covers its failure modes. Tests ----- - Existing PPTX CSP test now also asserts `worker-src blob:` is present. - Existing fallback-message test extended to cover the new unhandledrejection/error/timeout listeners. packages/api/src/files: 467/467 ✅. * 🔒 fix: gate office HTML routing on backend trust flag (textFormat) Codex P1 review on PR #12934: routing .docx/.csv/.xlsx/.xls/.ods/.pptx into the office preview buckets assumed `attachment.text` was already sanitized full-document HTML, but that guarantee only existed for the new code-output extractor path. Existing stored attachments and other non-code paths can still carry plain extracted text — `useArtifactProps` would then inject that as `index.html` inside the Sandpack iframe. Adds a `textFormat: 'html' | 'text' | null` trust flag persisted on the file record by the code-output extractor, surfaced over the SSE attachment payload and the TFile API type. The client's routing in `detectArtifactTypeFromFile` requires `textFormat === 'html'` before landing on an office HTML bucket; everything else (legacy attachments, RAG-extracted plain text from `parseDocument`, explicitly-marked 'text' entries) falls back to the PLAIN_TEXT bucket where the markdown viewer escapes content rather than executing it. Tests: new `getExtractedTextFormat` helper has 14 cases covering all office paths, legacy XLS MIME aliases, parseDocument fallthroughs, and null-input. Client `artifacts.test.ts` adds three security-gate tests proving downgrade behavior for missing/null/'text' textFormat, plus a `fileToArtifact` test that legacy office attachments without the flag end up in PLAIN_TEXT with their content escaped. * 🌐 fix: air-gapped DOCX preview — embed mammoth fallback in CDN doc Codex P2 review on PR #12934: the CDN-rendered DOCX path always pulled docx-preview + jszip from cdn.jsdelivr.net. Air-gapped or corporate- filtered networks where jsdelivr is blocked would degrade to a static "Preview unavailable" message even though the server already had a local mammoth renderer that could produce readable output. Now the dispatcher renders mammoth first and embeds the sanitized output inside the CDN document as a hidden `#lc-fallback` block. The iframe's existing `typeof docx === 'undefined'` check (which fires when the CDN scripts can't load) un-hides the fallback so the user sees a real preview. CDN-success path is unchanged: high-fidelity docx-preview output owns the viewport, mammoth fallback stays hidden. Two new safeguards in the dispatcher: - Size budget: if base64(binary) + mammoth body + wrapper > 512 KB (the `attachment.text` cache cap), drop to mammoth-only so a giant document still renders. The `OFFICE_HTML_OUTPUT_CAP` constant mirrors `MAX_TEXT_CACHE_BYTES` from extract.ts (separate constant to avoid a circular import; pinned by a unit test). - `lc-render` is hidden when fallback shows so the empty padded slot doesn't sit above the mammoth content. Tests: existing CDN-path tests updated for the new `wordDocToHtmlViaCdn(buffer, mammothBody)` signature; new test for the embedded fallback structure (`#lc-fallback`, mammoth body content, "High-fidelity renderer unavailable" notice, render-slot hide); new constant pin and per-fixture cap-respect assertion. * 🧪 feat: LibreOffice → PDF preview path (POC, opt-in via env) Per the plan-mode discussion: prove out a LibreOffice subprocess pipeline as an alternative to the docx-preview / pptx-preview CDN renderers. LibreOffice handles every office format Microsoft and LibreOffice itself can open (DOCX, PPTX, XLSX, ODT, ODP, ODS, RTF, many more), produces a PDF, and the host browser's built-in PDF viewer renders it inside the Sandpack iframe via a `data:` URI. No client-side JS dependency, no CDN dependency, true high fidelity for any feature LibreOffice supports. Off by default. Operators opt in by setting both: - `OFFICE_PREVIEW_LIBREOFFICE=true` - LibreOffice (`soffice` or `libreoffice`) on the server's `$PATH` When either is missing, the dispatcher falls through to the existing CDN/mammoth/slide-list pipeline so a misconfiguration doesn't break previews. Hardening (`packages/api/src/files/documents/libreoffice.ts`): - Fresh subprocess per call with isolated temp dir, stripped env (PATH/HOME/TMPDIR only), and `-env:UserInstallation` so concurrent conversions can't collide on shared `~/.config/libreoffice` locks - 30-second wall-time cap; SIGKILL on timeout - 50 MB PDF output cap to bound disk pressure - 512 KB output cap on the wrapped HTML so the SSE/cache contract stays intact (base64 inflates ~33%, effective PDF cap ~380 KB) - Macros disabled by default flags (`--norestore --invisible --nodefault --nofirststartwizard --nolockcheck`) - Tag-distinct `LibreOfficeUnavailableError` / `LibreOfficeConversionError` so callers can swallow appropriately Iframe wrapper (`buildPdfEmbedDocument`): - Native browser PDF viewer via `<iframe src="data:application/pdf; base64,...">` — works in Chrome, Edge, Safari, Firefox - CSP locks the iframe to `default-src 'none'; frame-src data:; connect-src 'none'; script-src 'unsafe-inline'` — no outbound network, no eval, no external scripts - `#view=FitH` for first-paint sizing - 4-second heuristic timer that swaps to a "Preview unavailable" fallback when the browser's PDF viewer is disabled (kiosk mode, Brave Shields, etc.) Wired into `wordDocToHtml` and `pptxToHtml` as the first branch — returns null when disabled / unavailable / oversized so the existing pipeline takes over. XLSX intentionally NOT routed through this path: SheetJS's HTML output is already excellent for spreadsheets (sortable, sticky headers) and PDF rendering of sheets is awkward. Tests (`libreoffice.spec.ts`, 30 cases — 25 always run, 5 conditional on the binary): env-gating parser semantics matching `OFFICE_PREVIEW_DISABLE_CDN`, fallthrough contract (never throws, returns null on any failure), CSP lock-down, fallback structure, binary probe caching + missing-binary path, error tagging, and integration tests that engage when `soffice`/`libreoffice` is on PATH (DOCX→PDF, PPTX→PDF, output-cap fallthrough). Integration tests skip cleanly on bare CI. * 🩹 fix: CI — preserve legacy download path for empty-text office attachments Two regressions surfaced after the textFormat security gate landed. 1. **Client** (`LogContent.test.tsx` "falls back to the legacy download branch for an office file with no extracted text"): When the security gate downgraded an office type without `textFormat: 'html'` to PLAIN_TEXT, the lenient empty-text gate on PLAIN_TEXT then accepted a missing `text` field and rendered a half-empty panel card. The historical contract is "office type + no text → legacy download UI"; the downgrade should only fire when there's actual plain text that needs safe-escaping. Fix in `detectArtifactTypeFromFile`: short-circuit to null when the office type lands in the security-gate branch with no text. The PLAIN_TEXT downgrade still fires for legacy attachments that DO carry plain text. 2. **API** (`process.spec.js` + `process-traversal.spec.js`): the `@librechat/api` mocks didn't expose `getExtractedTextFormat`, so `processCodeOutput` called `undefined(...)` → TypeError → tests got undefined results. Added the helper to both mocks with a faithful default (returns 'text' for non-null extractor output, null otherwise). Tests: new regression in `artifacts.test.ts` pinning the empty-text + no-textFormat → null contract for all four office types (.docx/.csv/.xlsx/.pptx), so a future refactor can't silently re-introduce the half-empty card. * 🩹 fix: PPTX slides scale to fit panel width (no horizontal scroll) Manual e2e on PR #12934: pptx-preview rendered slides at their native init dimensions (960×540 default). The artifact panel is much narrower than that, so the iframe got a horizontal scrollbar and only a corner of each slide showed at any time — the user had to drag-scroll across each slide to read it. Fix: keep pptx-preview's init at 960×540 so its internal layout math stays correct, then post-process each rendered slide: - Cache the slide's native width/height on its dataset BEFORE applying any transform (so subsequent re-fits don't measure the already-transformed box). - Wrap the slide in `.lc-slide-wrap` with explicit width/height set inline to the scaled dimensions; the wrap shrinks the layout space the slide occupies. - Apply `transform: scale(panel_width / 960)` to the slide itself with `transform-origin: top left` so the rendered output shrinks from the top-left corner into the wrap. - Cap the scale at 1.0 so small slides don't upscale and get blurry. Streaming + resize: - `MutationObserver` watches the container for slide insertions so streaming renders get scaled on arrival rather than waiting for the entire `previewer.preview` promise to settle. - `ResizeObserver` re-fits all wrapped slides when the iframe resizes (panel drag, window resize). Tests: new "bootstrap wraps + scales each slide" lock in the wrap class, scale computation, observer setup, and native-size caching so a future refactor can't silently re-introduce the overflow. * 🩹 fix: PPTX wrap+scale runs after preview, not during streaming Manual e2e on PR #12934: regenerated PPTX showed "Preview unavailable" in the iframe. Root cause: the MutationObserver I added in the previous commit fired during pptx-preview's render and moved slides out from under the library's references. pptx-preview's async pipeline raised an unhandled rejection, the iframe's window-level listener caught it, and the fallback message replaced the partial render. Fix: drop the MutationObserver. Apply the wrap+scale ONCE in a `finalize` step that runs: - On `previewer.preview().then` (the happy path) - On the 8-second timeout safety net IF the container has children (silent-failure path — pptx-preview emitted slides but never resolved its outer promise) To prevent the user from seeing an unscaled flash while pptx-preview renders into the 960px-wide canvas, the container is set to `visibility: hidden` at init and only revealed inside `finalize` after wrap+scale completes. Resize handling stays via `ResizeObserver` on `document.body`, installed AFTER the wrap pass so it doesn't fire during the wrap itself. Tests: regression assertion now also locks in: - `container.style.visibility = 'hidden' / 'visible'` (the flash- prevention contract) - Absence of MutationObserver (the bug we just removed — must NOT creep back in via a future "let's scale during streaming" idea) * 🩹 fix: PPTX slides fill panel width (drop upscale cap, per-slide scale) Manual e2e on PR #12934: slides rendered correctly but didn't fill the artifact panel — whitespace on either side. Two issues: 1. The scale was capped at `Math.min(1, available / SLIDE_W)`. On panels wider than 960px, the cap clamped the scale to 1.0 and slides rendered at native size with whitespace on the sides instead of stretching. 2. The scale was computed against the constant `SLIDE_W = 960`, but pptx-preview can emit slides whose `offsetWidth` differs from the init param if the source PPTX has a non-16:9 layout. Per-slide division of `available / nativeW` handles that case. Fix: replace `computeScale()` with two helpers — `availableWidth()` returns the panel content-box width and `scaleFor(nativeW)` returns the per-slide scale. No upscale cap. The slide content is rendered by pptx-preview against its 960×540 canvas using vector text / canvas — scaling up to e.g. 1500px doesn't visibly degrade quality. Tests: regression now also asserts: - `availableWidth()` and `scaleFor()` exist by name - The exact scale formula `availableWidth() / (nativeW || SLIDE_W)` - Negative assertion that `Math.min(1, ...)` is NOT present, so a future "let's add an upscale cap" rewrite can't silently re-introduce the whitespace. * 🩹 fix: PPTX preview fills panel height (no white gap below slides) Manual e2e on PR #12934: PPTX preview filled the panel width but left empty space below the last slide. DOCX didn't have this issue because its content (mammoth-rendered HTML) flows naturally and either fits exactly or overflows; PPTX slides are fixed-aspect 16:9 and don't grow with the panel. Two changes: 1. **Body fills the iframe viewport** — `html, body { min-height: 100vh }` plus `body { display: flex; flex-direction: column }` and `#lc-render { flex: 1 0 auto }`. The dark theme bg now fills the iframe even when total slide content is shorter than the panel, so a single-slide deck never reveals a "white below" gap. 2. **Per-slide scale honors viewport height** — `scaleFor(nativeW, nativeH)` now returns `min(width-fit, height-fit)` (largest factor that fits without overflowing either dimension). On a tall artifact panel with a short deck, slides grow up to the full panel height instead of staying at the width-bound size. Existing height-fit was always considered correct conceptually but the previous implementation only used width-fit, leaving half the viewport unused per slide. Tests: regression now also asserts `availableHeight()`, the `Math.min(sw, sh)` formula, and `min-height: 100vh` are in the bootstrap. Negative assertion for the old `Math.min(1, ...)` upscale cap remains. * 🩹 fix: revert body flex on PPTX bootstrap (caused black-screen render) Manual e2e regression on PR #12934: the previous commit added `body { display: flex; flex-direction: column }` plus `#lc-render { flex: 1 0 auto }` to fill the panel height. Side effect: pptx-preview's internal layout assumes block flow on its ancestor elements; making body a flex container caused slides to render as solid-black rectangles (sized correctly, but with no visible content inside). Fix: keep just `html, body { min-height: 100vh }` for the bg-fill effect — that alone gives empty space below short decks the dark theme bg without changing flow. Drop the body-flex and the `#lc-render { flex: 1 0 auto }` directives. The height-aware `scaleFor(nativeW, nativeH)` from the same commit stays — it doesn't interact with pptx-preview's layout, just chooses a per-slide scale. Each slide still grows to fit the viewport contain-style. Negative-assertion added to the regression test: `body { display: flex }` must NOT appear in the bootstrap, so a future "let's flex the body to make height work" rewrite can't silently re-introduce this. (Note: the user also flagged DOCX theming as faint body text; I'm leaving that for now per their note that it may be pre-existing. Not addressed in this commit.) * 🩹 fix: revert PPTX height-fill changes; lock DOCX CDN to light scheme Two fixes for separate manual e2e regressions on PR #12934. **1. PPTX black screen (single slide rendering as solid black).** The previous fix removed `body { display: flex }` thinking that was the sole cause, but the regression persisted. Bisecting against the last known-good commit (4e2d538b0, width-fit only), the actual culprit is the COMBINATION of: - `min-height: 100vh` on html/body - `availableHeight()` reading viewport-derived dimensions - `Math.min(sw, sh)` height-aware scale pptx-preview's CSS injection step interacts unpredictably with these. Reverting to width-only `scaleFor(nativeW)` and dropping the viewport min-height restores reliable rendering. Vertical empty space below short decks now shows the body's bg color (`var(--bg)`) which still matches the panel theme — that's an acceptable trade-off vs. the black-screen regression. Negative assertions added: `Math.min(sw, sh)`, `availableHeight`, `min-height: 100vh`, `body { display: flex }` must NOT appear in the bootstrap. So a future "let's fill height" rewrite has to demonstrate it doesn't break pptx-preview before it can land. **2. DOCX body text rendering as faint / translucent grey.** docx-preview emits page-style rendering with white pages and the docs native text colors. The CDN doc declared `color-scheme: light dark`, so on OS dark mode the iframes inheritable `--fg` resolved to `#e5e7eb` (light grey). docx-preview body text (no explicit color in the source DOCX) inherited that light-grey on the white page bg → barely-visible "translucent" rendering. Fix: declare `color-scheme: light` only in `buildDocxCdnDocument`, drop the dark-mode `@media` override. docx-preview is a light-mode- only renderer; matching that produces correct contrast regardless of OS theme. The mammoth-only `wrapAsDocument` path is unaffected — it owns its own bg + text colors and continues to respect the users OS scheme. New regression test pins the lock: CDN doc must contain `color-scheme: light`, must NOT contain `color-scheme: light dark`, must NOT contain `prefers-color-scheme: dark`. * 🩹 fix: relax connect-src to allow sourcemap fetches (silence CSP noise) Manual e2e on PR #12934: every time DevTools is open while viewing a DOCX or PPTX preview, the console fills with CSP violations like: Connecting to 'https://cdn.jsdelivr.net/npm/docx-preview@0.3.7/ dist/docx-preview.min.js.map' violates the following Content Security Policy directive: "connect-src 'none'". The request has been blocked. The actual rendering isn't affected (sourcemap fetches happen AFTER the script has already loaded and executed via `script-src`), but the noise is enough to make people suspect a real problem and distracts from useful console output. Fix: relax `connect-src` from `'none'` to `'self' https://cdn. jsdelivr.net` in both DOCX and PPTX CDN docs. This allows: - Same-origin fetches (sandpack-static-server) — covers any bundler-embedded sourcemaps + same-origin runtime fetches the renderer might make - jsdelivr fetches — covers sourcemaps from the CDN where we loaded the script Exfiltration risk stays minimal: the iframe is cross-origin to LibreChat so an attacker can't read application data anyway, and neither 'self' (sandpack-static-server) nor jsdelivr is a useful target for exfiltrating slide content to a host the attacker controls. Tests updated: assertions for `connect-src 'none'` swapped to `connect-src 'self' https://cdn.jsdelivr.net` for both DOCX + PPTX CDN docs. Added negative assertion for wildcard `*` in connect-src so a future "let's allow everything" rewrite can't widen the exfiltration surface. * 🩹 fix: surface PPTX/DOCX fallback reason (inline + console) Manual e2e on PR #12934: "Preview unavailable" appears in the iframe with no way to know what actually failed. The reason was tucked into the fallback element's `title` attribute (hover-only tooltip) — easy to miss and impossible to copy/paste. Now surfaces three ways: 1. Visible inline via a `<details>` element with the reason in monospace, folded so the friendly message stays primary but the diagnostic is one click away in the iframe itself. 2. `title` attribute (preserved) for hover tooltip. 3. `console.error('[pptx-preview] fallback fired:', reason)` so DevTools shows it in red — also the only reliable way to see the reason if the iframe is detached / re-mounted. DOCX gets the same console mirror (as `console.warn` since the fallback there is "high-fidelity unavailable, showing simplified preview" — informational, not error). The DOCX fallback already displays the mammoth-rendered content visibly, so no `<details>` needed there. Tests: regression assertions pin the diagnostic surfacing — the `<details>` element, the `title` write, and the `console.error` call must all be present in the bootstrap. * 🩹 fix: PPTX CDN embeds slide-list fallback + detects empty renders Manual e2e + DOM inspection on PR #12934: pptx-preview silently produces empty `.pptx-preview-wrapper` placeholders for pptxgenjs- generated decks. The library parses the file enough to create the 960×540 host element with a black bg, then fails to populate it. The outer Promise resolves "successfully" — no throw, no rejection, the bootstrap thinks rendering succeeded — and the user sees a black rectangle with no content and no fallback message. Fix mirrors the DOCX mammoth-fallback pattern from commit 0c0b0ce88: 1. **Server side**: `pptxToHtml` now renders the slide-list body (`<ol class="lc-pptx-list">...`) via the new `renderPptxSlidesBody` helper, then embeds it inside the CDN doc via the new `buildPptxCdnDocument(base64, slideListFallbackBody)` signature. Combined-doc size budget mirrors the DOCX pattern: if the CDN doc would exceed `OFFICE_HTML_OUTPUT_CAP` (512 KB), drop to slide-list only. 2. **Iframe bootstrap**: new `hasRenderedContent()` check after `wrapSlides()` walks each `.lc-slide-wrap` looking for actual child content inside pptx-preview's emitted slide nodes. If every wrap is empty, fires `showFallback('renderer-produced-empty- wrappers ...')` which reveals the embedded slide-list view instead of the previous static "Preview unavailable" message. 3. **CSS**: slide-list rules extracted to `PPTX_SLIDE_LIST_CSS` constant so they can be inlined into both the standalone slide- list document AND the CDN doc's `<style>` block (CSP `style-src` is `'unsafe-inline'` only — no external sheets). `renderPptxSlidesHtml` now delegates to `renderPptxSlidesBody` wrapped in `wrapAsDocument` — single source of truth for the slide markup. Tests (506 passing, +1 vs before): existing `pptxToHtmlViaCdn` call sites updated for the new fallback-body argument; new regression test pins `hasRenderedContent`, the `renderer-produced-empty-wrappers` reason string, the embedded fallback structure, and the inlined slide-list CSS. * fix: Detect Empty PPTX Preview Slides * 🩹 fix: LibreOffice PDF embed uses blob: URL (Chrome blocks data: PDFs) Manual e2e on PR #12934: enabling `OFFICE_PREVIEW_LIBREOFFICE=true` on a host with `soffice` installed surfaced "This page has been blocked by Chrome" inside the PDF preview iframe. Root cause: Chrome blocks `data:application/pdf;base64,...` navigations inside sandboxed iframes (anti-phishing measure since Chrome 76, see crbug.com/863001). The Sandpack iframe IS sandboxed (its `sandbox="..."` attribute lacks `allow-top-navigation` for data: URLs specifically), so when our inner `<iframe src="data: application/pdf;...">` tries to navigate, Chrome's interstitial fires and renders the "blocked" message. Fix: switch from `data:` URL to `blob:` URL. The bootstrap now: 1. Reads the base64 payload from a `<script type="application/ octet-stream;base64">` data block (same pattern as the DOCX and PPTX wrappers). 2. Decodes via `atob` + `Uint8Array.from`. 3. Creates a `Blob` with `type: 'application/pdf'`. 4. `URL.createObjectURL(blob)` produces a same-origin blob: URL. 5. Sets `pdfFrame.src = url + '#view=FitH'` — Chrome treats blob: URLs as legitimate navigation and serves the built-in PDF viewer. CSP updated: `frame-src blob:` (was `frame-src data:`). `data:` is now explicitly NOT allowed in `frame-src` since Chrome would block it anyway in our context — keeping it would be misleading documentation. Bonus: failure paths now log to `console.error` with a `[libreoffice-pdf]` prefix so DevTools surfaces blob-creation failures and PDF-viewer load timeouts in red. Tests updated: - "emits a complete sandboxed HTML document" now asserts the data-block + blob URL construction (not the old data: URL). - New CSP test "allows blob: in frame-src (NOT data:)" with both positive and negative assertions to lock in the change. - Integration test for `tryLibreOfficePreview` updated to look for the data block + `URL.createObjectURL` instead of the data: URL. - Large-payload test now verifies the data block round-trip rather than data: URL escaping (base64 alphabet has no characters that break out of `<script>` anyway). * 🩹 fix: LibreOffice PDF embed renders via pdf.js (Chrome blocks blob: PDFs too) Manual e2e on PR #12934 round 2: switching from `data:` to `blob:` URLs (commit d90f26c11) didn't fix the "This page has been blocked by Chrome" interstitial. Chrome blocks BOTH data: AND blob: PDF navigations inside sandboxed iframes — the built-in PDF viewer requires a top-level browsing context. The Sandpack host iframe is sandboxed, so neither approach works. Fix: switch from native browser PDF viewer to pdf.js (Mozilla's pdfjs-dist) loaded from CDN. pdf.js renders to `<canvas>` which works in any context — no plugin, no privileged viewer, no top-level requirement. ~1 MB CDN load is acceptable for a path that's already opt-in via `OFFICE_PREVIEW_LIBREOFFICE=true`. Implementation: - Pin pdf.js v3.11.174 (single-file UMD; v4+ uses ES modules which complicate the load + SRI flow) - Worker URL pointed at the same jsdelivr origin; CSP `worker-src https://cdn.jsdelivr.net blob:` allows it - DPR-aware canvas rendering: scale based on `panelWidth / page.viewport.width * devicePixelRatio` so retina displays get crisp pixels - Sequential page rendering (Promise chain) so a many-slide PDF doesn't spawn N parallel render jobs - 15 s timeout safety net (was 4 s for the native viewer; pdf.js with DPR=2 on a many-page PDF can take longer) CSP changes: - Added `script-src https://cdn.jsdelivr.net 'unsafe-inline'` (was inline-only) - Added `worker-src https://cdn.jsdelivr.net blob:` - Removed `frame-src` entirely (no nested iframes) - Removed `object-src` (no `<object>`/`<embed>` either) Same diagnostic surfacing as the other CDN paths: failure reasons shown via `<details>` disclosure inline + `console.error` to DevTools. Tests updated: PDF.js script presence, GlobalWorkerOptions setup, canvas render path, all the new failure detection paths. Negative assertions for both `data:application/pdf` and `blob:...application /pdf` so a future "let's just try the native viewer again" rewrite can't silently re-introduce the Chrome block. SRI hashes intentionally omitted (unlike docx-preview / pptx- preview) — operator opted in by setting the env flag and trusts the LibreOffice render pipeline. Worth adding once the path is proven in production. * 🧹 cleanup: trim unused _internal exports + stale JSDoc references After the LibreOffice + pdf.js path proved out, swept the office HTML modules for dead code and stale documentation. **Unused `_internal` exports removed (`html.ts`):** - `renderMammothBody` — only called within the file (by `wordDocToHtmlViaMammoth` and `wordDocToHtml`), never imported by tests. - `DOCX_PREVIEW_CDN` — internal config constant, never referenced. - `PPTX_PREVIEW_CDN` — same, never referenced. The remaining `_internal` surface (`wordDocToHtmlViaCdn`, `wordDocToHtmlViaMammoth`, `pptxToHtmlViaCdn`, `MAX_DOCX_CDN_BINARY_BYTES`, `MAX_PPTX_CDN_BINARY_BYTES`, `OFFICE_HTML_OUTPUT_CAP`) is all actively used by the spec file. **Stale JSDoc fixed (`libreoffice.ts`):** Module-level header still claimed we "embed the PDF as a base64 data:application/pdf URI" and "rely on the host browser's built-in PDF viewer". Both untrue after the pdf.js switch in commit b2cc81ad8. Updated to: - Describe the actual pipeline: PPTX → soffice → PDF → pdf.js → canvas - Document the dead-end iterations (data: blocked, blob: also blocked, pdf.js works) so future readers don't re-discover the same Chrome PDF-viewer-in-sandboxed-iframe limitation - Drop "(POC)" tag — the path is production-quality, just opt-in - Adjust disk footprint estimate (250-350 MB with `--no-install-recommends` is more accurate than the 500 MB original) No production code changes; tests still 505 passing. * ✨ feat: per-format LibreOffice opt-in (env value accepts format list) Manual e2e on PR #12934: enabling `OFFICE_PREVIEW_LIBREOFFICE=true` forces both DOCX and PPTX through the LibreOffice path. DOCX renders ~instantly via docx-preview and rarely needs the LibreOffice treatment; paying the ~2-3 s cold-start there hurts UX without adding much. Solution: extend the env var to accept three forms: - Truthy (`true`/`1`/`yes`): all formats — backwards compatible with the previous behavior - Falsy (`false`/`0`/`no`/empty/unset): no formats — default - Comma-separated list (`pptx`, `pptx,docx`): just those formats Practical guidance documented in the module header: most operators will set `OFFICE_PREVIEW_LIBREOFFICE=pptx` — pptx-preview chokes on pptxgenjs decks and the slide-list fallback loses formatting, so LibreOffice is the only path that produces a faithful PPTX preview. DOCX is well-served by docx-preview's existing CDN renderer. API: - New `isLibreOfficeEnabledFor(format)` is the per-format gate, used by `tryLibreOfficePreview` to short-circuit before doing work. - Existing `isLibreOfficeEnabled()` retained for "any format enabled" diagnostic checks (returns true if at least one format is opted in). - Internal `parseLibreOfficeEnablement` returns `'all' | Set | null` — keeps the gate future-proof: adding a new format to the LibreOffice route doesnt require operators to re-enumerate their env value. Edge cases handled: - Whitespace-tolerant: ` pptx , docx ` works - Case-insensitive on both env value AND format name - Empty list entries dropped: `pptx, ,docx` enables pptx + docx - Empty string treated as unset (not as a valid empty list) Tests: 21 new cases pinning the parse semantics + per-format gate (`pptx` env vs `docx` lookup → false, etc.). Existing `isLibreOfficeEnabled` tests retained but renamed to clarify the "any format" semantic. Total file tests: 526 passing (+21 vs before). * 🔒 fix: officeHtmlBucket only does MIME fallback when extension is empty Codex P2 review on PR #12934: the server's `officeHtmlBucket` falls back to MIME whenever the extension isn't an OFFICE extension. The client's `detectArtifactTypeFromFile` is stricter — it routes by extension first for ANY known extension (`.txt` → PLAIN_TEXT, `.md` → MARKDOWN, `.py` → CODE, etc.), only falling back to MIME when the extension is unknown. Mismatch case: `notes.txt` shipped with `Content-Type: application/ vnd.openxmlformats-officedocument.wordprocessingml.document`. Server runs `officeHtmlBucket` → extension `.txt` not office → MIME fallback → 'docx' → produces full HTML, sets `textFormat: 'html'`. Client routes by extension to PLAIN_TEXT (extension wins), markdown viewer escapes the HTML, user sees raw `<html>...` markup instead of the rendered preview. Fix: server only falls back to MIME when extension is genuinely empty (extensionless filename). Symmetric with the client's "extension wins for any known extension" semantic — neither will mis-route. Trade-off: a true DOCX renamed to `myfile.bin` with the canonical DOCX MIME no longer routes through office HTML on the server. The client would have routed to the office bucket via MIME, then the security gate (`textFormat !== 'html'`) would have downgraded to PLAIN_TEXT anyway. So the user-visible outcome is the same (raw bytes via PLAIN_TEXT) — the new behavior just avoids producing HTML that the client would never use. Long-term fix: share the extension routing table in data-provider so both server and client query the same source of truth. Out of scope for this PR. Tests: new 8-case `it.each` block in `officeHtmlBucket predicate` locks in the contract — `.txt`/`.md`/`.json`/`.py`/`.html`/`.css` + office MIME → null, and `.bin`/`.dat` + office MIME → null too. Existing extension-wins tests still pass unchanged. Total file tests: 534 (+8 vs before).

* 🌩️ feat: CloudFront CDN File Strategy + signed cookies Squashed from PR danny-avila#12193: - feat(storage): add CloudFront CDN file strategy - feat(auth): add CloudFront signed cookie support Note: package.json/package-lock.json dependency additions are intentionally omitted from this commit and will be re-added via `npm install` after rebase to avoid lock-file merge conflicts. The two new peer deps that need to be re-installed are: - @aws-sdk/client-cloudfront@^3.1032.0 - @aws-sdk/cloudfront-signer@^3.1012.0 Also fixes 4 missing destructured names in AuthService.spec.js (getUserById, generateToken, generateRefreshToken, createSession) that were referenced in tests but not imported from the mocked '~/models'. * 📦 chore: install CloudFront SDK deps for PR danny-avila#12193 Adds the two AWS CloudFront packages required by the rebased CloudFront CDN strategy: - @aws-sdk/client-cloudfront - @aws-sdk/cloudfront-signer Following the @aws-sdk/client-s3 pattern: - api/package.json: regular dependency (runtime resolution) - packages/api/package.json: peerDependency Generated by `npm install` against the freshly rebased lock file to avoid the merge conflicts that came from the original PR's lock-file edits being made against an older base of dev. * 🐛 fix: CI failures + review findings on CloudFront PR danny-avila#12193 CI fixes - Rename packages/data-provider/src/__tests__/cloudfront-config.test.ts → src/cloudfront-config.spec.ts. Jest's default testMatch picks up __tests__/ directories even inside dist/, so the compiled .d.ts shell was being executed as an empty test suite. Moving to .spec.ts (matching the rest of the package) avoids the dist/ pickup. - Add cookieExpiry: 1800 to CloudFront crud.test makeConfig: the schema applies a default so CloudFrontFullConfig requires it. Review findings addressed - #1 (Codex + comprehensive): Normalize CloudFront domain with /\/+$/ regex (and key with /^\/+/ regex) in buildCloudFrontUrl, matching the cookie code so resource policy and file URLs stay aligned even when the configured domain has multiple trailing slashes. Added tests. - #2: Move DEFAULT_BASE_PATH out of s3Config into shared packages/api/src/storage/constants.ts. ImageService no longer imports S3-specific config. - #3: getCloudFrontConfig() returns Readonly<CloudFrontFullConfig> | null to discourage mutation of the cached signing config. - #4: Add cross-field refinement tests for cloudfrontConfigSchema (invalidateOnDelete-without-distributionId, imageSigning="cookies"-without-cookieDomain). - danny-avila#6: Revert unrelated MCP comment re-indentation in librechat.example.yaml. - danny-avila#7: Add azure_blob to the strategy list comment. Skipped - danny-avila#5 (extractKeyFromS3Url with CloudFront URLs): existing deleteFileFromCloudFront tests already cover the path-equivalence assumption; renaming the helper is real refactor work beyond this PR's scope. - danny-avila#8, danny-avila#9 (NIT, low confidence): leaving for author judgement. * 🧹 chore: drop dead DEFAULT_BASE_PATH from s3Config test mock After moving DEFAULT_BASE_PATH to ~/storage/constants, crud.ts no longer reads it from s3Config — so the entry in the s3Config jest mock was misleading dead config. The tests still pass because the unmocked real constants module provides the value. --------- Co-authored-by: Danny Avila <danny@librechat.ai>

…anny-avila#12957) * 🗂️ feat: add `status` lifecycle to file records for two-phase previews Schema and model foundation for decoupling the agent's final response from CPU-heavy office-format HTML extraction. - `MongoFile.status: 'pending' | 'ready' | 'failed'` (indexed) and `previewError?: string` mirror the lifecycle: phase-1 emits the file record at `pending` so the response is unblocked; phase-2 transitions to `ready` (with text/textFormat) or `failed` (with previewError) in the background. Absent for legacy records — clients treat that as `ready` for back-compat. - Mirror types added to `TFile` in data-provider so frontend cache consumers see the new fields. - New `sweepOrphanedPreviews(maxAgeMs)` method on the file model recovers stale `pending` records left behind by a process restart mid-extraction; transitions them to `failed` with `previewError: 'orphaned'`. Cheap because `status` is indexed. * ⚡ feat: two-phase code-execution preview flow (unblocks final response) The agent's final response no longer waits on CPU-heavy office HTML extraction. Phase-1 (download + storage save + DB record at `status: 'pending'`) is awaited as before; phase-2 (extract + `updateFile`) runs in the background with a hard 60s ceiling. Three flows, all funneling through `processCodeOutput` and updated to the new `{ file, finalize? }` return shape: - `callbacks.js` (chat-completions + Open Responses streaming): emit the phase-1 attachment immediately (carries `status: 'pending'` for office buckets so the UI shows "preparing preview…"), then fire-and-forget `finalize()`. If the SSE stream is still open when phase-2 lands, push an `attachment` update event with the same `file_id` so the client merges over the placeholder in place. - `tools.js` direct endpoint: same split — return the phase-1 metadata immediately, run extraction in the background. Client polls for the resolved record. `finalize()` wraps the existing 12s per-render timeout in a 60s outer `withTimeout`. The HTML-or-null contract from danny-avila#12934 is preserved: office types that fail extraction transition to `status: 'failed'` with `previewError: 'parser-error' | 'timeout'` rather than falling back to plain text (would be an XSS vector). Promises continue running after the HTTP response closes (Node doesn't kill them). The boot-time orphan sweep covers the only case that loses progress — actual process restart mid-extraction. `primeFiles` annotates the agent's `toolContext` line for prior-turn files: `(preview not yet generated)` for pending, `(preview unavailable: <reason>)` for failed. The model can volunteer "you can still download it" instead of pretending the preview is fine. `hasOfficeHtmlPath` exported from `@librechat/api` so `processCodeOutput` can decide whether a file expects a preview at all. * 🔍 feat: `GET /api/files/:file_id/preview` endpoint and boot orphan sweep - New `GET /api/files/:file_id/preview` route returns `{ status, text?, textFormat?, previewError? }`. The frontend's `useFilePreview` React Query hook polls this while phase-2 is in flight, then auto-stops on terminal status. ACL identical to the download route (reuses `fileAccess` middleware). Defaults `status` to `'ready'` for legacy records so back-compat is implicit. `text` only included when `status === 'ready'` and non-null — preserves the HTML-or-null security contract from danny-avila#12934. - `sweepOrphanedPreviews()` invoked on boot in both `server/index.js` and `server/experimental.js`. Recovers any `pending` records left behind by a process restart mid-extraction (the only case the in-process two-phase flow can't handle on its own). Fire-and-forget so a transient sweep failure doesn't block startup. * 🖥️ feat: frontend two-phase preview consumer (polling + UI states) Wires the React side to the new lifecycle so the user sees what's happening with their file while phase-2 extraction runs in the background and after the response stream closes. - `useAttachmentHandler` upserts by `file_id` (was append-only) so the phase-2 SSE update event merges over the pending placeholder in place. Lightweight attachments without a `file_id` (web_search / file_search citations) keep the legacy append path. - `useFilePreview(file_id)` React Query hook with `refetchInterval: (data) => data?.status === 'pending' ? 2500 : false` so polling auto-stops on the first terminal response without the caller having to flip `enabled`. - `useAttachmentPreviewSync(attachment)` bridges polled data into `messageAttachmentsMap`. Polling enabled iff `status === 'pending' && isAnySubmitting` — per the design ask: active polling while the LLM is still generating, then quiet. Process-restart and post-stream cases are covered by polling on the next interaction. - `Attachment.tsx` renders a small `PreviewStatusIndicator` (spinner + "Preparing preview…" for pending, alert icon + "Preview unavailable" for failed) inside `FileAttachment`. Download button stays fully functional in both states. Two new English locale keys. - Data-provider scaffolding: `TFilePreview` type, `endpoints.filePreview`, `dataService.getFilePreview`, `QueryKeys.filePreview`. * 🧪 fix: stub `useAttachmentPreviewSync` in pre-existing Attachment test mocks The new `useAttachmentPreviewSync` hook is called unconditionally inside `FileAttachment` (added in the prior commit). Two pre-existing test files mock `~/hooks` to provide `useLocalize` only — the un-mocked preview hook reference resolved to undefined and crashed render with `(0 , _hooks.useAttachmentPreviewSync) is not a function` on the Ubuntu/Windows CI runners. Fix is local to the test mocks: add a no-op stub that returns `{ status: 'ready' }` so the component renders the legacy chip path. The two-phase preview behavior itself has its own dedicated suites (`useAttachmentHandler.spec.tsx`, `useAttachmentPreviewSync.spec.tsx`). * 🐛 fix: route phase-2 attachment update to current-run messageId Codex P1 review on PR danny-avila#12957. `processCodeOutput` intentionally preserves the original DB `messageId` across cross-turn filename reuse so `getCodeGeneratedFiles` can still trace a file back to the assistant message that originally produced it. The phase-1 SSE emit already routes by the current run's messageId — `processCodeOutput` runtime-overlays it via `Object.assign(file, { messageId, toolCallId })` and the callback writes `result.file` directly. Phase-2 was passing the raw `updateFile` return through `attachmentFromFileMetadata`, which read `messageId` straight off the DB record. On a turn-N run that re-emitted a filename from turn-1 (e.g. agent writes `output.csv` again), the phase-2 SSE update routed to `turn-1-msg` instead of `turn-N-msg`. Frontend's `useAttachmentHandler` upserts under the wrong messageAttachmentsMap slot — turn-N's pending chip stays stuck at "preparing preview…" while turn-1's already-resolved attachment gets re-merged. Fix: thread `runtimeMessageId` through `attachmentFromFileMetadata` and pass `metadata.run_id` from the phase-2 emit site. Mirrors how phase-1 sources its messageId. Tests cover the cross-turn reuse case plus the writableEnded / null-finalize / no-finalize paths to lock in the broader phase-2 emit contract. * 🛠️ refactor: address codex audit findings (wire-shape parity, DRY, defensive catch) Comprehensive audit on PR danny-avila#12957. Resolves all valid findings: - **MAJOR #1 — Wire-shape parity**: phase-1 ships the full `fileMetadata` record over SSE; phase-2 was using a tight `attachmentFromFileMetadata` projection. Drop the projection and have phase-2 spread `{...updated, messageId, toolCallId}` so both events match the long-standing legacy phase-1 shape clients depend on. - **MAJOR #2 — DRY**: extract `runPhase2Finalize({ finalize, fileId, onResolved })` into `process.js` (alongside `processCodeOutput` whose contract it pairs with). Both `callbacks.js` paths and `tools.js` now flow through it. Single catch path eliminates divergence surface — the fix landed in 01704d4 (cross-turn messageId routing) was a symptom of this duplication risk. - **MINOR #3 — JSDoc accuracy**: `finalizePreview`'s buffer is bounded by `fileSizeLimit`, not the 1MB extractor cap. Updated and added a note about peak heap from queued buffers. - **MINOR #4 — Defensive catch**: `runPhase2Finalize`'s catch attempts a best-effort `updateFile({ status: 'failed', previewError: 'unexpected' })` for the file_id, so a programming bug in `finalizePreview` doesn't leave the record stuck `'pending'` until the next boot-time orphan sweep. - **NIT danny-avila#6 — Stale PR refs**: 12952 → 12957 in 3 places. - **NIT danny-avila#7 — Schema bound**: `previewError` capped at `maxlength: 200` to prevent a future codepath from accidentally persisting a stack trace. Skipped per audit verdict (non-blocking): - danny-avila#5 (memory pressure): documented in JSDoc; impl change was reviewer's "consider", not actionable. - danny-avila#8 (double DB query per poll): low cost, indexed by_id, polling is gated narrow. - danny-avila#9 (TAttachment cast): the union type is intentional; the casts are safe widening, refactoring TAttachment is invasive and out of scope. Tests: 11 new (7 `runPhase2Finalize` unit tests covering happy path, null-finalize, throws, double-fail, no-fileId, no-onResolved; +4 wire-shape parity assertions in the existing cross-turn test). 328 backend tests pass; 528 frontend tests pass; lint and typecheck clean. * 🛡️ refactor: address codex P1+P2 + rename to drop phase-1/2 jargon Codex round 2 review on PR danny-avila#12957 caught two race conditions and one recovery gap, all triggered by cross-turn filename reuse (`claimCodeFile` intentionally returns the same `file_id` for the same `(filename, conversationId)` across turns). Plus naming cleanup the user requested — internal "phase 1 / phase 2" vocabulary leaks across sprints, replace it everywhere with terms describing what's actually happening. P1 — stale render overwrites newer revision (process.js) Two turns reusing `output.csv` share a `file_id`. If turn-1's background render resolves AFTER turn-2's persist step, the unconditional `updateFile` writes turn-1's stale text/status over turn-2's pending placeholder. Fix: stamp a fresh `previewRevision` UUID on every emit, thread it through `finalizePreview`, and make the commit conditional via a new optional `extraFilter` argument on `updateFile` (`{ previewRevision: <expected> }`). The defensive `updateFile` in `runPreviewFinalize`'s catch uses the same guard so a programming error from an older render also can't override a newer turn. P1 — stale React Query cache on pending remount (queries.ts) Same root cause from the frontend side. Cache key `[QueryKeys.filePreview, file_id]` may hold a prior turn's `'ready'` payload; with `refetchOnMount: false` and the polling gate on `pending`, polling never starts for the new placeholder. Fix: `useAttachmentHandler` invalidates that query whenever an attachment with a `file_id` arrives. Both initial-emit and update events trigger invalidation — uniform gate. P2 — quick-restart orphans skipped by boot sweep (files.js) Boot `sweepOrphanedPreviews` uses a 5-min cutoff for multi-instance safety. A crash + restart inside the cutoff leaves `pending` records that never get touched again. Fix: lazy sweep inside the preview endpoint — if a polled record is `pending` and `updatedAt` is older than 5 min, mark it `failed:orphaned` on the spot before responding. Conditional on the same `updatedAt` we observed so a concurrent legitimate update wins. Cheap, bounded by user activity. Naming cleanup - `runPhase2Finalize` → `runPreviewFinalize` - `PHASE_TWO_TIMEOUT_MS` → `PREVIEW_FINALIZE_TIMEOUT_MS` - All `phase-1` / `phase-2` / `two-phase` prose replaced with "the immediate emit", "the deferred render", "the persist step", "the deferred preview", etc. Skill-feature `phase 1/2` references (different feature) left alone. Tests: 10 new (4 lazy-sweep × preview endpoint, 3 cache-invalidation × useAttachmentHandler, 3 extraFilter × updateFile data-schemas). Backend 332/332, frontend 531/531, data-schemas 37/37, lint clean. * 🛠️ refactor: address comprehensive review (round 3) — stale-cache MAJOR + 3 minors Comprehensive review on PR danny-avila#12957 caught a P1 follow-on bug from the prior `invalidateQueries` fix, plus 3 maintainability findings. MAJOR: stale React Query cache not actually fixed by `invalidateQueries` The previous fix called `invalidateQueries` to flush stale cached preview data on cross-turn filename reuse. But `useFilePreview` had `refetchOnMount: false`, which made the new observer read the stale-marked 'ready' data without refetching. The polling `refetchInterval` then evaluated against stale 'ready' → returned `false` → polling never started → user stuck on stale content. Fix (belt-and-suspenders): a) `useAttachmentHandler` switched to `removeQueries` — drops the cache entry entirely so the next mount has nothing to read and must fetch. b) `useFilePreview` no longer sets `refetchOnMount: false`, so the React Query default (`true`) kicks in — second line of defense if any future codepath observes stale data before the handler has a chance to evict. MINOR: `finalizePreview` JSDoc missing `previewRevision` param Added with explanation of the conditional update guard. MINOR: asymmetric stream-writable guard between SSE protocols Chat-completions delegated the gate to `writeAttachmentUpdate`; Open Responses inlined `!res.writableEnded && res.headersSent`. Extracted `isStreamWritable(res, streamId)` predicate; both paths + `writeAttachmentUpdate` now share the single source of truth. NIT: `(data as Partial<TFile>).file_id` cast repeated 4 times Extracted to a `fileId` local at the top of the handler. Tests: existing 9 invalidate-tests rewritten as remove-tests; +1 new lock-in test asserts removeQueries is called and invalidateQueries is NOT (regression guard against round-3 finding). 332 backend pass, 532 frontend pass, lint clean. Skipped findings (deferred / acceptable): - MINOR: post-submission pending state has no auto-recovery — the `isAnySubmitting` polling gate was the user's explicit design; LLM context surfaces failed/pending so the model can volunteer. Worth a follow-up if real users hit it. - NIT: double DB query per preview poll — reviewer marked acceptable; changing `fileAccess` middleware is out of scope. * 🛡️ test: address comprehensive review NITs (initial-emit guard + isStreamWritable coverage) NIT — chat-completions initial emit skips writableEnded check The Open Responses initial emit was switched to use the new `isStreamWritable` predicate in the round-3 commit, but the chat-completions initial emit kept the older narrower check (`streamId || res.headersSent`). On a client disconnect mid-stream (`writableEnded === true`) it would still hit `res.write` and raise `ERR_STREAM_WRITE_AFTER_END` — caught by the outer IIFE catch but logged as noise. Switch this site to `isStreamWritable` too so both initial-emit paths share the same gate as the deferred update emits. NIT — `isStreamWritable` not directly unit-tested The predicate was only covered indirectly via the deferred-preview SSE tests (writableEnded skip, headersSent check). Export from `callbacks.js` and add 5 parametric tests pinning down each branch (streamId truthy, res null, !headersSent, writableEnded, happy path) so a future condition addition can't silently regress. * 🐛 fix: stuck "Preparing preview…" + inline the chip subtitle Two related fixes for a stuck-spinner bug a user reported in manual testing of PR danny-avila#12957. **Stuck spinner (the bug)** The deferred preview render can complete a few seconds AFTER the SSE stream closes (typical case: PPTX render finishes ~3s after the LLM emits FINAL). When that happens, the SSE update is silently dropped (`isStreamWritable` returns false on a closed stream) and polling is the only recovery path. The earlier polling gate was `status === 'pending' && isAnySubmitting`, which mirrored the original design intent ("only query while the LLM is still generating"). But `isAnySubmitting` flips false the moment the model emits FINAL — milliseconds before the deferred render commits. Polling never runs, the chip stays "Preparing preview…" forever even though the DB has `status: 'ready'` with valid HTML. Drop the `isAnySubmitting` part of the gate. `useFilePreview`'s `refetchInterval` is already a function-form that returns `false` on the first terminal response, so polling auto-stops within one tick of resolution. The server-side render ceiling (60s) plus the lazy sweep in the preview endpoint cap the worst case to ~24 polls per pending attachment. Polling itself never blocks UX — the gate's purpose was "don't waste cycles", and capping by terminal status is the correct expression of that. **Inline the chip subtitle (the visual)** The previous design rendered "Preparing preview…" as a loose-feeling spinner+text BELOW the file chip. The chip itself looked done while a floating annotation said it wasn't. `FileContainer` gains an optional `subtitle?: ReactNode` prop that overrides the default file-type label. `Attachment.tsx` passes a `PreviewStatusSubtitle` (spinner + "Preparing preview…" / alert + "Preview unavailable") into that slot when the file's preview is pending or failed. The chip footprint stays identical to its `'ready'` form — just the second row swaps from "PowerPoint Presentation" to the status indicator. No floating element, no layout shift. Tests: regression test pinning down "polling stays enabled after the LLM finishes" so a future revert can't reintroduce the stuck-spinner bug. Existing FileContainer tests pass unchanged (subtitle override is opt-in). 522 frontend tests pass; lint clean. * 🐛 fix: deferred-preview survives reload + matches artifact card chrome Fixes the remaining stuck-pending case after the polling gate fix: on a reloaded conversation, message.attachments come from the DB frozen at the immediate-persist `status: 'pending'`, but `messageAttachmentsMap` is empty because no SSE handler ever fired for that messageId. Polling now INSERTS a new live entry when no record matches the file_id, and `useAttachments` merges live entries onto DB entries by file_id so the resolved text/textFormat reach `artifactTypeForAttachment` and the chip routes through the proper PanelArtifact card. Also replaces the small file chip used during the pending state with a PreviewPlaceholderCard that mirrors ToolArtifactCard chrome, so the transition to the resolved PanelArtifact no longer reshapes the UI. * ✨ feat: auto-open panel when deferred preview resolves pending→ready The legacy auto-open path is gated only on `isSubmitting`, so an office-file preview that resolves *after* the SSE stream closes would render in place but never auto-open the panel — even though that's exactly the moment the result becomes meaningful to the user. Adds a per-file_id one-shot signal that `useAttachmentPreviewSync` flips on the pending→ready edge; `ToolArtifactCard` consumes it on mount and auto-opens regardless of submission state. The signal is *only* set on the actual transition (history loads of pre-resolved files don't trigger it) and is consumed once (panel close + reopen on the same card stays user-controlled). * 🐛 fix: drop placeholder Terminal overlay + scope auto-open to fresh resolutions Two fixes for issues spotted in manual testing of the deferred-preview auto-open feature: 1. PreviewPlaceholderCard was passing `file={attachment}` to FilePreview, which triggered SourceIcon's Terminal overlay (`metadata.fileIdentifier` is set on every code-execution file). The artifact card itself doesn't show that overlay; the placeholder shouldn't either, so the pending→resolved transition is visually seamless. 2. The `previewJustResolved` flag flipped on every pending→ready transition observed by the polling hook — including stale-pending DB records that resolve via the first poll on a *history load*. Conversations whose immediate-persist snapshot left attachments at `status: 'pending'` would yank the panel open every revisit. Adds `mountedDuringStreamRef` to the hook (mirroring ToolArtifactCard) so the flag fires only when the hook itself was mounted during an active turn — preserving the pre-PR contract that the panel only auto-opens for results the user is actively waiting on, never for history. * 🐛 fix: don't downgrade preview to failed when only the SSE emit throws Codex P2 finding on PR danny-avila#12957: the original chain placed `.catch` after `.then(onResolved)`, so a throw inside `onResolved` (transport-side errors — SSE write race after stream close, an emitter listener throwing) would propagate into the finalize catch and persist `status: 'failed'` / `previewError: 'unexpected'`. That surfaced "preview unavailable" in the UI for a perfectly valid file, and degraded next-turn LLM context to reflect a non-existent failure. Wraps `onResolved` in its own try/catch so emit errors are logged but do not affect the file's persisted status. Extraction success and emit success are now independent: if extraction succeeds and `finalizePreview` writes the terminal status, the polling layer / next page load surfaces the resolved preview even if this turn's SSE emit didn't land. * 🛡️ fix: run boot-time orphan sweep under system tenant context Codex P2 finding on PR danny-avila#12957: `File` is tenant-isolated, so under `TENANT_ISOLATION_STRICT=true` the boot-time `sweepOrphanedPreviews` threw `[TenantIsolation] Query attempted without tenant context in strict mode` and the recovery path silently failed every restart. Stale `status: 'pending'` records would be stuck until a user happened to poll the preview endpoint and trigger the lazy sweep — which only covers the file the user is currently looking at, not the bulk candidate set the boot sweep is designed to recover. Wraps the sweep in `runAsSystem(...)` in both boot paths (`api/server/index.js` and `api/server/experimental.js`) and pins the contract with regression tests in `file.spec.ts` — one test asserts the bare call throws under strict mode, the other asserts the `runAsSystem`-wrapped call succeeds. * 🧹 chore: trim verbose comments from previous commit * 🧹 chore: address review findings (dead branch, lazy-sweep cutoff, stale JSDoc) - finalizePreview: drop unreachable !isOfficeBucket branch (caller already gates on hasOfficeHtmlPath, so this path is always office) - preview endpoint: drop lazy-sweep cutoff from 5min to 2min — anything past the 60s render ceiling is definitively orphaned, and per-request sweep can be tighter than the per-instance boot sweep - strip stale `isSubmitting` references from JSDoc in 3 spots (the client-side gate was removed in 9a65840) Skipped: function-length (#3) and client-side polling cap (#4) — refactors without correctness/perf wins; remaining NITs. * 🧹 fix: trim 1 query off pending polls + clear stale lifecycle on cross-shape updates - Preview endpoint: reuse fileAccess middleware's record for the lifecycle check; only re-fetch with text on the terminal ready response. Cuts the typical poll lifecycle from 2(N+1) to N+1 queries, since the vast majority of polls hit while pending and don't need text at all. - processCodeOutput non-office branch: explicitly null out status, previewError, previewRevision (codex P2). Without this, an update at the same (filename, conversationId) where the prior emit was an office file leaves stale lifecycle fields and the client renders the wrong state for the now non-office artifact. - Tests: rewire preview.spec mocks for the new shape, add boundary test pinning the 2min cutoff, add regression test for the cross-shape update. * 🐛 fix: keep polling on transient errors but cap permanently-broken endpoint Codex P2: the previous `data?.status === 'pending' ? 2500 : false` gate killed polling on the first transient error. With `retry: false`, a 500 left `data` undefined, the callback returned false, and the chip was stuck "Preparing preview…" forever — exactly the bug the polling layer was supposed to recover from. Inverts the gate: stop on terminal success (`ready`/`failed`) or after 5 consecutive errors. Transient errors keep retrying; a permanently broken endpoint caps at ~12.5s instead of polling forever. Predicate extracted as `previewRefetchInterval` for direct unit testing without fighting React Query's timer machinery. * ✨ feat: render pending-preview files in their own row Pending deferred-preview chips now bucket into a separate row above the resolved attachments — reads as "this is still happening" rather than mixing with completed downloads. Once status flips to ready, the chip re-buckets into panelArtifacts; failed re-buckets into the file row alongside other downloads. * 🎨 fix: render pending-preview chips in the panel-artifact row, not the file row Previous bucketing put pending chips in the file row (since `artifactTypeForAttachment` returns null for empty-text records). The pending placeholder is a future panel artifact — sharing the row keeps the chip in place when it resolves instead of jumping rows. Plain files still get their own row. * 🐛 fix: phase-1 SSE replay must not regress a resolved attachment Codex P1: useEventHandlers.finalHandler iterates responseMessage.attachments at stream end and dispatches each through the attachment handler. Those records are the immediate-persist snapshot (status:pending, text:null) — if a deferred update has already moved the same file_id to ready/failed, the existing merge let the pending fields win and downgraded the resolved record. Result: chip flickers back to pending and polling restarts until the lazy sweep corrects. Pin the terminal lifecycle fields (status, text, textFormat, previewError) when existing is ready/failed and incoming is pending. Other field updates still go through. * 🐛 fix: track preview-poll error cap outside React Query state Codex P2: the previous cap relied on `query.state.fetchFailureCount`, but React Query v4's reducer resets that to 0 on every fetch dispatch (the `'fetch'` action). With `retry: false`, each failed poll left count at 1 and the next dispatch reset it back to 0, so the `>= 5` branch never fired and a permanently-broken endpoint polled forever. Track consecutive errors in a module-level Map keyed by file_id, incremented in a thin `fetchFilePreview` wrapper around the data service call. The Map is cleared on success and on cap-stop, so memory is bounded by in-flight pending file_ids per session.

* 🏗️ refactor: Derive App Version from Root package.json + Add buildInfo Schema The hardcoded `Constants.VERSION` in `data-provider` is now replaced at rollup build time via `@rollup/plugin-replace`, sourcing from the root `package.json` so version bumps are a single-file change. Adds the shape needed by the rest of the series: - `interface.buildInfo` boolean flag (default `true`) — lets self-hosters opt out of exposing commit/branch/date. - `buildInfo` on `TStartupConfig` — commit/commitShort/branch/buildDate. - `SettingsTabValues.ABOUT` — new settings tab enum value. Ref: danny-avila#12406 * 🛠️ feat: Add Build Metadata Resolver and Expose via /api/config Adds `resolveBuildInfo()` in `@librechat/api` that surfaces commit SHA, branch, and build date from (in order) `BUILD_*` env vars, then local git metadata. Result is cached per-process. `/api/config` includes a `buildInfo` field on both authenticated and anonymous responses when `interface.buildInfo !== false` and at least one resolver field is populated. Omitted entirely otherwise. Designed so pre-built Docker images carry metadata via build-arg while source installs pick it up from `.git` — no manual version tracking. Ref: danny-avila#12406 * ℹ️ feat: Add Settings → About Panel with Diagnostics Copy New Settings tab that renders the running build's version, commit (short SHA), branch, and build date in a monospaced block alongside a "Copy diagnostics" button that emits a preformatted text blob for pasting into support issues. Tab is hidden when `interface.buildInfo` is set to `false`. Reads from `startupConfig.buildInfo` provided by `/api/config`. Ref: danny-avila#12406 * 🐳 ci: Inject BUILD_COMMIT/BRANCH/DATE into Docker Images Adds optional `BUILD_COMMIT`, `BUILD_BRANCH`, `BUILD_DATE` ARGs to both `Dockerfile` and `Dockerfile.multi`, wired as `ENV` vars in the runtime stage so the backend's `resolveBuildInfo` picks them up. All image-publishing workflows (`tag`, `main`, `dev`, `dev-branch`, `dev-staging`) now compute `${github.sha}`, `${github.ref_name}`, and a UTC timestamp, then pass them to `docker/build-push-action` as `build-args`. Defaults are empty — non-CI builds (local `docker build`) still work, and the backend falls back to local `.git` metadata if ARGs aren't set. Ref: danny-avila#12406 * 📝 docs: Direct Bug Reporters to Settings → About for Version Info The previous instructions (`docker images | grep librechat`, `git rev-parse HEAD`) only worked for a subset of deployments and rarely produced a commit SHA for users pulling pre-built images. Point users to the new in-app Settings → About panel's "Copy diagnostics" button, which captures version, commit, branch, build date, and user agent in a single preformatted block. Fallback instructions preserved for older installs. Ref: danny-avila#12406 * 🐳 fix: Move BUILD_* ENV to End of Docker Stages to Preserve Layer Cache Per-commit BUILD_COMMIT/BUILD_DATE changes were being promoted to ENV before `npm ci` / `npm run frontend` (single-stage) and before `npm ci --omit=dev` (multi-stage api-build), which invalidated the cache for every subsequent layer on every CI run. Move the ARG/ENV block below the heavy install and build steps in both Dockerfiles. Metadata is still available in the runtime image but no longer busts layer reuse. Addresses codex review on danny-avila#12756. * 🔧 fix: Propagate interface.buildInfo=false to Unauthenticated /api/config The unauthenticated branch of `/api/config` was emitting an `interface` object only when `privacyPolicy` or `termsOfService` was set, which meant an admin's explicit `interface.buildInfo: false` opt-out was never visible to anonymous/guest clients. `Settings.tsx` gates the About tab on `startupConfig?.interface?.buildInfo !== false`, so a missing field fell through as "enabled" for those clients. Include `interface.buildInfo: false` in the unauth payload whenever it's explicitly disabled. Keep the implicit default (true) absent to preserve the minimal-unauth-payload convention. Addresses codex review on danny-avila#12756. * 🔀 ci: Trigger Dev Image Workflows on Root package.json + Dockerfile Changes The baked `Constants.VERSION` now reads from the root `package.json` via rollup-plugin-replace, but the `dev-images.yml` and `dev-branch-images.yml` path filters only matched `api/**`, `client/**`, `packages/**`. A release commit that only bumps root `package.json` would not trigger a rebuild, leaving `latest` dev images with stale Footer/About version metadata. Include `package.json`, `package-lock.json`, and both Dockerfiles in the path filters so dependency changes (lockfile rebuilds) and image build tweaks also rebuild dev images. Addresses codex review on danny-avila#12756. * 🧽 fix: Harden About Panel Lifecycle, A11y, and Loading Gate Review follow-ups on danny-avila#12756: - #1 timer leak: stash the copy-state `setTimeout` in a ref and clear it from a `useEffect` cleanup so unmounting the Settings dialog mid-toast doesn't fire `setCopied(false)` on an unmounted component. - #3 flash of About tab: gate `aboutEnabled` on `startupConfig != null` so the tab stays hidden until `/api/config` returns. For admins who disabled `interface.buildInfo`, the tab no longer briefly appears and vanishes on page load. - danny-avila#6 aria-live placement: move the live region off the interactive button onto a dedicated `<span role="status" aria-live="polite">` so screen readers announce the copied state, not the full button content on every re-render. - #2 missing coverage: add `About.spec.tsx` exercising populated/empty buildInfo rendering, invalid-date handling, diagnostics clipboard payload, copy-state toggling, unmount cleanup, and the live region. * ⚡ perf: Eagerly Resolve Build Info at Module Load Review follow-up #4 on danny-avila#12756: `resolveBuildInfo()` calls `execFileSync` with a 2s timeout on source installs without `BUILD_*` env vars. Paying this cost on the first HTTP request blocks the event loop mid-flight. Call `resolveBuildInfo()` once at config route module load so the resolver's cache is warm before any request arrives. Docker images with the BUILD_* env vars set sidestep the git path entirely, so this only affects the edge case of source installs. * 📝 docs: Document rollup Version Placeholder Contract Review follow-ups danny-avila#5 / danny-avila#8 on danny-avila#12756. The `__LIBRECHAT_VERSION__` placeholder relies on a substring replacement rule that only works because the token appears inside a string literal, and the substitution only runs during `npm run build:data-provider`. - Expand the `Constants.VERSION` JSDoc to spell out that consumers read the placeholder through the built dist bundle; source-level test imports would see the raw placeholder. - Add a NOTE above the rollup `replace` config warning future contributors not to repurpose the token as a bare identifier without switching to a quoted replacement value. Non-functional; prevents future contributors from stepping on a subtle constraint. * 🪪 fix: Only Toast "Copied" When Clipboard Copy Actually Succeeds Codex R5 on danny-avila#12756. `copy-to-clipboard` returns a boolean indicating whether the underlying `execCommand('copy')` / fallback prompt actually wrote to the clipboard. The previous handler flipped to the "Copied" state unconditionally, which in hardened browsers or when the permission prompt is dismissed would mislead users into filing bug reports without the diagnostics blob attached. Gate the state/timer/live-region on the boolean return; silently no-op on failure rather than showing a false positive. Adds a test asserting the button label stays at "Copy diagnostics" when the clipboard call fails. * 🐳 fix: Derive main image metadata from checkout * 🪪 fix: Keep About enabled until disabled * ✅ test: Avoid literal Settings mock text * 🧱 refactor: Rename Build Info Module

github-advanced-security AI found potential problems Oct 15, 2025

View reviewed changes

marlonka closed this in ccd049d May 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Render thinking summaries as markdown#4

feat: Render thinking summaries as markdown#4
google-labs-jules[bot] wants to merge 1 commit into
mainfrom
feat/markdown-thinking-summaries

google-labs-jules Bot commented Oct 15, 2025

Uh oh!

google-labs-jules Bot commented Oct 15, 2025

Uh oh!

Check failure

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant