diff --git a/src/config.ts b/src/config.ts index bc6c3fd..a94ffe6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -55,6 +55,22 @@ export interface Config { maxContextTokens: number } +/** Parse an integer env var; garbage (or trailing junk) falls back to the default. */ +function intFromEnv(name: string, fallback: number): number { + const raw = process.env[name]?.trim() + if (!raw || !/^\d+$/.test(raw)) return fallback + const n = parseInt(raw, 10) + return Number.isFinite(n) ? n : fallback +} + +/** Parse a float env var; garbage (or trailing junk) falls back to the default. */ +function floatFromEnv(name: string, fallback: number): number { + const raw = process.env[name]?.trim() + if (!raw || !/^-?\d+(\.\d+)?([eE][+-]?\d+)?$/.test(raw)) return fallback + const n = parseFloat(raw) + return Number.isFinite(n) ? n : fallback +} + export function configFromEnv(): Config { // load .env if present (won't override existing env vars) loadDotenv() @@ -66,8 +82,8 @@ export function configFromEnv(): Config { model: process.env.CORECODER_MODEL || 'gpt-5.5', apiKey, baseUrl: process.env.OPENAI_BASE_URL || process.env.CORECODER_BASE_URL || null, - maxTokens: parseInt(process.env.CORECODER_MAX_TOKENS || '4096', 10), - temperature: parseFloat(process.env.CORECODER_TEMPERATURE || '0'), - maxContextTokens: parseInt(process.env.CORECODER_MAX_CONTEXT || '128000', 10), + maxTokens: intFromEnv('CORECODER_MAX_TOKENS', 4096), + temperature: floatFromEnv('CORECODER_TEMPERATURE', 0), + maxContextTokens: intFromEnv('CORECODER_MAX_CONTEXT', 128000), } } diff --git a/src/llm.ts b/src/llm.ts index eb222cf..c36d095 100644 --- a/src/llm.ts +++ b/src/llm.ts @@ -136,6 +136,13 @@ class TransientError extends Error {} const sleep = (ms: number, signal?: AbortSignal) => new Promise((resolve, reject) => { + // a signal aborted *before* we got here (e.g. ^C landing between the + // failed request and the backoff) must reject immediately, not hang for + // the whole retry delay only to abort the next request anyway + if (signal?.aborted) { + reject(new DOMException('Aborted', 'AbortError')) + return + } const t = setTimeout(resolve, ms) signal?.addEventListener('abort', () => { clearTimeout(t) @@ -219,7 +226,15 @@ export class LLM implements LLMClient { for await (const data of sseEvents(res, signal)) { // Typed against the official SDK's wire contract (type-only import). - const chunk = JSON.parse(data) as ChatCompletionChunk + // Some "OpenAI-compatible" providers emit non-JSON data lines (heartbeats, + // keep-alives, stray whitespace); skip them rather than killing the whole + // stream on one malformed chunk. + let chunk: ChatCompletionChunk + try { + chunk = JSON.parse(data) as ChatCompletionChunk + } catch { + continue + } // usage info comes in the final chunk; some providers send usage with // null fields, so coerce to 0 to keep the running totals numeric diff --git a/src/tools/glob.ts b/src/tools/glob.ts index b2502b8..f4a5c90 100644 --- a/src/tools/glob.ts +++ b/src/tools/glob.ts @@ -13,6 +13,9 @@ import { expandPath } from './paths.js' /** Compile a glob pattern to a RegExp over posix-style relative paths. */ export function globToRegExp(pattern: string): RegExp { + // normalize Windows separators so `src\**\*.ts` behaves like `src/**/*.ts` + // (the walk produces /-joined relative paths, so a literal \ can never match) + pattern = pattern.replace(/\\/g, '/') let re = '' let i = 0 while (i < pattern.length) { diff --git a/src/tools/grep.ts b/src/tools/grep.ts index 30ec912..1412eb8 100644 --- a/src/tools/grep.ts +++ b/src/tools/grep.ts @@ -88,7 +88,11 @@ export const grepTool: Tool = { for (const line of text.split('\n')) { lineno++ if (regex.test(line)) { - matches.push(`${fp}:${lineno}: ${line.trimEnd()}`) + // cap match line width (same 2000-char limit as read_file): a single + // minified line would otherwise flood the context with one mega-match + const capped = + line.length > 2000 ? line.slice(0, 2000) + '… (line truncated)' : line + matches.push(`${fp}:${lineno}: ${capped.trimEnd()}`) if (matches.length >= MAX_MATCHES) { matches.push(`... (${MAX_MATCHES} match limit reached)`) return matches.join('\n') diff --git a/src/tools/read.ts b/src/tools/read.ts index 7e1349e..7f525c2 100644 --- a/src/tools/read.ts +++ b/src/tools/read.ts @@ -19,8 +19,16 @@ export const readFileTool: Tool = { async execute(args) { const filePath = String(args.file_path ?? '') - const offset = typeof args.offset === 'number' ? args.offset : 1 - const limit = typeof args.limit === 'number' ? args.limit : 2000 + // clamp junk arguments: a negative or zero limit would slice backwards and + // mislabel a real file as '(empty file)' + const offset = + typeof args.offset === 'number' && Number.isFinite(args.offset) + ? Math.max(1, Math.floor(args.offset)) + : 1 + const limit = + typeof args.limit === 'number' && Number.isFinite(args.limit) + ? Math.max(1, Math.floor(args.limit)) + : 2000 try { const p = expandPath(filePath) diff --git a/tests/robustness.test.ts b/tests/robustness.test.ts new file mode 100644 index 0000000..3df54c3 --- /dev/null +++ b/tests/robustness.test.ts @@ -0,0 +1,132 @@ +/** + * Regression tests for robustness fixes: retry-abort behavior, SSE tolerance, + * glob separators, argument clamping, env parsing, and grep output caps. + * + * Run with: npm test (tsc build + node --test) + */ + +import assert from 'node:assert/strict' +import { promises as fs } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { test } from 'node:test' + +import { configFromEnv } from '../src/config.js' +import { LLM, drain, type LLMResponse } from '../src/llm.js' +import { globToRegExp } from '../src/tools/glob.js' + +/** Drive a generator, collecting yields, and return { parts, resp }. */ +async function collect( + gen: AsyncGenerator, +): Promise<{ parts: string[]; resp: LLMResponse }> { + const parts: string[] = [] + let step = await gen.next() + while (!step.done) { + parts.push(step.value) + step = await gen.next() + } + return { parts, resp: step.value } +} + +// ---------------------------------------------------------------- llm: retry + abort + +test('retry backoff rejects immediately when the signal is already aborted', async () => { + const originalFetch = globalThis.fetch + // a provider that keeps answering 429, ignoring the abort signal: the abort + // lands *between* the transient failure and the backoff sleep, which used to + // hang the retry loop for the full delay before aborting the next request + globalThis.fetch = (async () => new Response('rate limited', { status: 429 })) as typeof fetch + const llm = new LLM({ model: 'm', apiKey: 'k' }) + const ac = new AbortController() + ac.abort() + + try { + const t0 = Date.now() + await assert.rejects( + drain(llm.chat([{ role: 'user', content: 'hi' }], undefined, ac.signal)), + (e: Error) => e.name === 'AbortError', + ) + assert.ok(Date.now() - t0 < 500, 'must reject immediately, not wait out the backoff') + } finally { + globalThis.fetch = originalFetch + } +}) + +test('stream survives non-JSON data lines from the provider', async () => { + const originalFetch = globalThis.fetch + // garbage heartbeat line first, then one valid chunk, then [DONE] + const body = + 'data: this is not json at all\n\n' + + 'data: {"id":"1","object":"chat.completion.chunk","choices":[{"index":0,"delta":{"content":"ok"}}]}\n\n' + + 'data: [DONE]\n\n' + globalThis.fetch = (async () => + new Response(body, { headers: { 'content-type': 'text/event-stream' } })) as typeof fetch + const llm = new LLM({ model: 'm', apiKey: 'k' }) + + try { + const { parts, resp } = await collect(llm.chat([{ role: 'user', content: 'hi' }])) + assert.deepEqual(parts, ['ok']) + assert.equal(resp.content, 'ok') + } finally { + globalThis.fetch = originalFetch + } +}) + +// ---------------------------------------------------------------- glob separators + +test('globToRegExp treats Windows backslashes as separators', () => { + assert.ok(globToRegExp('src\\**\\*.ts').test('src/a/b.ts')) + assert.ok(globToRegExp('src\\**\\*.ts').test('src/c.ts')) // **/ matches zero segments + assert.ok(!globToRegExp('src\\**\\*.ts').test('lib/a.ts')) +}) + +// ---------------------------------------------------------------- tool args + +test('read_file clamps junk offset/limit instead of slicing backwards', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'cc-read-')) + const file = path.join(dir, 'lines.txt') + await fs.writeFile(file, 'a\nb\nc\n') + const { readFileTool } = await import('../src/tools/read.js') + + const zero = await readFileTool.execute({ file_path: file, limit: 0 }) + assert.ok(zero.includes('a'), 'limit 0 still reads at least one line') + + const neg = await readFileTool.execute({ file_path: file, offset: 1, limit: -5 }) + assert.ok(neg.includes('a'), 'negative limit reads at least one line') + + const negOff = await readFileTool.execute({ file_path: file, offset: -3 }) + assert.ok(negOff.includes('a'), 'negative offset clamps to line 1') + + await fs.rm(dir, { recursive: true, force: true }) +}) + +test('grep caps match line width so one-line giants cannot flood the context', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'cc-grep-')) + const file = path.join(dir, 'big.js') + await fs.writeFile(file, 'x'.repeat(5000) + '\nshort\n') + const { grepTool } = await import('../src/tools/grep.js') + + const out = await grepTool.execute({ pattern: 'x', path: dir }) + assert.ok(out.includes('line truncated')) + assert.ok(out.length < 2500, `output should be capped, got ${out.length} chars`) + + await fs.rm(dir, { recursive: true, force: true }) +}) + +// ---------------------------------------------------------------- config env + +test('configFromEnv falls back on garbage numeric env vars', () => { + process.env.CORECODER_MAX_TOKENS = 'abc' + process.env.CORECODER_TEMPERATURE = 'hot' + process.env.CORECODER_MAX_CONTEXT = '1e999' // overflows to Infinity + try { + const cfg = configFromEnv() + assert.equal(cfg.maxTokens, 4096) + assert.equal(cfg.temperature, 0) + assert.equal(cfg.maxContextTokens, 128000) + } finally { + delete process.env.CORECODER_MAX_TOKENS + delete process.env.CORECODER_TEMPERATURE + delete process.env.CORECODER_MAX_CONTEXT + } +})