fix: preserve transform_js and rate_limit on http_client tools

[buger]

Summary

• When http_client tools were extracted from ai_mcp_servers config, the CustomToolDefinition was constructed with only a hardcoded subset of fields
• transform_js and rate_limit were silently dropped, so the MCP server's transform_js application code never fired (tool.transform_js was always undefined)
• This caused disqualified candidates to appear in Workable API list responses despite the workable-api tool having a transform_js filter configured
• The AI then processed these disqualified candidates, and in some cases reverted and moved them back to active stages

Fix

Copy transform_js and rate_limit fields from entry.config when constructing the CustomToolDefinition for http_client tools (2 lines added in ai-check-provider.ts).

Test plan

• Verified transform_js is defined on CustomToolDefinition interface
• Built successfully
• Workable transform unit tests pass with --no-mocks
• Run screening test with --no-mocks and verify "Filtered N disqualified candidates" appears in logs

🤖 Generated with Claude Code

[probelabs]

PR Overview: Fix preserve transform_js and rate_limit on http_client tools

Summary

This PR fixes a critical bug where transform_js and rate_limit configurations were silently dropped when http_client tools were extracted from ai_mcp_servers config. The fix ensures these important fields are properly preserved when converting MCP server entries to CustomToolDefinition objects.

Problem Description

When http_client tools were configured in ai_mcp_servers (e.g., for REST API integrations like Workable), the extraction logic in AICheckProvider constructed CustomToolDefinition objects with only a hardcoded subset of fields:

• name, type, description, base_url, auth, headers, timeout, inputSchema

Missing fields:

• transform_js - JavaScript expressions to filter/transform API responses
• rate_limit - Rate limiting configuration for API requests

Real-World Impact

The bug caused disqualified candidates to appear in Workable API list responses despite having a transform_js filter configured. The AI then processed these disqualified candidates and in some cases reverted and moved them back to active stages - a serious data integrity issue.

Solution

Core Fix (2 lines added)

File: src/providers/ai-check-provider.ts:1510-1516

// Preserve transform_js and rate_limit from the original tool config
...(entry.config.transform_js
  ? { transform_js: entry.config.transform_js as string }
  : {}),
...(entry.config.rate_limit
  ? { rate_limit: entry.config.rate_limit as CustomToolDefinition['rate_limit'] }
  : {}),

This conditional spread syntax ensures both fields are copied from entry.config when present.

Additional Changes

1. Config Reload Runtime Refactoring

New file: src/runners/config-reload-runtime.ts (76 lines)

Extracts config reload logic from cli-main.ts into a dedicated module:

• Unified SIGUSR2 signal handling for hot-reload
• File watching support with ConfigWatcher
• Proper cleanup via cleanup() Promise
• Graceful handling when no config path is specified

Benefits:

• Better separation of concerns
• Reusable across different runner contexts
• Improved testability

2. ConfigWatcher Enhancement

File: src/config/config-watcher.ts

Added optional listenForSignals parameter to start() method:

• Prevents duplicate SIGUSR2 handlers when used with new runtime
• Maintains backward compatibility (defaults to true)

3. CLI Main Simplification

File: src/cli-main.ts

Replaced inline config watcher setup (36 lines deleted) with:

const { setupRunnerConfigReloadRuntime } = await import('./runners/config-reload-runtime');
const configReloadRuntime = await setupRunnerConfigReloadRuntime({...});

Cleanup now uses:

await configReloadRuntime.cleanup();

4. Comprehensive Test Coverage

New file: tests/unit/runners/config-reload-runtime.test.ts (130 lines)

Tests cover:

• SIGUSR2 reload handling without file watch mode
• File watching with duplicate signal handler prevention
• Graceful degradation when no config path provided
• Proper cleanup of resources (watcher, signal handlers, snapshot store)

Architecture & Impact

Component Relationships

graph TD
    A[ai_mcp_servers config] --> B[AICheckProvider.execute]
    B --> C[Extract http_client entries]
    C --> D[Build CustomToolDefinition]
    D --> E{transform_js present?}
    D --> F{rate_limit present?}
    E --> G[Copy transform_js field]
    F --> H[Copy rate_limit field]
    G --> I[Register in CustomToolsSSEServer]
    H --> I
    I --> J[AI calls tool via MCP]
    J --> K[mcp-custom-sse-server.ts]
    K --> L[executeHttpClientTool]
    L --> M[Apply transform_js filter]
    L --> N[Apply rate_limit]
    M --> O[Filtered response to AI]
    N --> O

Loading

Data Flow

