Li Liu
2b0e1ba711
Phase A - Quick fixes: - Remove debug logging that leaked prompt content (qwen-code) - Fix orchestrator session leak with try/finally cleanup - CJK-aware token estimation for better accuracy - Issue parser validation (line > 0, endLine >= line, non-empty fields) - Improved similarity matching with stop words filtering and description weight Phase B - Medium fixes: - Add retry utility with exponential backoff for API providers - Config validation at load time (required fields, empty API key warnings) - GitHub PR comment deduplication (skip already-posted comments) - Ctrl+C graceful exit for interactive comment review Phase C - Structured logging: - Logger class with debug/info/warn/error levels (MAGPIE_LOG_LEVEL env var) Phase D - Type safety: - Replace `any` types with proper types across discuss.ts, review.ts, issue-parser.ts, commenter.ts, repo-orchestrator.ts, history-collector.ts Phase E - Session helper extraction: - CliSessionHelper class shared by 4 CLI providers, reducing duplication Phase F - Split review.ts (1991 → 6 files): - review.ts (command + action), interactive.ts, repo-review.ts, session-cmds.ts, utils.ts, types.ts Phase G - Tests: - 6 new test files (retry, logger, session-helper, issue-parser-enhanced, loader-validation, orchestrator-session) - Fix pre-existing test failures (commenter, anthropic) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
77 lines
2.7 KiB
TypeScript
77 lines
2.7 KiB
TypeScript
import { describe, it, expect, vi } from 'vitest'
|
|
import { withRetry } from '../../src/utils/retry.js'
|
|
|
|
describe('withRetry', () => {
|
|
it('returns result on first success', async () => {
|
|
const fn = vi.fn().mockResolvedValue('ok')
|
|
const result = await withRetry(fn)
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(1)
|
|
})
|
|
|
|
it('retries on transient error then succeeds', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce({ code: 'ETIMEDOUT' })
|
|
.mockResolvedValue('ok')
|
|
const result = await withRetry(fn, { backoffMs: [1, 1, 1] })
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(2)
|
|
})
|
|
|
|
it('retries on 429 status error', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce({ status: 429, message: 'rate limited' })
|
|
.mockResolvedValue('ok')
|
|
const result = await withRetry(fn, { backoffMs: [1, 1, 1] })
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(2)
|
|
})
|
|
|
|
it('retries on 5xx status error', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce({ status: 503, message: 'service unavailable' })
|
|
.mockResolvedValue('ok')
|
|
const result = await withRetry(fn, { backoffMs: [1, 1, 1] })
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(2)
|
|
})
|
|
|
|
it('throws immediately on non-transient error', async () => {
|
|
const fn = vi.fn().mockRejectedValue(new Error('auth failed'))
|
|
await expect(withRetry(fn, { backoffMs: [1, 1, 1] })).rejects.toThrow('auth failed')
|
|
expect(fn).toHaveBeenCalledTimes(1)
|
|
})
|
|
|
|
it('exhausts all attempts and throws last error', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce({ code: 'ECONNRESET', message: 'reset 1' })
|
|
.mockRejectedValueOnce({ code: 'ECONNRESET', message: 'reset 2' })
|
|
.mockRejectedValue({ code: 'ECONNRESET', message: 'reset 3' })
|
|
await expect(withRetry(fn, { maxAttempts: 3, backoffMs: [1, 1, 1] })).rejects.toEqual(
|
|
expect.objectContaining({ code: 'ECONNRESET', message: 'reset 3' })
|
|
)
|
|
expect(fn).toHaveBeenCalledTimes(3)
|
|
})
|
|
|
|
it('respects custom shouldRetry', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce(new Error('custom retryable'))
|
|
.mockResolvedValue('ok')
|
|
const result = await withRetry(fn, {
|
|
backoffMs: [1],
|
|
shouldRetry: (err) => err instanceof Error && err.message.includes('retryable')
|
|
})
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(2)
|
|
})
|
|
|
|
it('retries on message-based transient detection', async () => {
|
|
const fn = vi.fn()
|
|
.mockRejectedValueOnce(new Error('connection timeout occurred'))
|
|
.mockResolvedValue('ok')
|
|
const result = await withRetry(fn, { backoffMs: [1] })
|
|
expect(result).toBe('ok')
|
|
expect(fn).toHaveBeenCalledTimes(2)
|
|
})
|
|
})
|