feat: add P2 features — retry policy, prompt templating, usage tracking, tech debt cleanup
- Extract shared splitMessage() into channels/utils.ts (dedup 4 adapters) - Add Slack user name resolution with caching (users.info API) - Add withRetry() with exponential backoff + jitter, isRetryable() filter - Wire retry config into ModelRouter.chat() (non-streaming only) - Add assembleSystemPrompt() multi-file template system (SOUL/AGENTS/IDENTITY/USER/TOOLS.md) - Add usage tracking accumulators in NativeAgent + AgentOrchestrator - Add estimateCost() with per-model pricing table - Add /usage TUI command with full usage report formatting - Add retrySchema and promptSchema to config schema Tests: 569 passing, typecheck clean
This commit is contained in:
@@ -0,0 +1,169 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { isRetryable, withRetry, DEFAULT_RETRY_CONFIG } from './retry.js';
|
||||
import type { RetryConfig } from './retry.js';
|
||||
|
||||
describe('isRetryable', () => {
|
||||
it('returns true for generic errors', () => {
|
||||
const error = new Error('Connection timeout');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for authentication errors', () => {
|
||||
const error = new Error('Invalid API key: authentication failed');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for invalid_api_key errors', () => {
|
||||
const error = new Error('Error: invalid_api_key');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for unauthorized errors', () => {
|
||||
const error = new Error('Request unauthorized');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for invalid_request errors', () => {
|
||||
const error = new Error('invalid_request: missing parameter');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for context_length_exceeded errors', () => {
|
||||
const error = new Error('context_length_exceeded: max 128k tokens');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for content_policy errors', () => {
|
||||
const error = new Error('content_policy violation detected');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('is case-insensitive when matching patterns', () => {
|
||||
const error = new Error('AUTHENTICATION error');
|
||||
expect(isRetryable(error, DEFAULT_RETRY_CONFIG.nonRetryablePatterns)).toBe(false);
|
||||
});
|
||||
|
||||
it('uses custom patterns when provided', () => {
|
||||
const error = new Error('quota exceeded');
|
||||
expect(isRetryable(error, ['quota'])).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('withRetry', () => {
|
||||
// Use minimal real delays to avoid fake-timer race conditions
|
||||
const fastConfig: RetryConfig = {
|
||||
maxRetries: 3,
|
||||
initialDelayMs: 1,
|
||||
backoffMultiplier: 1,
|
||||
maxDelayMs: 5,
|
||||
nonRetryablePatterns: DEFAULT_RETRY_CONFIG.nonRetryablePatterns,
|
||||
};
|
||||
|
||||
it('succeeds on first attempt without delay', async () => {
|
||||
const fn = vi.fn().mockResolvedValue('success');
|
||||
|
||||
const result = await withRetry(fn, fastConfig);
|
||||
|
||||
expect(result).toBe('success');
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('retries on transient failure then succeeds', async () => {
|
||||
const fn = vi.fn()
|
||||
.mockRejectedValueOnce(new Error('timeout'))
|
||||
.mockRejectedValueOnce(new Error('timeout'))
|
||||
.mockResolvedValueOnce('recovered');
|
||||
|
||||
const result = await withRetry(fn, fastConfig, 'test-op');
|
||||
|
||||
expect(result).toBe('recovered');
|
||||
expect(fn).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('throws after maxRetries exhausted', async () => {
|
||||
const fn = vi.fn().mockRejectedValue(new Error('persistent failure'));
|
||||
|
||||
await expect(withRetry(fn, fastConfig, 'test-op')).rejects.toThrow('persistent failure');
|
||||
// 1 initial + 3 retries = 4 total
|
||||
expect(fn).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it('does not retry non-retryable errors', async () => {
|
||||
const fn = vi.fn().mockRejectedValue(new Error('invalid_api_key'));
|
||||
|
||||
await expect(withRetry(fn, fastConfig)).rejects.toThrow('invalid_api_key');
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not retry authentication errors', async () => {
|
||||
const fn = vi.fn().mockRejectedValue(new Error('Request unauthorized'));
|
||||
|
||||
await expect(withRetry(fn, fastConfig)).rejects.toThrow('Request unauthorized');
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('converts non-Error throws to Error objects', async () => {
|
||||
const fn = vi.fn().mockRejectedValue('string error');
|
||||
|
||||
await expect(withRetry(fn, { ...fastConfig, maxRetries: 0 })).rejects.toThrow('string error');
|
||||
});
|
||||
|
||||
it('respects maxDelayMs cap', async () => {
|
||||
const cappedConfig: RetryConfig = {
|
||||
maxRetries: 2,
|
||||
initialDelayMs: 1,
|
||||
backoffMultiplier: 10,
|
||||
maxDelayMs: 2,
|
||||
nonRetryablePatterns: [],
|
||||
};
|
||||
|
||||
let callCount = 0;
|
||||
const fn = vi.fn().mockImplementation(() => {
|
||||
callCount++;
|
||||
if (callCount < 3) return Promise.reject(new Error('fail'));
|
||||
return Promise.resolve('ok');
|
||||
});
|
||||
|
||||
// If maxDelayMs weren't respected, a 10x multiplier could cause very long waits.
|
||||
// With maxDelayMs=2ms, this completes quickly.
|
||||
const result = await withRetry(fn, cappedConfig, 'capped-test');
|
||||
expect(result).toBe('ok');
|
||||
expect(fn).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('uses default config when none provided', async () => {
|
||||
const fn = vi.fn().mockResolvedValue('default-config');
|
||||
|
||||
const result = await withRetry(fn);
|
||||
|
||||
expect(result).toBe('default-config');
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('increases delay exponentially between retries', async () => {
|
||||
const timestamps: number[] = [];
|
||||
const config: RetryConfig = {
|
||||
maxRetries: 2,
|
||||
initialDelayMs: 20,
|
||||
backoffMultiplier: 2,
|
||||
maxDelayMs: 1000,
|
||||
nonRetryablePatterns: [],
|
||||
};
|
||||
|
||||
const fn = vi.fn().mockImplementation(() => {
|
||||
timestamps.push(Date.now());
|
||||
if (timestamps.length < 3) return Promise.reject(new Error('fail'));
|
||||
return Promise.resolve('ok');
|
||||
});
|
||||
|
||||
await withRetry(fn, config, 'backoff-test');
|
||||
|
||||
expect(fn).toHaveBeenCalledTimes(3);
|
||||
// First retry delay: ~20ms (jitter 50-100% of 20 = 10-20ms)
|
||||
// Second retry delay: ~40ms (jitter 50-100% of 40 = 20-40ms)
|
||||
const firstDelay = timestamps[1] - timestamps[0];
|
||||
const secondDelay = timestamps[2] - timestamps[1];
|
||||
// Second delay should be roughly double the first (within jitter range)
|
||||
expect(secondDelay).toBeGreaterThanOrEqual(firstDelay * 0.7);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user