sequenceDiagram
    participant Config as ai_mcp_servers config
    participant AI as AICheckProvider
    participant SSE as CustomToolsSSEServer
    participant HTTP as HTTP Client
    participant API as External API
    
    Config->>AI: http_client entry with transform_js/rate_limit
    AI->>AI: Build CustomToolDefinition (FIX: preserve fields)
    AI->>SSE: Register tool with preserved config
    AI->>SSE: Start SSE server
    
    Note over AI: AI agent decides to call tool
    AI->>SSE: MCP tool call
    SSE->>HTTP: Execute HTTP request
    HTTP->>API: Apply rate_limit if configured
    API->>HTTP: Response data
    HTTP->>SSE: Raw response
    SSE->>SSE: Apply transform_js if configured
    SSE->>AI: Filtered/transformed response
    
    Note over AI: AI receives clean data

Loading

Affected System Components

1. AI Check Provider (src/providers/ai-check-provider.ts)

   • Tool extraction logic from ai_mcp_servers
   • CustomToolDefinition construction

2. MCP Custom Tools SSE Server (src/providers/mcp-custom-sse-server.ts)

   • Executes http_client tools
   • Applies transform_js filters (lines 1439-1462)
   • Applies rate_limit via rateLimitedFetch

3. HTTP Client Provider (src/providers/http-client-provider.ts)

   • Standalone http_client check type
   • Also supports transform_js and rate_limit
   • Reference implementation for tool behavior

4. Config Reload System (refactored)

   • cli-main.ts - Simplified startup
   • config-reload-runtime.ts - New module
   • config-watcher.ts - Enhanced with signal option

Scope Discovery & Context

Direct Impact

• All http_client tools configured via ai_mcp_servers or ai_mcp_servers_js
• REST API integrations using transform_js for response filtering
• High-volume API calls relying on rate_limit configuration

Related Files (for review)

Type Definitions:

• src/types/config.ts - CustomToolDefinition interface (lines 1234-1315)
• src/types/config.ts - RateLimitConfig interface (lines 507-527)

Execution Logic:

• src/providers/mcp-custom-sse-server.ts:1439-1462 - transform_js application
• src/providers/mcp-custom-sse-server.ts:1410 - rate_limit usage
• src/providers/http-client-provider.ts:299-335 - transform_js in standalone provider
• src/providers/http-client-provider.ts:240-275 - rate_limit in standalone provider

Utilities:

• src/utils/rate-limiter.ts - Token bucket implementation
• src/utils/sandbox.ts - Secure JavaScript execution for transform_js

Tests:

• tests/unit/providers/http-client-provider.test.ts:615-680 - transform_js tests
• tests/unit/utils/rate-limiter.test.ts:142-264 - rate_limit tests

Potential Edge Cases

1. Backward Compatibility: Tools without these fields continue to work (conditional spread)
2. Type Safety: Proper type assertions ensure type correctness
3. Runtime Validation: transform_js syntax is validated at config load time
4. Error Handling: transform_js execution errors are caught and logged

Testing Strategy

Manual Testing (from PR description)

• ✅ Verified transform_js is defined on CustomToolDefinition interface
• ✅ Built successfully
• ✅ Workable transform unit tests pass with --no-mocks
• ⏳ Run screening test with --no-mocks and verify "Filtered N disqualified candidates" appears in logs

Automated Testing

• ✅ New test suite for config reload runtime (130 lines)
• ✅ Existing tests for transform_js functionality
• ✅ Existing tests for rate_limit functionality

References

Code Changes

• src/providers/ai-check-provider.ts:1510-1516 - Core fix (preserve transform_js and rate_limit)
• src/runners/config-reload-runtime.ts - New config reload module
• src/config/config-watcher.ts:116-129 - Enhanced start() method
• src/cli-main.ts:1821-1838 - Simplified config reload setup
• tests/unit/runners/config-reload-runtime.test.ts - Comprehensive test coverage

Related Documentation

• docs/ai-custom-tools.md - Custom tools configuration
• docs/ai-custom-tools-usage.md - Tool usage patterns
• docs/configuration.md - Config file reference

Type Definitions

• src/types/config.ts:1234-1315 - CustomToolDefinition interface
• src/types/config.ts:507-527 - RateLimitConfig interface

Metadata

• Review Effort: 2 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2026-03-24T10:26:05.862Z | Triggered by: pr_opened | Commit: 4739e05

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Architecture Issues (1)

Severity	Location	Issue
🟠 Error	contract:0	Output schema validation failed: must have required property 'issues'

Performance Issues (5)

