feat: global AI concurrency limiter (max_ai_concurrency)

[buger]

Summary

• Adds max_ai_concurrency config option to VisorConfig that caps concurrent AI API calls across all checks in a run
• Creates a shared DelegationManager in buildEngineContextForRun when max_ai_concurrency is set
• Propagates the limiter through ai-check-provider → AIReviewService → ProbeAgent via _parentContext.sharedConcurrencyLimiter
• Adds DelegationManager to the @probelabs/probe mock for test support
• Adds 5 unit tests covering context creation, limiter propagation, and absence when unconfigured

Test plan

• npx jest tests/unit/concurrency-limiter.test.ts — 5/5 pass
• Full test suite: 265/266 suites pass (1 pre-existing telemetry e2e failure unrelated to this change)

🤖 Generated with Claude Code

[probelabs]

PR Overview: Global AI Concurrency Limiter

Summary

This PR introduces a global AI concurrency limiter (max_ai_concurrency) that caps concurrent AI API calls across all checks in a Visor run. This prevents overwhelming AI providers when multiple checks execute in parallel.

Files Changed

File	Changes	Description
__mocks__/@probelabs/probe.ts	+66 lines	Added mock DelegationManager class for testing concurrency limiter features
src/ai-review-service.ts	+7 lines	Added concurrencyLimiter to AIReviewConfig interface and propagation to ProbeAgent options
src/providers/ai-check-provider.ts	+6 lines	Propagates shared limiter from _parentContext.sharedConcurrencyLimiter to AIReviewService
src/providers/command-check-provider.ts	+4/-21 lines	Simplified command normalization (removed multiline eval handling)
src/state-machine/context/build-engine-context.ts	+27 lines	Creates shared DelegationManager when max_ai_concurrency is configured
src/types/config.ts	+4 lines	Added max_ai_concurrency?: number to VisorConfig interface
src/types/engine.ts	+2 lines	Added sharedConcurrencyLimiter?: any to EngineContext interface
tests/unit/concurrency-limiter.test.ts	+179 lines	New test file with 5 unit tests covering limiter creation and propagation
package.json + package-lock.json	Updated	Bumped @probelabs/probe from rc230 to rc232
tests/e2e/telemetry-transform-e2e.test.ts	+2/-2 lines	Fixed newline escaping in test commands

Total: 311 additions, 454 deletions across 13 files

Architecture & Impact Assessment

What This PR Accomplishes

• Adds a new configuration option max_ai_concurrency to globally limit concurrent AI API calls
• Creates a shared DelegationManager instance at engine initialization when the limit is configured
• Propagates the limiter through the call chain: EngineContext → AICheckProvider → AIReviewService → ProbeAgent
• Gracefully handles cases where DelegationManager is unavailable (older probe versions)

Key Technical Changes

1. Configuration Layer (src/types/config.ts)

   • Added max_ai_concurrency?: number to VisorConfig
   • Documented as "Maximum total concurrent AI API calls across all checks (default: unlimited)"

2. Engine Context (src/state-machine/context/build-engine-context.ts)

   • Lazy-loads DelegationManager from @probelabs/probe with graceful fallback
   • Creates shared limiter instance when max_ai_concurrency is set
   • Sets maxPerSession: 999 to disable per-session limits (only global limiting needed)
   • Adds limiter to EngineContext.sharedConcurrencyLimiter

3. Provider Layer (src/providers/ai-check-provider.ts)

   • Extracts sharedConcurrencyLimiter from sessionInfo._parentContext
   • Passes to AIReviewService via aiConfig.concurrencyLimiter

4. Service Layer (src/ai-review-service.ts)

   • Added concurrencyLimiter?: any to AIReviewConfig interface
   • Propagates to ProbeAgent options during instantiation

5. Testing (tests/unit/concurrency-limiter.test.ts)

   • 5 comprehensive unit tests covering:
     • Limiter creation when max_ai_concurrency is set
     • No limiter when unconfigured
     • Edge case: max_ai_concurrency: 1
     • Propagation through ai-check-provider
     • No propagation when limiter absent

Affected System Components

graph TD
    A[VisorConfig] -->|max_ai_concurrency| B[buildEngineContextForRun]
    B -->|creates| C[DelegationManager]
    C -->|sharedConcurrencyLimiter| D[EngineContext]
    D -->|_parentContext| E[AICheckProvider]
    E -->|concurrencyLimiter| F[AIReviewService]
    F -->|concurrencyLimiter| G[ProbeAgent]
    G -->|acquire/release| H[AI API Calls]
    
    style C fill:#e1f5ff
    style D fill:#fff4e1
    style E fill:#f0e1ff
    style F fill:#ffe1f0

