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>
108 lines
3.5 KiB
TypeScript
108 lines
3.5 KiB
TypeScript
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
|
import type { MagpieConfig } from '../../src/config/types.js'
|
|
|
|
// Mock fs and yaml
|
|
vi.mock('fs', () => ({
|
|
readFileSync: vi.fn(),
|
|
existsSync: vi.fn()
|
|
}))
|
|
vi.mock('yaml', () => ({
|
|
parse: vi.fn()
|
|
}))
|
|
|
|
// Mock logger to suppress output
|
|
vi.mock('../../src/utils/logger.js', () => ({
|
|
logger: {
|
|
warn: vi.fn(),
|
|
error: vi.fn(),
|
|
info: vi.fn(),
|
|
debug: vi.fn()
|
|
}
|
|
}))
|
|
|
|
import { loadConfig } from '../../src/config/loader.js'
|
|
import { existsSync, readFileSync } from 'fs'
|
|
import { parse } from 'yaml'
|
|
import { logger } from '../../src/utils/logger.js'
|
|
|
|
const validConfig: MagpieConfig = {
|
|
defaults: { max_rounds: 3, check_convergence: true },
|
|
providers: {
|
|
anthropic: { api_key: 'test-key' }
|
|
},
|
|
reviewers: {
|
|
claude: { model: 'anthropic:claude-3-5-sonnet', prompt: 'Review this code' }
|
|
},
|
|
summarizer: { model: 'anthropic:claude-3-5-sonnet', prompt: 'Summarize' },
|
|
analyzer: { model: 'anthropic:claude-3-5-sonnet', prompt: 'Analyze' }
|
|
}
|
|
|
|
describe('loadConfig - validation', () => {
|
|
beforeEach(() => {
|
|
vi.resetAllMocks()
|
|
vi.mocked(existsSync).mockReturnValue(true)
|
|
vi.mocked(readFileSync).mockReturnValue('yaml content')
|
|
})
|
|
|
|
it('loads valid config without error', () => {
|
|
vi.mocked(parse).mockReturnValue(structuredClone(validConfig))
|
|
expect(() => loadConfig('/path/to/config.yaml')).not.toThrow()
|
|
})
|
|
|
|
it('throws when max_rounds <= 0', () => {
|
|
const bad = structuredClone(validConfig)
|
|
bad.defaults.max_rounds = 0
|
|
vi.mocked(parse).mockReturnValue(bad)
|
|
expect(() => loadConfig('/path/to/config.yaml')).toThrow('max_rounds must be > 0')
|
|
})
|
|
|
|
it('throws when no reviewers defined', () => {
|
|
const bad = structuredClone(validConfig)
|
|
bad.reviewers = {}
|
|
vi.mocked(parse).mockReturnValue(bad)
|
|
expect(() => loadConfig('/path/to/config.yaml')).toThrow('at least one reviewer')
|
|
})
|
|
|
|
it('throws when reviewer missing model', () => {
|
|
const bad = structuredClone(validConfig)
|
|
bad.reviewers = { claude: { model: '', prompt: 'test' } }
|
|
vi.mocked(parse).mockReturnValue(bad)
|
|
expect(() => loadConfig('/path/to/config.yaml')).toThrow('missing a "model" field')
|
|
})
|
|
|
|
it('throws when reviewer missing prompt', () => {
|
|
const bad = structuredClone(validConfig)
|
|
bad.reviewers = { claude: { model: 'test:model', prompt: '' } }
|
|
vi.mocked(parse).mockReturnValue(bad)
|
|
expect(() => loadConfig('/path/to/config.yaml')).toThrow('missing a "prompt" field')
|
|
})
|
|
|
|
it('throws when summarizer missing model', () => {
|
|
const bad = structuredClone(validConfig)
|
|
bad.summarizer = { model: '', prompt: 'summarize' }
|
|
vi.mocked(parse).mockReturnValue(bad)
|
|
expect(() => loadConfig('/path/to/config.yaml')).toThrow('summarizer is missing a "model"')
|
|
})
|
|
|
|
it('warns on empty API key', () => {
|
|
const config = structuredClone(validConfig)
|
|
config.providers = { anthropic: { api_key: '' } }
|
|
vi.mocked(parse).mockReturnValue(config)
|
|
loadConfig('/path/to/config.yaml')
|
|
expect(vi.mocked(logger.warn)).toHaveBeenCalledWith(
|
|
expect.stringContaining('api_key is empty')
|
|
)
|
|
})
|
|
|
|
it('does not warn when API key is set', () => {
|
|
vi.mocked(parse).mockReturnValue(structuredClone(validConfig))
|
|
loadConfig('/path/to/config.yaml')
|
|
expect(vi.mocked(logger.warn)).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('throws when config file not found', () => {
|
|
vi.mocked(existsSync).mockReturnValue(false)
|
|
expect(() => loadConfig('/path/to/missing.yaml')).toThrow('Config file not found')
|
|
})
|
|
})
|