Severity	Location	Issue
🟡 Warning	src/runners/config-reload-runtime.ts:47	The ConfigWatcher is started with { listenForSignals: false } to prevent duplicate SIGUSR2 handlers, but this creates a subtle performance issue: the watcher still performs platform checks on every start() call even though signals are disabled. This adds unnecessary overhead during initialization. 💡 Suggestion Consider refactoring ConfigWatcher.start() to accept a more explicit signal handling mode (e.g., 'none', 'watcher-only', 'external') to avoid redundant platform checks when signals are handled externally. Alternatively, document this pattern more clearly to prevent future confusion.
🟡 Warning	src/providers/ai-check-provider.ts:1510	The code uses conditional object spread to preserve transform_js and rate_limit fields. While functionally correct, this pattern creates new object literals on every iteration of the httpClientEntriesFromMcp loop, which could cause minor memory pressure in configurations with many http_client tools. 💡 Suggestion For configurations with a large number of http_client tools, consider pre-building a base object with common fields and using Object.assign() or restructuring to avoid creating intermediate objects. However, for typical use cases (fewer than 10 http_client tools), this overhead is negligible.
🟡 Warning	src/runners/config-reload-runtime.ts:28	The ConfigSnapshotStore.initialize() is called with await but the error handling only logs a warning. If initialization fails silently, the snapshotStore remains undefined, and subsequent operations that depend on it will fail. This could lead to degraded performance if retries occur. 💡 Suggestion Consider implementing a retry mechanism or failing fast when snapshot store initialization fails, rather than continuing in a degraded state. Silent failures can lead to harder-to-diagnose performance issues later.
🟡 Warning	src/config/config-watcher.ts:116	The start() method performs process.platform !== 'win32' check every time it's called, even though the platform doesn't change at runtime. This is a micro-optimization but could be cached at the class level. 💡 Suggestion Cache the platform check result in the constructor or class property to avoid repeated string comparisons. This is a minor optimization but shows attention to detail in hot paths.
🟡 Warning	src/runners/config-reload-runtime.ts:68	The cleanup() method uses .catch(() => {}) to suppress errors from snapshotStore.shutdown(). This could hide database connection leaks or other resource cleanup failures that might accumulate over time in long-running processes. 💡 Suggestion Consider logging cleanup errors instead of silently suppressing them. Resource cleanup failures should be visible for debugging potential memory leaks or connection pool exhaustion. 🔧 Suggested Fix if (snapshotStore) { await snapshotStore.shutdown().catch((err) => { logger.warn(`[ConfigReload] Snapshot store cleanup failed: ${err}`); }); }

\n\n

Architecture Issues (1)

Severity	Location	Issue
🟠 Error	contract:0	Output schema validation failed: must have required property 'issues'

\n\n ### Performance Issues (5)

Severity	Location	Issue
🟡 Warning	src/runners/config-reload-runtime.ts:47	The ConfigWatcher is started with { listenForSignals: false } to prevent duplicate SIGUSR2 handlers, but this creates a subtle performance issue: the watcher still performs platform checks on every start() call even though signals are disabled. This adds unnecessary overhead during initialization. 💡 Suggestion Consider refactoring ConfigWatcher.start() to accept a more explicit signal handling mode (e.g., 'none', 'watcher-only', 'external') to avoid redundant platform checks when signals are handled externally. Alternatively, document this pattern more clearly to prevent future confusion.
🟡 Warning	src/providers/ai-check-provider.ts:1510	The code uses conditional object spread to preserve transform_js and rate_limit fields. While functionally correct, this pattern creates new object literals on every iteration of the httpClientEntriesFromMcp loop, which could cause minor memory pressure in configurations with many http_client tools. 💡 Suggestion For configurations with a large number of http_client tools, consider pre-building a base object with common fields and using Object.assign() or restructuring to avoid creating intermediate objects. However, for typical use cases (fewer than 10 http_client tools), this overhead is negligible.
🟡 Warning	src/runners/config-reload-runtime.ts:28	The ConfigSnapshotStore.initialize() is called with await but the error handling only logs a warning. If initialization fails silently, the snapshotStore remains undefined, and subsequent operations that depend on it will fail. This could lead to degraded performance if retries occur. 💡 Suggestion Consider implementing a retry mechanism or failing fast when snapshot store initialization fails, rather than continuing in a degraded state. Silent failures can lead to harder-to-diagnose performance issues later.
🟡 Warning	src/config/config-watcher.ts:116	The start() method performs process.platform !== 'win32' check every time it's called, even though the platform doesn't change at runtime. This is a micro-optimization but could be cached at the class level. 💡 Suggestion Cache the platform check result in the constructor or class property to avoid repeated string comparisons. This is a minor optimization but shows attention to detail in hot paths.
🟡 Warning	src/runners/config-reload-runtime.ts:68	The cleanup() method uses .catch(() => {}) to suppress errors from snapshotStore.shutdown(). This could hide database connection leaks or other resource cleanup failures that might accumulate over time in long-running processes. 💡 Suggestion Consider logging cleanup errors instead of silently suppressing them. Resource cleanup failures should be visible for debugging potential memory leaks or connection pool exhaustion. 🔧 Suggested Fix if (snapshotStore) { await snapshotStore.shutdown().catch((err) => { logger.warn(`[ConfigReload] Snapshot store cleanup failed: ${err}`); }); }

\n\n ### Quality Issues (1)

Severity	Location	Issue
🟠 Error	contract:0	Output schema validation failed: must have required property 'issues'

---

Powered by Visor from Probelabs

Last updated: 2026-03-24T10:18:46.029Z | Triggered by: pr_opened | Commit: 4739e05

💡 TIP: You can chat with Visor using /visor ask <your question>