Loading

The concurrency limiter follows a dependency injection pattern:

1. Factory: buildEngineContextForRun creates the limiter singleton
2. Context: EngineContext carries the limiter as shared state
3. Provider: AICheckProvider extracts from parent context
4. Service: AIReviewService receives and forwards to agent
5. Agent: ProbeAgent uses DelegationManager.acquire/release for gating

Scope Discovery & Context Expansion

Direct Impact

• All AI checks using ProbeAgent will respect the global concurrency limit
• Parallel check execution (max_parallelism) can now be higher than AI concurrency without overwhelming providers
• Resource management: Prevents API rate limits and cost overruns from parallel AI calls

Related Files (Inferred)

Based on the changes, these files are likely related but not modified:

• src/state-machine/engine.ts - Uses EngineContext during execution
• src/providers/check-provider.interface.ts - Defines ExecutionContext with _parentContext
• docs/configuration.md - Should document the new max_ai_concurrency option

Potential Edge Cases

1. Older probe versions: Code gracefully handles missing DelegationManager export
2. Zero or negative values: Not explicitly validated (may need config validation)
3. Interaction with max_parallelism: High parallelism + low AI concurrency = queue buildup
4. Cleanup: Limiter has cleanup() method but unclear when called

Testing Coverage

✅ Well-covered:

• Limiter creation with various values
• Propagation through provider chain
• Absence when unconfigured

⚠️ Potential gaps:

• Integration test with actual parallel AI checks
• Behavior when limit is reached (queuing/rejection)
• Cleanup on engine shutdown
• Interaction with max_parallelism setting

Reviewer Guidance

Key Areas to Review

1. build-engine-context.ts (lines 11-23, 116-126): Lazy loading and limiter creation logic
2. ai-check-provider.ts (lines 1008-1011): Context extraction and propagation
3. ai-review-service.ts (lines 221-222, 1882-1885): Config interface and ProbeAgent option passing
4. tests/unit/concurrency-limiter.test.ts: Test coverage and assertions

Potential Concerns

• Type safety: Limiter typed as any throughout (could use proper interface)
• Error handling: No validation of max_ai_concurrency value (must be positive integer)
• Documentation: New config option needs user-facing documentation
• Backward compatibility: Gracefully handles missing DelegationManager but may silently ignore config

Configuration Example

# .visor.yaml
max_parallelism: 10      # Run up to 10 checks in parallel
max_ai_concurrency: 3    # But only 3 concurrent AI API calls

checks:
  security-review:
    type: ai
    prompt: security
  code-review:
    type: ai
    prompt: review
  # ... 8 more AI checks
# All 10 checks can start, but only 3 will make AI calls at once

Metadata

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

---

Powered by Visor from Probelabs

Last updated: 2026-02-15T19:15:02.349Z | Triggered by: pr_updated | Commit: ce33a3c

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

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

Architecture Issues (8)

