LibreChat/api/server/services/ActionService.spec.js

540 lines
19 KiB
JavaScript
Raw Normal View History

🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
const { Constants, actionDelimiter, actionDomainSeparator } = require('librechat-data-provider');
const { domainParser, legacyDomainEncode, validateAndUpdateTool } = require('./ActionService');
jest.mock('keyv');
⚗️ feat: Agent Context Compaction/Summarization (#12287) * chore: imports/types Add summarization config and package-level summarize handler contracts Register summarize handlers across server controller paths Port cursor dual-read/dual-write summary support and UI status handling Selectively merge cursor branch files for BaseClient summary content block detection (last-summary-wins), dual-write persistence, summary block unit tests, and on_summarize_status SSE event handling with started/completed/failed branches. Co-authored-by: Cursor <cursoragent@cursor.com> refactor: type safety feat: add localization for summarization status messages refactor: optimize summary block detection in BaseClient Updated the logic for identifying existing summary content blocks to use a reverse loop for improved efficiency. Added a new test case to ensure the last summary content block is updated correctly when multiple summary blocks exist. chore: add runName to chainOptions in AgentClient refactor: streamline summarization configuration and handler integration Removed the deprecated summarizeNotConfigured function and replaced it with a more flexible createSummarizeFn. Updated the summarization handler setup across various controllers to utilize the new function, enhancing error handling and configuration resolution. Improved overall code clarity and maintainability by consolidating summarization logic. feat(summarization): add staged chunk-and-merge fallback feat(usage): track summarization usage separately from messages feat(summarization): resolve prompt from config in runtime fix(endpoints): use @librechat/api provider config loader refactor(agents): import getProviderConfig from @librechat/api chore: code order feat(app-config): auto-enable summarization when configured feat: summarization config refactor(summarization): streamline persist summary handling and enhance configuration validation Removed the deprecated createDeferredPersistSummary function and integrated a new createPersistSummary function for MongoDB persistence. Updated summarization handlers across various controllers to utilize the new persistence method. Enhanced validation for summarization configuration to ensure provider, model, and prompt are properly set, improving error handling and overall robustness. refactor(summarization): update event handling and remove legacy summarize handlers Replaced the deprecated summarization handlers with new event-driven handlers for summarization start and completion across multiple controllers. This change enhances the clarity of the summarization process and improves the integration of summarization events in the application. Additionally, removed unused summarization functions and streamlined the configuration loading process. refactor(summarization): standardize event names in handlers Updated event names in the summarization handlers to use constants from GraphEvents for consistency and clarity. This change improves maintainability and reduces the risk of errors related to string literals in event handling. feat(summarization): enhance usage tracking for summarization events Added logic to track summarization usage in multiple controllers by checking the current node type. If the node indicates a summarization task, the usage type is set accordingly. This change improves the granularity of usage data collected during summarization processes. feat(summarization): integrate SummarizationConfig into AppSummarizationConfig type Enhanced the AppSummarizationConfig type by extending it with the SummarizationConfig type from librechat-data-provider. This change improves type safety and consistency in the summarization configuration structure. test: add end-to-end tests for summarization functionality Introduced a comprehensive suite of end-to-end tests for the summarization feature, covering the full LibreChat pipeline from message creation to summarization. This includes a new setup file for environment configuration and a Jest configuration specifically for E2E tests. The tests utilize real API keys and ensure proper integration with the summarization process, enhancing overall test coverage and reliability. refactor(summarization): include initial summary in formatAgentMessages output Updated the formatAgentMessages function to return an initial summary alongside messages and index token count map. This change is reflected in multiple controllers and the corresponding tests, enhancing the summarization process by providing additional context for each agent's response. refactor: move hydrateMissingIndexTokenCounts to tokenMap utility Extracted the hydrateMissingIndexTokenCounts function from the AgentClient and related tests into a new tokenMap utility file. This change improves code organization and reusability, allowing for better management of token counting logic across the application. refactor(summarization): standardize step event handling and improve summary rendering Refactored the step event handling in the useStepHandler and related components to utilize constants for event names, enhancing consistency and maintainability. Additionally, improved the rendering logic in the Summary component to conditionally display the summary text based on its availability, providing a better user experience during the summarization process. feat(summarization): introduce baseContextTokens and reserveTokensRatio for improved context management Added baseContextTokens to the InitializedAgent type to calculate the context budget based on agentMaxContextNum and maxOutputTokensNum. Implemented reserveTokensRatio in the createRun function to allow configurable context token management. Updated related tests to validate these changes and ensure proper functionality. feat(summarization): add minReserveTokens, context pruning, and overflow recovery configurations Introduced new configuration options for summarization, including minReserveTokens, context pruning settings, and overflow recovery parameters. Updated the createRun function to accommodate these new options and added a comprehensive test suite to validate their functionality and integration within the summarization process. feat(summarization): add updatePrompt and reserveTokensRatio to summarization configuration Introduced an updatePrompt field for updating existing summaries with new messages, enhancing the flexibility of the summarization process. Additionally, added reserveTokensRatio to the configuration schema, allowing for improved management of token allocation during summarization. Updated related tests to validate these new features. feat(logging): add on_agent_log event handler for structured logging Implemented an on_agent_log event handler in both the agents' callbacks and responses to facilitate structured logging of agent activities. This enhancement allows for better tracking and debugging of agent interactions by logging messages with associated metadata. Updated the summarization process to ensure proper handling of log events. fix: remove duplicate IBalanceUpdate interface declaration perf(usage): single-pass partition of collectedUsage Replace two Array.filter() passes with a single for-of loop that partitions message vs. summarization usages in one iteration. fix(BaseClient): shallow-copy message content before mutating and preserve string content Avoid mutating the original message.content array in-place when appending a summary block. Also convert string content to a text content part instead of silently discarding it. fix(ui): fix Part.tsx indentation and useStepHandler summarize-complete handling - Fix SUMMARY else-if branch indentation in Part.tsx to match chain level - Guard ON_SUMMARIZE_COMPLETE with didFinalize flag to avoid unnecessary re-renders when no summarizing parts exist - Protect against undefined completeData.summary instead of unsafe spread fix(agents): use strict enabled check for summarization handlers Change summarizationConfig?.enabled !== false to === true so handlers are not registered when summarizationConfig is undefined. chore: fix initializeClient JSDoc and move DEFAULT_RESERVE_RATIO to module scope refactor(Summary): align collapse/expand behavior with Reasoning component - Single render path instead of separate streaming vs completed branches - Use useMessageContext for isSubmitting/isLatestMessage awareness so the "Summarizing..." label only shows during active streaming - Default to collapsed (matching Reasoning), user toggles to expand - Add proper aria attributes (aria-hidden, role, aria-controls, contentId) - Hide copy button while actively streaming feat(summarization): default to self-summarize using agent's own provider/model When no summarization config is provided (neither in librechat.yaml nor on the agent), automatically enable summarization using the agent's own provider and model. The agents package already provides default prompts, so no prompt configuration is needed. Also removes the dead resolveSummarizationLLMConfig in summarize.ts (and its spec) — run.ts buildAgentContext is the single source of truth for summarization config resolution. Removes the duplicate RuntimeSummarizationConfig local type in favor of the canonical SummarizationConfig from data-provider. chore: schema and type cleanup for summarization - Add trigger field to summarizationAgentOverrideSchema so per-agent trigger overrides in librechat.yaml are not silently stripped by Zod - Remove unused SummarizationStatus type from runs.ts - Make AppSummarizationConfig.enabled non-optional to reflect the invariant that loadSummarizationConfig always sets it refactor(responses): extract duplicated on_agent_log handler refactor(run): use agents package types for summarization config Import SummarizationConfig, ContextPruningConfig, and OverflowRecoveryConfig from @librechat/agents and use them to type-check the translation layer in buildAgentContext. This ensures the config object passed to the agent graph matches what it expects. - Use `satisfies AgentSummarizationConfig` on the config object - Cast contextPruningConfig and overflowRecoveryConfig to agents types - Properly narrow trigger fields from DeepPartial to required shape feat(config): add maxToolResultChars to base endpoint schema Add maxToolResultChars to baseEndpointSchema so it can be configured on any endpoint in librechat.yaml. Resolved during agent initialization using getProviderConfig's endpoint resolution: custom endpoint config takes precedence, then the provider-specific endpoint config, then the shared `all` config. Passed through to the agents package ToolNode, which uses it to cap tool result length before it enters the context window. When not configured, the agents package computes a sensible default from maxContextTokens. fix(summarization): forward agent model_parameters in self-summarize default When no explicit summarization config exists, the self-summarize default now forwards the agent's model_parameters as the summarization parameters. This ensures provider-specific settings (e.g. Bedrock region, credentials, endpoint host) are available when the agents package constructs the summarization LLM. fix(agents): register summarization handlers by default Change the enabled gate from === true to !== false so handlers register when no explicit summarization config exists. This aligns with the self-summarize default where summarization is always on unless explicitly disabled via enabled: false. refactor(summarization): let agents package inherit clientOptions for self-summarize Remove model_parameters forwarding from the self-summarize default. The agents package now reuses the agent's own clientOptions when the summarization provider matches the agent's provider, inheriting all provider-specific settings (region, credentials, proxy, etc.) automatically. refactor(summarization): use MessageContentComplex[] for summary content Unify summary content to always use MessageContentComplex[] arrays, matching the pattern used by on_message_delta. No more string | array unions — content is always an array of typed blocks ({ type: 'text', text: '...' } for text, { type: 'reasoning_content', ... } for reasoning). Agents package: - SummaryContentBlock.content: MessageContentComplex[] (was string) - tokenCount now optional (not sent on deltas) - Removed reasoning field — reasoning is now a content block type - streamAndCollect normalizes all chunks to content block arrays - Delta events pass content blocks directly LibreChat: - SummaryContentPart.content: Agents.MessageContentComplex[] - Updated Part.tsx, Summary.tsx, useStepHandler.ts, BaseClient.js - Summary.tsx derives display text from content blocks via useMemo - Aggregator uses simple array spread refactor(summarization): enhance summary handling and text extraction - Updated BaseClient.js to improve summary text extraction, accommodating both legacy and new content formats. - Modified summarization logic to ensure consistent handling of summary content across different message formats. - Adjusted test cases in summarization.e2e.spec.js to utilize the new summary text extraction method. - Refined SSE useStepHandler to initialize summary content as an array. - Updated configuration schema by removing unused minReserveTokens field. - Cleaned up SummaryContentPart type by removing rangeHash property. These changes streamline the summarization process and ensure compatibility with various content structures. refactor(summarization): streamline usage tracking and logging - Removed direct checks for summarization nodes in ModelEndHandler and replaced them with a dedicated markSummarizationUsage function for better readability and maintainability. - Updated OpenAIChatCompletionController and responses handlers to utilize the new markSummarizationUsage function for setting usage types. - Enhanced logging functionality by ensuring the logger correctly handles different log levels. - Introduced a new useCopyToClipboard hook in the Summary component to encapsulate clipboard copy logic, improving code reusability and clarity. These changes improve the overall structure and efficiency of the summarization handling and logging processes. refactor(summarization): update summary content block documentation - Removed outdated comment regarding the last summary content block in BaseClient.js. - Added a new comment to clarify the purpose of the findSummaryContentBlock method, ensuring consistency in documentation. These changes enhance code clarity and maintainability by providing accurate descriptions of the summarization logic. refactor(summarization): update summary content structure in tests - Modified the summarization content structure in e2e tests to use an array format for text, aligning with recent changes in summary handling. - Updated test descriptions to clarify the behavior of context token calculations, ensuring consistency and clarity in the tests. These changes enhance the accuracy and maintainability of the summarization tests by reflecting the updated content structure. refactor(summarization): remove legacy E2E test setup and configuration - Deleted the e2e-setup.js and jest.e2e.config.js files, which contained legacy configurations for E2E tests using real API keys. - Introduced a new summarization.e2e.ts file that implements comprehensive E2E backend integration tests for the summarization process, utilizing real AI providers and tracking summaries throughout the run. These changes streamline the testing framework by consolidating E2E tests into a single, more robust file while removing outdated configurations. refactor(summarization): enhance E2E tests and error handling - Added a cleanup step to force exit after all tests to manage Redis connections. - Updated the summarization model to 'claude-haiku-4-5-20251001' for consistency across tests. - Improved error handling in the processStream function to capture and return processing errors. - Enhanced logging for cross-run tests and tight context scenarios to provide better insights into test execution. These changes improve the reliability and clarity of the E2E tests for the summarization process. refactor(summarization): enhance test coverage for maxContextTokens behavior - Updated run-summarization.test.ts to include a new test case ensuring that maxContextTokens does not exceed user-defined limits, even when calculated ratios suggest otherwise. - Modified summarization.e2e.ts to replace legacy UsageMetadata type with a more appropriate type for collectedUsage, improving type safety and clarity in the test setup. These changes improve the robustness of the summarization tests by validating context token constraints and refining type definitions. feat(summarization): add comprehensive E2E tests for summarization process - Introduced a new summarization.e2e.test.ts file that implements extensive end-to-end integration tests for the summarization pipeline, covering the full flow from LibreChat to agents. - The tests utilize real AI providers and include functionality to track summaries during and between runs. - Added necessary cleanup steps to manage Redis connections post-tests and ensure proper exit. These changes enhance the testing framework by providing robust coverage for the summarization process, ensuring reliability and performance under real-world conditions. fix(service): import logger from winston configuration - Removed the import statement for logger from '@librechat/data-schemas' and replaced it with an import from '~/config/winston'. - This change ensures that the logger is correctly sourced from the updated configuration, improving consistency in logging practices across the application. refactor(summary): simplify Summary component and enhance token display - Removed the unused `meta` prop from the `SummaryButton` component to streamline its interface. - Updated the token display logic to use a localized string for better internationalization support. - Adjusted the rendering of the `meta` information to improve its visibility within the `Summary` component. These changes enhance the clarity and usability of the Summary component while ensuring better localization practices. feat(summarization): add maxInputTokens configuration for summarization - Introduced a new `maxInputTokens` property in the summarization configuration schema to control the amount of conversation context sent to the summarizer, with a default value of 10000. - Updated the `createRun` function to utilize the new `maxInputTokens` setting, allowing for more flexible summarization based on agent context. These changes enhance the summarization capabilities by providing better control over input token limits, improving the overall summarization process. refactor(summarization): simplify maxInputTokens logic in createRun function - Updated the logic for the `maxInputTokens` property in the `createRun` function to directly use the agent's base context tokens when the resolved summarization configuration does not specify a value. - This change streamlines the configuration process and enhances clarity in how input token limits are determined for summarization. These modifications improve the maintainability of the summarization configuration by reducing complexity in the token calculation logic. feat(summary): enhance Summary component to display meta information - Updated the SummaryContent component to accept an optional `meta` prop, allowing for additional contextual information to be displayed above the main content. - Adjusted the rendering logic in the Summary component to utilize the new `meta` prop, improving the visibility of supplementary details. These changes enhance the user experience by providing more context within the Summary component, making it clearer and more informative. refactor(summarization): standardize reserveRatio configuration in summarization logic - Replaced instances of `reserveTokensRatio` with `reserveRatio` in the `createRun` function and related tests to unify the terminology across the codebase. - Updated the summarization configuration schema to reflect this change, ensuring consistency in how the reserve ratio is defined and utilized. - Removed the per-agent override logic for summarization configuration, simplifying the overall structure and enhancing clarity. These modifications improve the maintainability and readability of the summarization logic by standardizing the configuration parameters. * fix: circular dependency of `~/models` * chore: update logging scope in agent log handlers Changed log scope from `[agentus:${data.scope}]` to `[agents:${data.scope}]` in both the callbacks and responses controllers to ensure consistent logging format across the application. * feat: calibration ratio * refactor(tests): update summarizationConfig tests to reflect changes in enabled property Modified tests to check for the new `summarizationEnabled` property instead of the deprecated `enabled` field in the summarization configuration. This change ensures that the tests accurately validate the current configuration structure and behavior of the agents. * feat(tests): add markSummarizationUsage mock for improved test coverage Introduced a mock for the markSummarizationUsage function in the responses unit tests to enhance the testing of summarization usage tracking. This addition supports better validation of summarization-related functionalities and ensures comprehensive test coverage for the agents' response handling. * refactor(tests): simplify event handler setup in createResponse tests Removed redundant mock implementations for event handlers in the createResponse unit tests, streamlining the setup process. This change enhances test clarity and maintainability while ensuring that the tests continue to validate the correct behavior of usage tracking during on_chat_model_end events. * refactor(agents): move calibration ratio capture to finally block Reorganized the logic for capturing the calibration ratio in the AgentClient class to ensure it is executed in the finally block. This change guarantees that the ratio is captured even if the run is aborted, enhancing the reliability of the response message persistence. Removed redundant code and improved clarity in the handling of context metadata. * refactor(agents): streamline bulk write logic in recordCollectedUsage function Removed redundant bulk write operations and consolidated document handling in the recordCollectedUsage function. The logic now combines all documents into a single bulk write operation, improving efficiency and reducing error handling complexity. Updated logging to provide consistent error messages for bulk write failures. * refactor(agents): enhance summarization configuration resolution in createRun function Streamlined the summarization configuration logic by introducing a base configuration and allowing for overrides from agent-specific settings. This change improves clarity and maintainability, ensuring that the summarization configuration is consistently applied while retaining flexibility for customization. Updated the handling of summarization parameters to ensure proper integration with the agent's model and provider settings. * refactor(agents): remove unused tokenCountMap and streamline calibration ratio handling Eliminated the unused tokenCountMap variable from the AgentClient class to enhance code clarity. Additionally, streamlined the logic for capturing the calibration ratio by using optional chaining and a fallback value, ensuring that context metadata is consistently defined. This change improves maintainability and reduces potential confusion in the codebase. * refactor(agents): extract agent log handler for improved clarity and reusability Refactored the agent log handling logic by extracting it into a dedicated function, `agentLogHandler`, enhancing code clarity and reusability across different modules. Updated the event handlers in both the OpenAI and responses controllers to utilize the new handler, ensuring consistent logging behavior throughout the application. * test: add summarization event tests for useStepHandler Implemented a series of tests for the summarization events in the useStepHandler hook. The tests cover scenarios for ON_SUMMARIZE_START, ON_SUMMARIZE_DELTA, and ON_SUMMARIZE_COMPLETE events, ensuring proper handling of summarization logic, including message accumulation and finalization. This addition enhances test coverage and validates the correct behavior of the summarization process within the application. * refactor(config): update summarizationTriggerSchema to use enum for type validation Changed the type of the `type` field in the summarizationTriggerSchema from a string to an enum with a single value 'token_count'. This modification enhances type safety and ensures that only valid types are accepted in the configuration, improving overall clarity and maintainability of the schema. * test(usage): add bulk write tests for message and summarization usage Implemented tests for the bulk write functionality in the recordCollectedUsage function, covering scenarios for combined message and summarization usage, summarization-only usage, and message-only usage. These tests ensure correct document handling and token rollup calculations, enhancing test coverage and validating the behavior of the usage tracking logic. * refactor(Chat): enhance clipboard copy functionality and type definitions in Summary component Updated the Summary component to improve the clipboard copy functionality by handling clipboard permission errors. Refactored type definitions for SummaryProps to use a more specific type, enhancing type safety. Adjusted the SummaryButton and FloatingSummaryBar components to accept isCopied and onCopy props, promoting better separation of concerns and reusability. * chore(translations): remove unused "Expand Summary" key from English translations Deleted the "Expand Summary" key from the English translation file to streamline the localization resources and improve clarity in the user interface. This change helps maintain an organized and efficient translation structure. * refactor: adjust token counting for Claude model to account for API discrepancies Implemented a correction factor for token counting when using the Claude model, addressing discrepancies between Anthropic's API and local tokenizer results. This change ensures accurate token counts by applying a scaling factor, improving the reliability of token-related functionalities. * refactor(agents): implement token count adjustment for Claude model messages Added a method to adjust token counts for messages processed by the Claude model, applying a correction factor to align with API expectations. This enhancement improves the accuracy of token counting, ensuring reliable functionality when interacting with the Claude model. * refactor(agents): token counting for media content in messages Introduced a new method to estimate token costs for image and document blocks in messages, improving the accuracy of token counting. This enhancement ensures that media content is properly accounted for, particularly for the Claude model, by integrating additional token estimation logic for various content types. Updated the token counting function to utilize this new method, enhancing overall reliability and functionality. * chore: fix missing import * fix(agents): clamp baseContextTokens and document reserve ratio change Prevent negative baseContextTokens when maxOutputTokens exceeds the context window (misconfigured models). Document the 10%→5% default reserve ratio reduction introduced alongside summarization. * fix(agents): include media tokens in hydrated token counts Add estimateMediaTokensForMessage to createTokenCounter so the hydration path (used by hydrateMissingIndexTokenCounts) matches the precomputed path in AgentClient.getTokenCountForMessage. Without this, messages containing images or documents were systematically undercounted during hydration, risking context window overflow. Add 34 unit tests covering all block-type branches of estimateMediaTokensForMessage. * fix(agents): include summarization output tokens in usage return value The returned output_tokens from recordCollectedUsage now reflects all billed LLM calls (message + summarization). Previously, summarization completions were billed but excluded from the returned metadata, causing a discrepancy between what users were charged and what the response message reported. * fix(tests): replace process.exit with proper Redis cleanup in e2e test The summarization E2E test used process.exit(0) to work around a Redis connection opened at import time, which killed the Jest runner and bypassed teardown. Use ioredisClient.quit() and keyvRedisClient.disconnect() for graceful cleanup instead. * fix(tests): update getConvo imports in OpenAI and response tests Refactor test files to import getConvo from the main models module instead of the Conversation submodule. This change ensures consistency across tests and simplifies the import structure, enhancing maintainability. * fix(clients): improve summary text validation in BaseClient Refactor the summary extraction logic to ensure that only non-empty summary texts are considered valid. This change enhances the robustness of the message processing by utilizing a dedicated method for summary text retrieval, improving overall reliability. * fix(config): replace z.any() with explicit union in summarization schema Model parameters (temperature, top_p, etc.) are constrained to primitive types rather than the policy-violating z.any(). * refactor(agents): deduplicate CLAUDE_TOKEN_CORRECTION constant Export from the TS source in packages/api and import in the JS client, eliminating the static class property that could drift out of sync. * refactor(agents): eliminate duplicate selfProvider in buildAgentContext selfProvider and provider were derived from the same expression with different type casts. Consolidated to a single provider variable. * refactor(agents): extract shared SSE handlers and restrict log levels - buildSummarizationHandlers() factory replaces triplicated handler blocks across responses.js and openai.js - agentLogHandlerObj exported from callbacks.js for consistent reuse - agentLogHandler restricted to an allowlist of safe log levels (debug, info, warn, error) instead of accepting arbitrary strings * fix(SSE): batch summarize deltas, add exhaustiveness check, conditional error announcement - ON_SUMMARIZE_DELTA coalesces rapid-fire renders via requestAnimationFrame instead of calling setMessages per chunk - Exhaustive never-check on TStepEvent catches unhandled variants at compile time when new StepEvents are added - ON_SUMMARIZE_COMPLETE error announcement only fires when a summary part was actually present and removed * feat(agents): persist instruction overhead in contextMeta and seed across runs Extend contextMeta with instructionOverhead and toolCount so the provider-observed instruction overhead is persisted on the response message and seeded into the pruner on subsequent runs. This enables the pruner to use a calibrated budget from the first call instead of waiting for a provider observation, preventing the ratio collapse caused by local tokenizer overestimating tool schema tokens. The seeded overhead is only used when encoding and tool count match between runs, ensuring stale values from different configurations are discarded. * test(agents): enhance OpenAI test mocks for summarization handlers Updated the OpenAI test suite to include additional mock implementations for summarization handlers, including buildSummarizationHandlers, markSummarizationUsage, and agentLogHandlerObj. This improves test coverage and ensures consistent behavior during testing. * fix(agents): address review findings for summarization v2 Cancel rAF on unmount to prevent stale Recoil writes from dead component context. Clear orphaned summarizing:true parts when ON_SUMMARIZE_COMPLETE arrives without a summary payload. Add null guard and safe spread to agentLogHandler. Handle Anthropic-format base64 image/* documents in estimateMediaTokensForMessage. Use role="region" for expandable summary content. Add .describe() to contextMeta Zod fields. Extract duplicate usage loop into helper. * refactor: simplify contextMeta to calibrationRatio + encoding only Remove instructionOverhead and toolCount from cross-run persistence — instruction tokens change too frequently between runs (prompt edits, tool changes) for a persisted seed to be reliable. The intra-run calibration in the pruner still self-corrects via provider observations. contextMeta now stores only the tokenizer-bias ratio and encoding, which are stable across instruction changes. * test(SSE): enhance useStepHandler tests for ON_SUMMARIZE_COMPLETE behavior Updated the test for ON_SUMMARIZE_COMPLETE to clarify that it finalizes the existing part with summarizing set to false when the summary is undefined. Added assertions to verify the correct behavior of message updates and the state of summary parts. * refactor(BaseClient): remove handleContextStrategy and truncateToolCallOutputs functions Eliminated the handleContextStrategy method from BaseClient to streamline message handling. Also removed the truncateToolCallOutputs function from the prompts module, simplifying the codebase and improving maintainability. * refactor: add AGENT_DEBUG_LOGGING option and refactor token count handling in BaseClient Introduced AGENT_DEBUG_LOGGING to .env.example for enhanced debugging capabilities. Refactored token count handling in BaseClient by removing the handleTokenCountMap method and simplifying token count updates. Updated AgentClient to log detailed token count recalculations and adjustments, improving traceability during message processing. * chore: update dependencies in package-lock.json and package.json files Bumped versions of several dependencies, including @librechat/agents to ^3.1.62 and various AWS SDK packages to their latest versions. This ensures compatibility and incorporates the latest features and fixes. * chore: imports order * refactor: extract summarization config resolution from buildAgentContext * refactor: rename and simplify summarization configuration shaping function * refactor: replace AgentClient token counting methods with single-pass pure utility Extract getTokenCount() and getTokenCountForMessage() from AgentClient into countFormattedMessageTokens(), a pure function in packages/api that handles text, tool_call, image, and document content types in one loop. - Decompose estimateMediaTokensForMessage into block-level helpers (estimateImageDataTokens, estimateImageBlockTokens, estimateDocumentBlockTokens) shared by both estimateMediaTokensForMessage and the new single-pass function - Remove redundant per-call getEncoding() resolution (closure captures once) - Remove deprecated gpt-3.5-turbo-0301 model branching - Drop this.getTokenCount guard from BaseClient.sendMessage * refactor: streamline token counting in createTokenCounter function Simplified the createTokenCounter function by removing the media token estimation and directly calculating the token count. This change enhances clarity and performance by consolidating the token counting logic into a single pass, while maintaining compatibility with Claude's token correction. * refactor: simplify summarization configuration types Removed the AppSummarizationConfig type and directly used SummarizationConfig in the AppConfig interface. This change streamlines the type definitions and enhances consistency across the codebase. * chore: import order * fix: summarization event handling in useStepHandler - Cancel pending summarizeDeltaRaf in clearStepMaps to prevent stale frames firing after map reset or component unmount - Move announcePolite('summarize_completed') inside the didFinalize guard so screen readers only announce when finalization actually occurs - Remove dead cleanup closure returned from stepHandler useCallback body that was never invoked by any caller * fix: estimate tokens for non-PDF/non-image base64 document blocks Previously estimateDocumentBlockTokens returned 0 for unrecognized MIME types (e.g. text/plain, application/json), silently underestimating context budget. Fall back to character-based heuristic or countTokens. * refactor: return cloned usage from markSummarizationUsage Avoid mutating LangChain's internal usage_metadata object by returning a shallow clone with the usage_type tag. Update all call sites in callbacks, openai, and responses controllers to use the returned value. * refactor: consolidate debug logging loops in buildMessages Merge the two sequential O(n) debug-logging passes over orderedMessages into a single pass inside the map callback where all data is available. * refactor: narrow SummaryContentPart.content type Replace broad Agents.MessageContentComplex[] with the specific Array<{ type: ContentTypes.TEXT; text: string }> that all producers and consumers already use, improving compile-time safety. * refactor: use single output array in recordCollectedUsage Have processUsageGroup append to a shared array instead of returning separate arrays that are spread into a third, reducing allocations. * refactor: use for...in in hydrateMissingIndexTokenCounts Replace Object.entries with for...in to avoid allocating an intermediate tuple array during token map hydration.
2026-03-21 12:03:10 -04:00
jest.mock('~/models', () => ({
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
getActions: jest.fn(),
deleteActions: jest.fn(),
}));
⚗️ feat: Agent Context Compaction/Summarization (#12287) * chore: imports/types Add summarization config and package-level summarize handler contracts Register summarize handlers across server controller paths Port cursor dual-read/dual-write summary support and UI status handling Selectively merge cursor branch files for BaseClient summary content block detection (last-summary-wins), dual-write persistence, summary block unit tests, and on_summarize_status SSE event handling with started/completed/failed branches. Co-authored-by: Cursor <cursoragent@cursor.com> refactor: type safety feat: add localization for summarization status messages refactor: optimize summary block detection in BaseClient Updated the logic for identifying existing summary content blocks to use a reverse loop for improved efficiency. Added a new test case to ensure the last summary content block is updated correctly when multiple summary blocks exist. chore: add runName to chainOptions in AgentClient refactor: streamline summarization configuration and handler integration Removed the deprecated summarizeNotConfigured function and replaced it with a more flexible createSummarizeFn. Updated the summarization handler setup across various controllers to utilize the new function, enhancing error handling and configuration resolution. Improved overall code clarity and maintainability by consolidating summarization logic. feat(summarization): add staged chunk-and-merge fallback feat(usage): track summarization usage separately from messages feat(summarization): resolve prompt from config in runtime fix(endpoints): use @librechat/api provider config loader refactor(agents): import getProviderConfig from @librechat/api chore: code order feat(app-config): auto-enable summarization when configured feat: summarization config refactor(summarization): streamline persist summary handling and enhance configuration validation Removed the deprecated createDeferredPersistSummary function and integrated a new createPersistSummary function for MongoDB persistence. Updated summarization handlers across various controllers to utilize the new persistence method. Enhanced validation for summarization configuration to ensure provider, model, and prompt are properly set, improving error handling and overall robustness. refactor(summarization): update event handling and remove legacy summarize handlers Replaced the deprecated summarization handlers with new event-driven handlers for summarization start and completion across multiple controllers. This change enhances the clarity of the summarization process and improves the integration of summarization events in the application. Additionally, removed unused summarization functions and streamlined the configuration loading process. refactor(summarization): standardize event names in handlers Updated event names in the summarization handlers to use constants from GraphEvents for consistency and clarity. This change improves maintainability and reduces the risk of errors related to string literals in event handling. feat(summarization): enhance usage tracking for summarization events Added logic to track summarization usage in multiple controllers by checking the current node type. If the node indicates a summarization task, the usage type is set accordingly. This change improves the granularity of usage data collected during summarization processes. feat(summarization): integrate SummarizationConfig into AppSummarizationConfig type Enhanced the AppSummarizationConfig type by extending it with the SummarizationConfig type from librechat-data-provider. This change improves type safety and consistency in the summarization configuration structure. test: add end-to-end tests for summarization functionality Introduced a comprehensive suite of end-to-end tests for the summarization feature, covering the full LibreChat pipeline from message creation to summarization. This includes a new setup file for environment configuration and a Jest configuration specifically for E2E tests. The tests utilize real API keys and ensure proper integration with the summarization process, enhancing overall test coverage and reliability. refactor(summarization): include initial summary in formatAgentMessages output Updated the formatAgentMessages function to return an initial summary alongside messages and index token count map. This change is reflected in multiple controllers and the corresponding tests, enhancing the summarization process by providing additional context for each agent's response. refactor: move hydrateMissingIndexTokenCounts to tokenMap utility Extracted the hydrateMissingIndexTokenCounts function from the AgentClient and related tests into a new tokenMap utility file. This change improves code organization and reusability, allowing for better management of token counting logic across the application. refactor(summarization): standardize step event handling and improve summary rendering Refactored the step event handling in the useStepHandler and related components to utilize constants for event names, enhancing consistency and maintainability. Additionally, improved the rendering logic in the Summary component to conditionally display the summary text based on its availability, providing a better user experience during the summarization process. feat(summarization): introduce baseContextTokens and reserveTokensRatio for improved context management Added baseContextTokens to the InitializedAgent type to calculate the context budget based on agentMaxContextNum and maxOutputTokensNum. Implemented reserveTokensRatio in the createRun function to allow configurable context token management. Updated related tests to validate these changes and ensure proper functionality. feat(summarization): add minReserveTokens, context pruning, and overflow recovery configurations Introduced new configuration options for summarization, including minReserveTokens, context pruning settings, and overflow recovery parameters. Updated the createRun function to accommodate these new options and added a comprehensive test suite to validate their functionality and integration within the summarization process. feat(summarization): add updatePrompt and reserveTokensRatio to summarization configuration Introduced an updatePrompt field for updating existing summaries with new messages, enhancing the flexibility of the summarization process. Additionally, added reserveTokensRatio to the configuration schema, allowing for improved management of token allocation during summarization. Updated related tests to validate these new features. feat(logging): add on_agent_log event handler for structured logging Implemented an on_agent_log event handler in both the agents' callbacks and responses to facilitate structured logging of agent activities. This enhancement allows for better tracking and debugging of agent interactions by logging messages with associated metadata. Updated the summarization process to ensure proper handling of log events. fix: remove duplicate IBalanceUpdate interface declaration perf(usage): single-pass partition of collectedUsage Replace two Array.filter() passes with a single for-of loop that partitions message vs. summarization usages in one iteration. fix(BaseClient): shallow-copy message content before mutating and preserve string content Avoid mutating the original message.content array in-place when appending a summary block. Also convert string content to a text content part instead of silently discarding it. fix(ui): fix Part.tsx indentation and useStepHandler summarize-complete handling - Fix SUMMARY else-if branch indentation in Part.tsx to match chain level - Guard ON_SUMMARIZE_COMPLETE with didFinalize flag to avoid unnecessary re-renders when no summarizing parts exist - Protect against undefined completeData.summary instead of unsafe spread fix(agents): use strict enabled check for summarization handlers Change summarizationConfig?.enabled !== false to === true so handlers are not registered when summarizationConfig is undefined. chore: fix initializeClient JSDoc and move DEFAULT_RESERVE_RATIO to module scope refactor(Summary): align collapse/expand behavior with Reasoning component - Single render path instead of separate streaming vs completed branches - Use useMessageContext for isSubmitting/isLatestMessage awareness so the "Summarizing..." label only shows during active streaming - Default to collapsed (matching Reasoning), user toggles to expand - Add proper aria attributes (aria-hidden, role, aria-controls, contentId) - Hide copy button while actively streaming feat(summarization): default to self-summarize using agent's own provider/model When no summarization config is provided (neither in librechat.yaml nor on the agent), automatically enable summarization using the agent's own provider and model. The agents package already provides default prompts, so no prompt configuration is needed. Also removes the dead resolveSummarizationLLMConfig in summarize.ts (and its spec) — run.ts buildAgentContext is the single source of truth for summarization config resolution. Removes the duplicate RuntimeSummarizationConfig local type in favor of the canonical SummarizationConfig from data-provider. chore: schema and type cleanup for summarization - Add trigger field to summarizationAgentOverrideSchema so per-agent trigger overrides in librechat.yaml are not silently stripped by Zod - Remove unused SummarizationStatus type from runs.ts - Make AppSummarizationConfig.enabled non-optional to reflect the invariant that loadSummarizationConfig always sets it refactor(responses): extract duplicated on_agent_log handler refactor(run): use agents package types for summarization config Import SummarizationConfig, ContextPruningConfig, and OverflowRecoveryConfig from @librechat/agents and use them to type-check the translation layer in buildAgentContext. This ensures the config object passed to the agent graph matches what it expects. - Use `satisfies AgentSummarizationConfig` on the config object - Cast contextPruningConfig and overflowRecoveryConfig to agents types - Properly narrow trigger fields from DeepPartial to required shape feat(config): add maxToolResultChars to base endpoint schema Add maxToolResultChars to baseEndpointSchema so it can be configured on any endpoint in librechat.yaml. Resolved during agent initialization using getProviderConfig's endpoint resolution: custom endpoint config takes precedence, then the provider-specific endpoint config, then the shared `all` config. Passed through to the agents package ToolNode, which uses it to cap tool result length before it enters the context window. When not configured, the agents package computes a sensible default from maxContextTokens. fix(summarization): forward agent model_parameters in self-summarize default When no explicit summarization config exists, the self-summarize default now forwards the agent's model_parameters as the summarization parameters. This ensures provider-specific settings (e.g. Bedrock region, credentials, endpoint host) are available when the agents package constructs the summarization LLM. fix(agents): register summarization handlers by default Change the enabled gate from === true to !== false so handlers register when no explicit summarization config exists. This aligns with the self-summarize default where summarization is always on unless explicitly disabled via enabled: false. refactor(summarization): let agents package inherit clientOptions for self-summarize Remove model_parameters forwarding from the self-summarize default. The agents package now reuses the agent's own clientOptions when the summarization provider matches the agent's provider, inheriting all provider-specific settings (region, credentials, proxy, etc.) automatically. refactor(summarization): use MessageContentComplex[] for summary content Unify summary content to always use MessageContentComplex[] arrays, matching the pattern used by on_message_delta. No more string | array unions — content is always an array of typed blocks ({ type: 'text', text: '...' } for text, { type: 'reasoning_content', ... } for reasoning). Agents package: - SummaryContentBlock.content: MessageContentComplex[] (was string) - tokenCount now optional (not sent on deltas) - Removed reasoning field — reasoning is now a content block type - streamAndCollect normalizes all chunks to content block arrays - Delta events pass content blocks directly LibreChat: - SummaryContentPart.content: Agents.MessageContentComplex[] - Updated Part.tsx, Summary.tsx, useStepHandler.ts, BaseClient.js - Summary.tsx derives display text from content blocks via useMemo - Aggregator uses simple array spread refactor(summarization): enhance summary handling and text extraction - Updated BaseClient.js to improve summary text extraction, accommodating both legacy and new content formats. - Modified summarization logic to ensure consistent handling of summary content across different message formats. - Adjusted test cases in summarization.e2e.spec.js to utilize the new summary text extraction method. - Refined SSE useStepHandler to initialize summary content as an array. - Updated configuration schema by removing unused minReserveTokens field. - Cleaned up SummaryContentPart type by removing rangeHash property. These changes streamline the summarization process and ensure compatibility with various content structures. refactor(summarization): streamline usage tracking and logging - Removed direct checks for summarization nodes in ModelEndHandler and replaced them with a dedicated markSummarizationUsage function for better readability and maintainability. - Updated OpenAIChatCompletionController and responses handlers to utilize the new markSummarizationUsage function for setting usage types. - Enhanced logging functionality by ensuring the logger correctly handles different log levels. - Introduced a new useCopyToClipboard hook in the Summary component to encapsulate clipboard copy logic, improving code reusability and clarity. These changes improve the overall structure and efficiency of the summarization handling and logging processes. refactor(summarization): update summary content block documentation - Removed outdated comment regarding the last summary content block in BaseClient.js. - Added a new comment to clarify the purpose of the findSummaryContentBlock method, ensuring consistency in documentation. These changes enhance code clarity and maintainability by providing accurate descriptions of the summarization logic. refactor(summarization): update summary content structure in tests - Modified the summarization content structure in e2e tests to use an array format for text, aligning with recent changes in summary handling. - Updated test descriptions to clarify the behavior of context token calculations, ensuring consistency and clarity in the tests. These changes enhance the accuracy and maintainability of the summarization tests by reflecting the updated content structure. refactor(summarization): remove legacy E2E test setup and configuration - Deleted the e2e-setup.js and jest.e2e.config.js files, which contained legacy configurations for E2E tests using real API keys. - Introduced a new summarization.e2e.ts file that implements comprehensive E2E backend integration tests for the summarization process, utilizing real AI providers and tracking summaries throughout the run. These changes streamline the testing framework by consolidating E2E tests into a single, more robust file while removing outdated configurations. refactor(summarization): enhance E2E tests and error handling - Added a cleanup step to force exit after all tests to manage Redis connections. - Updated the summarization model to 'claude-haiku-4-5-20251001' for consistency across tests. - Improved error handling in the processStream function to capture and return processing errors. - Enhanced logging for cross-run tests and tight context scenarios to provide better insights into test execution. These changes improve the reliability and clarity of the E2E tests for the summarization process. refactor(summarization): enhance test coverage for maxContextTokens behavior - Updated run-summarization.test.ts to include a new test case ensuring that maxContextTokens does not exceed user-defined limits, even when calculated ratios suggest otherwise. - Modified summarization.e2e.ts to replace legacy UsageMetadata type with a more appropriate type for collectedUsage, improving type safety and clarity in the test setup. These changes improve the robustness of the summarization tests by validating context token constraints and refining type definitions. feat(summarization): add comprehensive E2E tests for summarization process - Introduced a new summarization.e2e.test.ts file that implements extensive end-to-end integration tests for the summarization pipeline, covering the full flow from LibreChat to agents. - The tests utilize real AI providers and include functionality to track summaries during and between runs. - Added necessary cleanup steps to manage Redis connections post-tests and ensure proper exit. These changes enhance the testing framework by providing robust coverage for the summarization process, ensuring reliability and performance under real-world conditions. fix(service): import logger from winston configuration - Removed the import statement for logger from '@librechat/data-schemas' and replaced it with an import from '~/config/winston'. - This change ensures that the logger is correctly sourced from the updated configuration, improving consistency in logging practices across the application. refactor(summary): simplify Summary component and enhance token display - Removed the unused `meta` prop from the `SummaryButton` component to streamline its interface. - Updated the token display logic to use a localized string for better internationalization support. - Adjusted the rendering of the `meta` information to improve its visibility within the `Summary` component. These changes enhance the clarity and usability of the Summary component while ensuring better localization practices. feat(summarization): add maxInputTokens configuration for summarization - Introduced a new `maxInputTokens` property in the summarization configuration schema to control the amount of conversation context sent to the summarizer, with a default value of 10000. - Updated the `createRun` function to utilize the new `maxInputTokens` setting, allowing for more flexible summarization based on agent context. These changes enhance the summarization capabilities by providing better control over input token limits, improving the overall summarization process. refactor(summarization): simplify maxInputTokens logic in createRun function - Updated the logic for the `maxInputTokens` property in the `createRun` function to directly use the agent's base context tokens when the resolved summarization configuration does not specify a value. - This change streamlines the configuration process and enhances clarity in how input token limits are determined for summarization. These modifications improve the maintainability of the summarization configuration by reducing complexity in the token calculation logic. feat(summary): enhance Summary component to display meta information - Updated the SummaryContent component to accept an optional `meta` prop, allowing for additional contextual information to be displayed above the main content. - Adjusted the rendering logic in the Summary component to utilize the new `meta` prop, improving the visibility of supplementary details. These changes enhance the user experience by providing more context within the Summary component, making it clearer and more informative. refactor(summarization): standardize reserveRatio configuration in summarization logic - Replaced instances of `reserveTokensRatio` with `reserveRatio` in the `createRun` function and related tests to unify the terminology across the codebase. - Updated the summarization configuration schema to reflect this change, ensuring consistency in how the reserve ratio is defined and utilized. - Removed the per-agent override logic for summarization configuration, simplifying the overall structure and enhancing clarity. These modifications improve the maintainability and readability of the summarization logic by standardizing the configuration parameters. * fix: circular dependency of `~/models` * chore: update logging scope in agent log handlers Changed log scope from `[agentus:${data.scope}]` to `[agents:${data.scope}]` in both the callbacks and responses controllers to ensure consistent logging format across the application. * feat: calibration ratio * refactor(tests): update summarizationConfig tests to reflect changes in enabled property Modified tests to check for the new `summarizationEnabled` property instead of the deprecated `enabled` field in the summarization configuration. This change ensures that the tests accurately validate the current configuration structure and behavior of the agents. * feat(tests): add markSummarizationUsage mock for improved test coverage Introduced a mock for the markSummarizationUsage function in the responses unit tests to enhance the testing of summarization usage tracking. This addition supports better validation of summarization-related functionalities and ensures comprehensive test coverage for the agents' response handling. * refactor(tests): simplify event handler setup in createResponse tests Removed redundant mock implementations for event handlers in the createResponse unit tests, streamlining the setup process. This change enhances test clarity and maintainability while ensuring that the tests continue to validate the correct behavior of usage tracking during on_chat_model_end events. * refactor(agents): move calibration ratio capture to finally block Reorganized the logic for capturing the calibration ratio in the AgentClient class to ensure it is executed in the finally block. This change guarantees that the ratio is captured even if the run is aborted, enhancing the reliability of the response message persistence. Removed redundant code and improved clarity in the handling of context metadata. * refactor(agents): streamline bulk write logic in recordCollectedUsage function Removed redundant bulk write operations and consolidated document handling in the recordCollectedUsage function. The logic now combines all documents into a single bulk write operation, improving efficiency and reducing error handling complexity. Updated logging to provide consistent error messages for bulk write failures. * refactor(agents): enhance summarization configuration resolution in createRun function Streamlined the summarization configuration logic by introducing a base configuration and allowing for overrides from agent-specific settings. This change improves clarity and maintainability, ensuring that the summarization configuration is consistently applied while retaining flexibility for customization. Updated the handling of summarization parameters to ensure proper integration with the agent's model and provider settings. * refactor(agents): remove unused tokenCountMap and streamline calibration ratio handling Eliminated the unused tokenCountMap variable from the AgentClient class to enhance code clarity. Additionally, streamlined the logic for capturing the calibration ratio by using optional chaining and a fallback value, ensuring that context metadata is consistently defined. This change improves maintainability and reduces potential confusion in the codebase. * refactor(agents): extract agent log handler for improved clarity and reusability Refactored the agent log handling logic by extracting it into a dedicated function, `agentLogHandler`, enhancing code clarity and reusability across different modules. Updated the event handlers in both the OpenAI and responses controllers to utilize the new handler, ensuring consistent logging behavior throughout the application. * test: add summarization event tests for useStepHandler Implemented a series of tests for the summarization events in the useStepHandler hook. The tests cover scenarios for ON_SUMMARIZE_START, ON_SUMMARIZE_DELTA, and ON_SUMMARIZE_COMPLETE events, ensuring proper handling of summarization logic, including message accumulation and finalization. This addition enhances test coverage and validates the correct behavior of the summarization process within the application. * refactor(config): update summarizationTriggerSchema to use enum for type validation Changed the type of the `type` field in the summarizationTriggerSchema from a string to an enum with a single value 'token_count'. This modification enhances type safety and ensures that only valid types are accepted in the configuration, improving overall clarity and maintainability of the schema. * test(usage): add bulk write tests for message and summarization usage Implemented tests for the bulk write functionality in the recordCollectedUsage function, covering scenarios for combined message and summarization usage, summarization-only usage, and message-only usage. These tests ensure correct document handling and token rollup calculations, enhancing test coverage and validating the behavior of the usage tracking logic. * refactor(Chat): enhance clipboard copy functionality and type definitions in Summary component Updated the Summary component to improve the clipboard copy functionality by handling clipboard permission errors. Refactored type definitions for SummaryProps to use a more specific type, enhancing type safety. Adjusted the SummaryButton and FloatingSummaryBar components to accept isCopied and onCopy props, promoting better separation of concerns and reusability. * chore(translations): remove unused "Expand Summary" key from English translations Deleted the "Expand Summary" key from the English translation file to streamline the localization resources and improve clarity in the user interface. This change helps maintain an organized and efficient translation structure. * refactor: adjust token counting for Claude model to account for API discrepancies Implemented a correction factor for token counting when using the Claude model, addressing discrepancies between Anthropic's API and local tokenizer results. This change ensures accurate token counts by applying a scaling factor, improving the reliability of token-related functionalities. * refactor(agents): implement token count adjustment for Claude model messages Added a method to adjust token counts for messages processed by the Claude model, applying a correction factor to align with API expectations. This enhancement improves the accuracy of token counting, ensuring reliable functionality when interacting with the Claude model. * refactor(agents): token counting for media content in messages Introduced a new method to estimate token costs for image and document blocks in messages, improving the accuracy of token counting. This enhancement ensures that media content is properly accounted for, particularly for the Claude model, by integrating additional token estimation logic for various content types. Updated the token counting function to utilize this new method, enhancing overall reliability and functionality. * chore: fix missing import * fix(agents): clamp baseContextTokens and document reserve ratio change Prevent negative baseContextTokens when maxOutputTokens exceeds the context window (misconfigured models). Document the 10%→5% default reserve ratio reduction introduced alongside summarization. * fix(agents): include media tokens in hydrated token counts Add estimateMediaTokensForMessage to createTokenCounter so the hydration path (used by hydrateMissingIndexTokenCounts) matches the precomputed path in AgentClient.getTokenCountForMessage. Without this, messages containing images or documents were systematically undercounted during hydration, risking context window overflow. Add 34 unit tests covering all block-type branches of estimateMediaTokensForMessage. * fix(agents): include summarization output tokens in usage return value The returned output_tokens from recordCollectedUsage now reflects all billed LLM calls (message + summarization). Previously, summarization completions were billed but excluded from the returned metadata, causing a discrepancy between what users were charged and what the response message reported. * fix(tests): replace process.exit with proper Redis cleanup in e2e test The summarization E2E test used process.exit(0) to work around a Redis connection opened at import time, which killed the Jest runner and bypassed teardown. Use ioredisClient.quit() and keyvRedisClient.disconnect() for graceful cleanup instead. * fix(tests): update getConvo imports in OpenAI and response tests Refactor test files to import getConvo from the main models module instead of the Conversation submodule. This change ensures consistency across tests and simplifies the import structure, enhancing maintainability. * fix(clients): improve summary text validation in BaseClient Refactor the summary extraction logic to ensure that only non-empty summary texts are considered valid. This change enhances the robustness of the message processing by utilizing a dedicated method for summary text retrieval, improving overall reliability. * fix(config): replace z.any() with explicit union in summarization schema Model parameters (temperature, top_p, etc.) are constrained to primitive types rather than the policy-violating z.any(). * refactor(agents): deduplicate CLAUDE_TOKEN_CORRECTION constant Export from the TS source in packages/api and import in the JS client, eliminating the static class property that could drift out of sync. * refactor(agents): eliminate duplicate selfProvider in buildAgentContext selfProvider and provider were derived from the same expression with different type casts. Consolidated to a single provider variable. * refactor(agents): extract shared SSE handlers and restrict log levels - buildSummarizationHandlers() factory replaces triplicated handler blocks across responses.js and openai.js - agentLogHandlerObj exported from callbacks.js for consistent reuse - agentLogHandler restricted to an allowlist of safe log levels (debug, info, warn, error) instead of accepting arbitrary strings * fix(SSE): batch summarize deltas, add exhaustiveness check, conditional error announcement - ON_SUMMARIZE_DELTA coalesces rapid-fire renders via requestAnimationFrame instead of calling setMessages per chunk - Exhaustive never-check on TStepEvent catches unhandled variants at compile time when new StepEvents are added - ON_SUMMARIZE_COMPLETE error announcement only fires when a summary part was actually present and removed * feat(agents): persist instruction overhead in contextMeta and seed across runs Extend contextMeta with instructionOverhead and toolCount so the provider-observed instruction overhead is persisted on the response message and seeded into the pruner on subsequent runs. This enables the pruner to use a calibrated budget from the first call instead of waiting for a provider observation, preventing the ratio collapse caused by local tokenizer overestimating tool schema tokens. The seeded overhead is only used when encoding and tool count match between runs, ensuring stale values from different configurations are discarded. * test(agents): enhance OpenAI test mocks for summarization handlers Updated the OpenAI test suite to include additional mock implementations for summarization handlers, including buildSummarizationHandlers, markSummarizationUsage, and agentLogHandlerObj. This improves test coverage and ensures consistent behavior during testing. * fix(agents): address review findings for summarization v2 Cancel rAF on unmount to prevent stale Recoil writes from dead component context. Clear orphaned summarizing:true parts when ON_SUMMARIZE_COMPLETE arrives without a summary payload. Add null guard and safe spread to agentLogHandler. Handle Anthropic-format base64 image/* documents in estimateMediaTokensForMessage. Use role="region" for expandable summary content. Add .describe() to contextMeta Zod fields. Extract duplicate usage loop into helper. * refactor: simplify contextMeta to calibrationRatio + encoding only Remove instructionOverhead and toolCount from cross-run persistence — instruction tokens change too frequently between runs (prompt edits, tool changes) for a persisted seed to be reliable. The intra-run calibration in the pruner still self-corrects via provider observations. contextMeta now stores only the tokenizer-bias ratio and encoding, which are stable across instruction changes. * test(SSE): enhance useStepHandler tests for ON_SUMMARIZE_COMPLETE behavior Updated the test for ON_SUMMARIZE_COMPLETE to clarify that it finalizes the existing part with summarizing set to false when the summary is undefined. Added assertions to verify the correct behavior of message updates and the state of summary parts. * refactor(BaseClient): remove handleContextStrategy and truncateToolCallOutputs functions Eliminated the handleContextStrategy method from BaseClient to streamline message handling. Also removed the truncateToolCallOutputs function from the prompts module, simplifying the codebase and improving maintainability. * refactor: add AGENT_DEBUG_LOGGING option and refactor token count handling in BaseClient Introduced AGENT_DEBUG_LOGGING to .env.example for enhanced debugging capabilities. Refactored token count handling in BaseClient by removing the handleTokenCountMap method and simplifying token count updates. Updated AgentClient to log detailed token count recalculations and adjustments, improving traceability during message processing. * chore: update dependencies in package-lock.json and package.json files Bumped versions of several dependencies, including @librechat/agents to ^3.1.62 and various AWS SDK packages to their latest versions. This ensures compatibility and incorporates the latest features and fixes. * chore: imports order * refactor: extract summarization config resolution from buildAgentContext * refactor: rename and simplify summarization configuration shaping function * refactor: replace AgentClient token counting methods with single-pass pure utility Extract getTokenCount() and getTokenCountForMessage() from AgentClient into countFormattedMessageTokens(), a pure function in packages/api that handles text, tool_call, image, and document content types in one loop. - Decompose estimateMediaTokensForMessage into block-level helpers (estimateImageDataTokens, estimateImageBlockTokens, estimateDocumentBlockTokens) shared by both estimateMediaTokensForMessage and the new single-pass function - Remove redundant per-call getEncoding() resolution (closure captures once) - Remove deprecated gpt-3.5-turbo-0301 model branching - Drop this.getTokenCount guard from BaseClient.sendMessage * refactor: streamline token counting in createTokenCounter function Simplified the createTokenCounter function by removing the media token estimation and directly calculating the token count. This change enhances clarity and performance by consolidating the token counting logic into a single pass, while maintaining compatibility with Claude's token correction. * refactor: simplify summarization configuration types Removed the AppSummarizationConfig type and directly used SummarizationConfig in the AppConfig interface. This change streamlines the type definitions and enhances consistency across the codebase. * chore: import order * fix: summarization event handling in useStepHandler - Cancel pending summarizeDeltaRaf in clearStepMaps to prevent stale frames firing after map reset or component unmount - Move announcePolite('summarize_completed') inside the didFinalize guard so screen readers only announce when finalization actually occurs - Remove dead cleanup closure returned from stepHandler useCallback body that was never invoked by any caller * fix: estimate tokens for non-PDF/non-image base64 document blocks Previously estimateDocumentBlockTokens returned 0 for unrecognized MIME types (e.g. text/plain, application/json), silently underestimating context budget. Fall back to character-based heuristic or countTokens. * refactor: return cloned usage from markSummarizationUsage Avoid mutating LangChain's internal usage_metadata object by returning a shallow clone with the usage_type tag. Update all call sites in callbacks, openai, and responses controllers to use the returned value. * refactor: consolidate debug logging loops in buildMessages Merge the two sequential O(n) debug-logging passes over orderedMessages into a single pass inside the map callback where all data is available. * refactor: narrow SummaryContentPart.content type Replace broad Agents.MessageContentComplex[] with the specific Array<{ type: ContentTypes.TEXT; text: string }> that all producers and consumers already use, improving compile-time safety. * refactor: use single output array in recordCollectedUsage Have processUsageGroup append to a shared array instead of returning separate arrays that are spread into a third, reducing allocations. * refactor: use for...in in hydrateMissingIndexTokenCounts Replace Object.entries with for...in to avoid allocating an intermediate tuple array during token map hydration.
2026-03-21 12:03:10 -04:00
const { getActions } = require('~/models');
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
let mockDomainCache = {};
jest.mock('~/cache/getLogStores', () => {
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
return jest.fn().mockImplementation(() => ({
get: async (key) => mockDomainCache[key] ?? null,
set: async (key, value) => {
mockDomainCache[key] = value;
return true;
},
}));
});
beforeEach(() => {
mockDomainCache = {};
getActions.mockReset();
});
const SEP = actionDomainSeparator;
const DELIM = actionDelimiter;
const MAX = Constants.ENCODED_DOMAIN_LENGTH;
const domainSepRegex = new RegExp(SEP, 'g');
describe('domainParser', () => {
describe('nullish input', () => {
it.each([null, undefined, ''])('returns undefined for %j', async (input) => {
expect(await domainParser(input, true)).toBeUndefined();
expect(await domainParser(input, false)).toBeUndefined();
});
});
describe('short-path encoding (hostname ≤ threshold)', () => {
it.each([
['examp.com', `examp${SEP}com`],
['swapi.tech', `swapi${SEP}tech`],
['a.b', `a${SEP}b`],
])('replaces dots in %s → %s', async (domain, expected) => {
expect(await domainParser(domain, true)).toBe(expected);
});
it('handles domain exactly at threshold length', async () => {
const domain = 'a'.repeat(MAX - 4) + '.com';
expect(domain).toHaveLength(MAX);
const result = await domainParser(domain, true);
expect(result).toBe(domain.replace(/\./g, SEP));
});
});
describe('base64-path encoding (hostname > threshold)', () => {
it('produces a key of exactly ENCODED_DOMAIN_LENGTH chars', async () => {
const result = await domainParser('api.example.com', true);
expect(result).toHaveLength(MAX);
});
it('encodes hostname, not full URL', async () => {
const hostname = 'api.example.com';
const expectedKey = Buffer.from(hostname).toString('base64').substring(0, MAX);
expect(await domainParser(hostname, true)).toBe(expectedKey);
});
it('populates decode cache for round-trip', async () => {
const hostname = 'longdomainname.com';
const key = await domainParser(hostname, true);
expect(mockDomainCache[key]).toBe(Buffer.from(hostname).toString('base64'));
expect(await domainParser(key, false)).toBe(hostname);
});
});
describe('protocol stripping', () => {
it('https:// URL and bare hostname produce identical encoding', async () => {
const encoded = await domainParser('https://swapi.tech', true);
expect(encoded).toBe(await domainParser('swapi.tech', true));
expect(encoded).toBe(`swapi${SEP}tech`);
});
it('http:// URL and bare hostname produce identical encoding', async () => {
const encoded = await domainParser('http://api.example.com', true);
expect(encoded).toBe(await domainParser('api.example.com', true));
});
it('different https:// domains produce unique keys', async () => {
const keys = await Promise.all([
domainParser('https://api.example.com', true),
domainParser('https://api.weather.com', true),
domainParser('https://data.github.com', true),
]);
const unique = new Set(keys);
expect(unique.size).toBe(keys.length);
});
it('long hostname after stripping still uses base64 path', async () => {
const result = await domainParser('https://api.example.com', true);
expect(result).toHaveLength(MAX);
expect(result).not.toContain(SEP);
});
it('short hostname after stripping uses dot-replacement path', async () => {
const result = await domainParser('https://a.b.c', true);
expect(result).toBe(`a${SEP}b${SEP}c`);
});
it('strips path and query from full URL before encoding', async () => {
const result = await domainParser('https://api.example.com/v1/endpoint?foo=bar', true);
expect(result).toBe(await domainParser('api.example.com', true));
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('unicode domains', () => {
it('encodes unicode hostname via base64 path', async () => {
const domain = 'täst.example.com';
const result = await domainParser(domain, true);
expect(result).toHaveLength(MAX);
expect(result).toBe(Buffer.from(domain).toString('base64').substring(0, MAX));
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('round-trips unicode hostname through encode then decode', async () => {
const domain = 'täst.example.com';
const key = await domainParser(domain, true);
expect(await domainParser(key, false)).toBe(domain);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('strips protocol before encoding unicode hostname', async () => {
const withProto = 'https://täst.example.com';
const bare = 'täst.example.com';
expect(await domainParser(withProto, true)).toBe(await domainParser(bare, true));
});
});
describe('decode path', () => {
it('short-path encoded domain decodes via separator replacement', async () => {
expect(await domainParser(`examp${SEP}com`, false)).toBe('examp.com');
});
it('base64-path encoded domain decodes via cache lookup', async () => {
const hostname = 'api.example.com';
const key = await domainParser(hostname, true);
expect(await domainParser(key, false)).toBe(hostname);
});
it('returns input unchanged for unknown non-separator strings', async () => {
expect(await domainParser('not_base64_encoded', false)).toBe('not_base64_encoded');
});
it('returns a string without throwing for corrupt cache entries', async () => {
mockDomainCache['corrupt_key'] = '!!!';
const result = await domainParser('corrupt_key', false);
expect(typeof result).toBe('string');
});
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('legacyDomainEncode', () => {
it.each(['', null, undefined])('returns empty string for %j', (input) => {
expect(legacyDomainEncode(input)).toBe('');
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('is synchronous (returns a string, not a Promise)', () => {
const result = legacyDomainEncode('examp.com');
expect(result).toBe(`examp${SEP}com`);
expect(result).not.toBeInstanceOf(Promise);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('uses dot-replacement for short domains', () => {
expect(legacyDomainEncode('examp.com')).toBe(`examp${SEP}com`);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('uses base64 prefix of full input for long domains', () => {
const domain = 'https://swapi.tech';
const expected = Buffer.from(domain).toString('base64').substring(0, MAX);
expect(legacyDomainEncode(domain)).toBe(expected);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('all https:// URLs collide to the same key', () => {
const results = [
legacyDomainEncode('https://api.example.com'),
legacyDomainEncode('https://api.weather.com'),
legacyDomainEncode('https://totally.different.host'),
];
expect(new Set(results).size).toBe(1);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('matches what old domainParser would have produced', () => {
const domain = 'https://api.example.com';
const legacy = legacyDomainEncode(domain);
expect(legacy).toBe(Buffer.from(domain).toString('base64').substring(0, MAX));
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('produces same result as new domainParser for short bare hostnames', async () => {
const domain = 'swapi.tech';
expect(legacyDomainEncode(domain)).toBe(await domainParser(domain, true));
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
});
describe('validateAndUpdateTool', () => {
const mockReq = { user: { id: 'user123' } };
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('returns tool unchanged when name passes tool-name regex', async () => {
const tool = { function: { name: 'getPeople_action_swapi---tech' } };
const result = await validateAndUpdateTool({
req: mockReq,
tool,
assistant_id: 'asst_1',
});
expect(result).toEqual(tool);
expect(getActions).not.toHaveBeenCalled();
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('matches action when metadata.domain has https:// prefix and tool domain is bare hostname', async () => {
getActions.mockResolvedValue([{ metadata: { domain: 'https://api.example.com' } }]);
const tool = { function: { name: `getPeople${DELIM}api.example.com` } };
const result = await validateAndUpdateTool({
req: mockReq,
tool,
assistant_id: 'asst_1',
});
expect(result).not.toBeNull();
expect(result.function.name).toMatch(/^getPeople_action_/);
expect(result.function.name).not.toContain('.');
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('matches action when metadata.domain has no protocol', async () => {
getActions.mockResolvedValue([{ metadata: { domain: 'api.example.com' } }]);
const tool = { function: { name: `getPeople${DELIM}api.example.com` } };
const result = await validateAndUpdateTool({
req: mockReq,
tool,
assistant_id: 'asst_1',
});
expect(result).not.toBeNull();
expect(result.function.name).toMatch(/^getPeople_action_/);
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('returns null when no action matches the domain', async () => {
getActions.mockResolvedValue([{ metadata: { domain: 'https://other.domain.com' } }]);
const tool = { function: { name: `getPeople${DELIM}api.example.com` } };
const result = await validateAndUpdateTool({
req: mockReq,
tool,
assistant_id: 'asst_1',
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
expect(result).toBeNull();
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
it('returns null when action has no metadata', async () => {
getActions.mockResolvedValue([{ metadata: null }]);
const tool = { function: { name: `getPeople${DELIM}api.example.com` } };
const result = await validateAndUpdateTool({
req: mockReq,
tool,
assistant_id: 'asst_1',
});
expect(result).toBeNull();
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
});
describe('backward-compatible tool name matching', () => {
function normalizeToolName(name) {
return name.replace(domainSepRegex, '_');
}
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
function buildToolName(functionName, encodedDomain) {
return `${functionName}${DELIM}${encodedDomain}`;
}
describe('definition-phase matching', () => {
it('new encoding matches agent tools stored with new encoding', async () => {
const metadataDomain = 'https://swapi.tech';
const encoded = await domainParser(metadataDomain, true);
const normalized = normalizeToolName(encoded);
const storedTool = buildToolName('getPeople', encoded);
const defToolName = `getPeople${DELIM}${normalized}`;
expect(normalizeToolName(storedTool)).toBe(defToolName);
});
it('legacy encoding matches agent tools stored with legacy encoding', async () => {
const metadataDomain = 'https://swapi.tech';
const legacy = legacyDomainEncode(metadataDomain);
const legacyNormalized = normalizeToolName(legacy);
const storedTool = buildToolName('getPeople', legacy);
const legacyDefName = `getPeople${DELIM}${legacyNormalized}`;
expect(normalizeToolName(storedTool)).toBe(legacyDefName);
});
it('new definition matches old stored tools via legacy fallback', async () => {
const metadataDomain = 'https://swapi.tech';
const newDomain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const newNorm = normalizeToolName(newDomain);
const legacyNorm = normalizeToolName(legacyDomain);
const oldStoredTool = buildToolName('getPeople', legacyDomain);
const newToolName = `getPeople${DELIM}${newNorm}`;
const legacyToolName = `getPeople${DELIM}${legacyNorm}`;
const storedNormalized = normalizeToolName(oldStoredTool);
const hasMatch = storedNormalized === newToolName || storedNormalized === legacyToolName;
expect(hasMatch).toBe(true);
});
it('pre-normalized Set eliminates per-tool normalization', async () => {
const metadataDomain = 'https://api.example.com';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const normalizedDomain = normalizeToolName(domain);
const legacyNormalized = normalizeToolName(legacyDomain);
const storedTools = [
buildToolName('getWeather', legacyDomain),
buildToolName('getForecast', domain),
];
const preNormalized = new Set(storedTools.map((t) => normalizeToolName(t)));
const toolName = `getWeather${DELIM}${normalizedDomain}`;
const legacyToolName = `getWeather${DELIM}${legacyNormalized}`;
expect(preNormalized.has(toolName) || preNormalized.has(legacyToolName)).toBe(true);
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('execution-phase tool lookup', () => {
it('model-called tool name resolves via normalizedToDomain map (new encoding)', async () => {
const metadataDomain = 'https://api.example.com';
const domain = await domainParser(metadataDomain, true);
const normalized = normalizeToolName(domain);
const normalizedToDomain = new Map();
normalizedToDomain.set(normalized, domain);
const modelToolName = `getWeather${DELIM}${normalized}`;
let matched = '';
for (const [norm, canonical] of normalizedToDomain.entries()) {
if (modelToolName.includes(norm)) {
matched = canonical;
break;
}
}
expect(matched).toBe(domain);
const functionName = modelToolName.replace(`${DELIM}${normalizeToolName(matched)}`, '');
expect(functionName).toBe('getWeather');
});
it('model-called tool name resolves via legacy entry in normalizedToDomain map', async () => {
const metadataDomain = 'https://api.example.com';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const legacyNorm = normalizeToolName(legacyDomain);
const normalizedToDomain = new Map();
normalizedToDomain.set(normalizeToolName(domain), domain);
normalizedToDomain.set(legacyNorm, domain);
const legacyModelToolName = `getWeather${DELIM}${legacyNorm}`;
let matched = '';
for (const [norm, canonical] of normalizedToDomain.entries()) {
if (legacyModelToolName.includes(norm)) {
matched = canonical;
break;
}
}
expect(matched).toBe(domain);
});
it('legacy guard skips duplicate map entry for short bare hostnames', async () => {
const domain = 'swapi.tech';
const newEncoding = await domainParser(domain, true);
const legacyEncoding = legacyDomainEncode(domain);
expect(newEncoding).toBe(legacyEncoding);
const normalizedToDomain = new Map();
normalizedToDomain.set(newEncoding, newEncoding);
if (legacyEncoding !== newEncoding) {
normalizedToDomain.set(legacyEncoding, newEncoding);
}
expect(normalizedToDomain.size).toBe(1);
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('processRequiredActions matching (assistants path)', () => {
it('legacy tool from OpenAI matches via normalizedToDomain with both encodings', async () => {
const metadataDomain = 'https://swapi.tech';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const normalizedToDomain = new Map();
normalizedToDomain.set(domain, domain);
if (legacyDomain !== domain) {
normalizedToDomain.set(legacyDomain, domain);
}
const legacyToolName = buildToolName('getPeople', legacyDomain);
let currentDomain = '';
let matchedKey = '';
for (const [key, canonical] of normalizedToDomain.entries()) {
if (legacyToolName.includes(key)) {
currentDomain = canonical;
matchedKey = key;
break;
}
}
expect(currentDomain).toBe(domain);
expect(matchedKey).toBe(legacyDomain);
const functionName = legacyToolName.replace(`${DELIM}${matchedKey}`, '');
expect(functionName).toBe('getPeople');
});
it('new tool name matches via the canonical domain key', async () => {
const metadataDomain = 'https://swapi.tech';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const normalizedToDomain = new Map();
normalizedToDomain.set(domain, domain);
if (legacyDomain !== domain) {
normalizedToDomain.set(legacyDomain, domain);
}
const newToolName = buildToolName('getPeople', domain);
let currentDomain = '';
let matchedKey = '';
for (const [key, canonical] of normalizedToDomain.entries()) {
if (newToolName.includes(key)) {
currentDomain = canonical;
matchedKey = key;
break;
}
}
expect(currentDomain).toBe(domain);
expect(matchedKey).toBe(domain);
const functionName = newToolName.replace(`${DELIM}${matchedKey}`, '');
expect(functionName).toBe('getPeople');
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('save-route cleanup', () => {
it('tool filter removes tools matching new encoding', async () => {
const metadataDomain = 'https://swapi.tech';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const tools = [
buildToolName('getPeople', domain),
buildToolName('unrelated', 'other---domain'),
];
const filtered = tools.filter((t) => !t.includes(domain) && !t.includes(legacyDomain));
expect(filtered).toEqual([buildToolName('unrelated', 'other---domain')]);
});
it('tool filter removes tools matching legacy encoding', async () => {
const metadataDomain = 'https://swapi.tech';
const domain = await domainParser(metadataDomain, true);
const legacyDomain = legacyDomainEncode(metadataDomain);
const tools = [
buildToolName('getPeople', legacyDomain),
buildToolName('unrelated', 'other---domain'),
];
const filtered = tools.filter((t) => !t.includes(domain) && !t.includes(legacyDomain));
expect(filtered).toEqual([buildToolName('unrelated', 'other---domain')]);
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('delete-route domain extraction', () => {
it('domain extracted from actions array is usable as-is for tool filtering', async () => {
const metadataDomain = 'https://api.example.com';
const domain = await domainParser(metadataDomain, true);
const actionId = 'abc123';
const actionEntry = `${domain}${DELIM}${actionId}`;
const [storedDomain] = actionEntry.split(DELIM);
expect(storedDomain).toBe(domain);
const tools = [buildToolName('getWeather', domain), buildToolName('getPeople', 'other')];
const filtered = tools.filter((t) => !t.includes(storedDomain));
expect(filtered).toEqual([buildToolName('getPeople', 'other')]);
});
});
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271) * fix: strip protocol from domain before encoding in `domainParser` All https:// (and http://) domains produced the same 10-char base64 prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name collisions for agents with multiple actions. Strip the protocol before encoding so the base64 key is derived from the hostname. Add `legacyDomainEncode` to preserve the old encoding logic for backward-compatible matching of existing stored actions. * fix: backward-compatible tool matching in ToolService Update `getActionToolDefinitions` to match stored tools against both new and legacy domain encodings. Update `loadActionToolsForExecution` to resolve model-called tool names via a `normalizedToDomain` map that includes both encoding variants, with legacy fallback for request builder lookup. * fix: action route save/delete domain encoding issues Save routes now remove old tools matching either new or legacy domain encoding, preventing stale entries when an action's encoding changes on update. Delete routes no longer re-encode the already-encoded domain extracted from the stored actions array, which was producing incorrect keys and leaving orphaned tools. * test: comprehensive coverage for action domain encoding Rewrite ActionService tests to cover real matching patterns used by ToolService and action routes. Tests verify encode/decode round-trips, protocol stripping, backward-compatible tool name matching at both definition and execution phases, save-route cleanup of old/new encodings, delete-route domain extraction, and the collision fix for multi-action agents. * fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync CRITICAL: processRequiredActions (assistants path) was not updated with legacy domain matching — existing assistants with https:// domain actions would silently fail post-deployment because domainMap only had new encoding. MAJOR: loadAgentTools definitionsOnly=false path had the same issue. Both now use a normalizedToDomain map with legacy+new entries and extract function names via the matched key (not the canonical domain). Also: make legacyDomainEncode synchronous (no async operations), store legacyNormalized in processedActionSets to eliminate recomputation in the per-tool fallback, and hoist domainSeparatorRegex to module level. * refactor: clarify domain variable naming and tool-filter helpers in action routes Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from encoded key in both agent and assistant save routes. Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool to make the distinct data-shape guards explicit. Remove await on now-synchronous legacyDomainEncode. * test: expand coverage for all review findings - Add validateAndUpdateTool tests (protocol-stripping match logic) - Restore unicode domain encode/decode/round-trip tests - Add processRequiredActions matching pattern tests (assistants path) - Add legacy guard skip test for short bare hostnames - Add pre-normalized Set test for definition-phase optimization - Fix corrupt-cache test to assert typeof instead of toBeDefined - Verify legacyDomainEncode is synchronous (not a Promise) - Remove all await on legacyDomainEncode (now sync) 58 tests, up from 44. * fix: address follow-up review findings A-E A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous legacyDomainEncode — changed to @returns {string}. B: Rename normalizedToDomain to domainLookupMap in processRequiredActions and loadAgentTools where keys are raw encoded domains (not normalized), avoiding confusion with loadActionToolsForExecution where keys ARE normalized. C: Pre-normalize actionToolNames into a Set<string> in getActionToolDefinitions, replacing O(signatures × tools) per-check .some() + .replace() with O(1) Set.has() lookups. D: Remove stripProtocol from ActionService exports — it is a one-line internal helper. Spec tests for it removed; behavior is fully covered by domainParser protocol-stripping tests. E: Fix pre-existing bug where processRequiredActions re-loaded action sets on every missing-tool iteration. The guard !actionSets.length always re-triggered because actionSets was reassigned to a plain object (whose .length is undefined). Replaced with a null-check on a dedicated actionSetsData variable. * fix: strip path and query from domain URLs in stripProtocol URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously retained the path after protocol stripping, contaminating the encoded domain key. Now strips everything after the first '/' following the host, using string indexing instead of URL parsing to avoid punycode normalization of unicode hostnames. Closes Copilot review comments 1, 2, and 5.
2026-03-17 01:38:51 -04:00
describe('multi-action agents (collision scenario)', () => {
it('two https:// actions now produce distinct tool names', async () => {
const domain1 = await domainParser('https://api.weather.com', true);
const domain2 = await domainParser('https://api.spacex.com', true);
const tool1 = buildToolName('getData', domain1);
const tool2 = buildToolName('getData', domain2);
expect(tool1).not.toBe(tool2);
});
it('two https:// actions used to collide in legacy encoding', () => {
const legacy1 = legacyDomainEncode('https://api.weather.com');
const legacy2 = legacyDomainEncode('https://api.spacex.com');
const tool1 = buildToolName('getData', legacy1);
const tool2 = buildToolName('getData', legacy2);
expect(tool1).toBe(tool2);
});
});
});