Severity	Location	Issue
🟠 Error	src/state-machine/context/build-engine-context.ts:117	The sharedConcurrencyLimiter is created but never explicitly cleaned up. DelegationManager has a cleanup() method that should be called to release resources and reject pending promises. This could cause memory leaks or hanging promises. 💡 Suggestion Implement proper lifecycle management for sharedConcurrencyLimiter. Add cleanup logic in the engine shutdown/completion path. Consider using a disposal pattern or ensuring cleanup is called even on error paths. 🔧 Suggested Fix // Add to EngineContext: export interface EngineContext { // ... existing properties /** Cleanup function for shared resources */ cleanup?: () => Promise<void> | void; } // In buildEngineContextForRun: return { // ... existing properties cleanup: async () => { if (sharedConcurrencyLimiter) { sharedConcurrencyLimiter.cleanup(); } }, }; // Call cleanup in engine completion path
🟢 Info	src/providers/ai-check-provider.ts:1008	Extracting sharedConcurrencyLimiter from _parentContext using optional chaining is inconsistent with how other shared resources (policyEngine, workspace) are accessed. This creates an inconsistent pattern for accessing shared context. 💡 Suggestion Standardize how shared resources are accessed from _parentContext. Consider creating a helper method or consistent pattern for extracting optional shared resources. The current approach mixes direct property access with optional chaining in an inconsistent way. 🔧 Suggested Fix // Consider a consistent pattern for accessing shared resources: const getSharedResource = <T>(key: string): T | undefined => { return (sessionInfo as any)?._parentContext?.[key]; }; const sharedLimiter = getSharedResource<any>('sharedConcurrencyLimiter'); const policyEngine = getSharedResource<any>('policyEngine'); const workspace = getSharedResource<any>('workspace');
🟡 Warning	src/state-machine/context/build-engine-context.ts:11	Lazy-loading DelegationManager with try/catch and module-level caching is over-engineered for this use case. The graceful fallback silently ignores configuration errors, making debugging difficult. If DelegationManager is unavailable, users should get a clear error rather than silent failure. 💡 Suggestion Replace lazy-loading with direct import. Let module resolution fail naturally if DelegationManager doesn't exist. This provides clearer error messages and reduces complexity. If backward compatibility is needed, use a feature flag or version check instead of silent fallback. 🔧 Suggested Fix // Direct import - fails fast if DelegationManager unavailable import { DelegationManager } from '@probelabs/probe'; // In buildEngineContextForRun: if (clonedConfig.max_ai_concurrency) { sharedConcurrencyLimiter = new DelegationManager({ maxConcurrent: clonedConfig.max_ai_concurrency, maxPerSession: 999, }); }
🟡 Warning	src/state-machine/context/build-engine-context.ts:121	Hard-coded magic number 999 for maxPerSession is a special case that obscures intent. This value was chosen to disable per-session limits, but there's no documentation explaining why 999 specifically or whether this could cause issues. 💡 Suggestion Use a named constant or Infinity to clearly express intent. Consider adding a comment explaining why per-session limits are disabled for global AI concurrency limiting. 🔧 Suggested Fix // Disable per-session limits - only global limiting needed for AI concurrency const DISABLE_PER_SESSION_LIMITS = Infinity; sharedConcurrencyLimiter = new _DelegationManager({ maxConcurrent: clonedConfig.max_ai_concurrency, maxPerSession: DISABLE_PER_SESSION_LIMITS, });
🟡 Warning	src/ai-review-service.ts:221	Using 'any' type for concurrencyLimiter bypasses TypeScript's type checking. This defeats the purpose of TypeScript and makes the codebase less maintainable. The DelegationManager interface should be properly imported and typed. 💡 Suggestion Import and use the proper DelegationManager type from @probelabs/probe. If the type is not available in older versions, create a local interface definition that matches the expected API (acquire, release, getStats, cleanup). 🔧 Suggested Fix import type { DelegationManager } from '@probelabs/probe'; interface AIReviewConfig { // ... other properties /** Shared concurrency limiter for global AI call gating */ concurrencyLimiter?: DelegationManager; }
🟡 Warning	src/types/engine.ts:127	sharedConcurrencyLimiter typed as 'any' propagates type unsafety throughout the codebase. This is particularly problematic in EngineContext which is a core type used across the state machine. 💡 Suggestion Use proper typing for sharedConcurrencyLimiter. Create a shared interface if DelegationManager import is problematic for backward compatibility. 🔧 Suggested Fix import type { DelegationManager } from '@probelabs/probe'; export interface EngineContext { // ... other properties /** Shared concurrency limiter for global AI call gating across all ProbeAgent instances */ sharedConcurrencyLimiter?: DelegationManager; }
🟡 Warning	src/types/config.ts:1423	max_ai_concurrency configuration option lacks validation. Zero, negative, or non-integer values could cause unexpected behavior. The configuration schema should validate this is a positive integer. 💡 Suggestion Add validation for max_ai_concurrency in the config loading/normalization phase. Ensure it's a positive integer greater than 0. Provide clear error messages for invalid values. 🔧 Suggested Fix // In config validation: if (config.max_ai_concurrency !== undefined) { if (typeof config.max_ai_concurrency !== 'number' || !Number.isInteger(config.max_ai_concurrency) || config.max_ai_concurrency < 1) { throw new Error( `max_ai_concurrency must be a positive integer, got: ${config.max_ai_concurrency}` ); } }
🟡 Warning	__mocks__/@probelabs/probe.ts:31	Mock DelegationManager duplicates the real implementation from @probelabs/probe. This creates maintenance burden and risks drift between mock and real implementations. The mock is 66 lines of complex concurrency logic that must stay in sync. 💡 Suggestion Consider using the real DelegationManager in tests instead of maintaining a mock. If mocking is necessary, use Jest's automock or a simpler mock that only implements the specific methods needed. Alternatively, extract the concurrency limiting logic into a shared utility that can be tested independently. 🔧 Suggested Fix // Use Jest automock: jest.mock('@probelabs/probe', () => ({ ...jest.requireActual('@probelabs/probe'), DelegationManager: jest.fn().mockImplementation((opts) => ({ acquire: jest.fn(), release: jest.fn(), getStats: jest.fn(() => ({ maxConcurrent: opts.maxConcurrent })), cleanup: jest.fn(), })), }));

Performance Issues (1)

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

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-02-15T19:15:05.348Z | Triggered by: pr_updated | Commit: ce33a3c

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