From 7e1b2d8f29e2980636d10cba2ad3152224522cb6 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 10:27:08 +0800 Subject: [PATCH 1/9] fix(ui): stop elapsed-time ticker while a permission prompt is open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 250ms nowTick interval kept firing during accept/decline/always prompts, re-painting the whole dynamic Ink frame 4x/second — with a tall diff+prompt on screen this read as violent scroll-jumping until the user answered. The ticker now goes quiet via a ref (timer and task start time stay intact, the readout just freezes), and tool-activity lines hide under the prompt. --- src/cli/ui.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/cli/ui.tsx b/src/cli/ui.tsx index e629487..9331cc1 100644 --- a/src/cli/ui.tsx +++ b/src/cli/ui.tsx @@ -138,12 +138,18 @@ export function App(props: AppProps): React.ReactElement { const [taskStartedAt, setTaskStartedAt] = useState(null); const [nowTick, setNowTick] = useState(() => Date.now()); const [lastElapsedMs, setLastElapsedMs] = useState(0); + // The ticker goes QUIET while a permission prompt is open: each tick re-renders the + // whole dynamic frame, and with a tall diff+confirmation on screen that 4Hz re-paint + // reads as violent scroll-jumping. A ref (not an effect dep) keeps the timer and the + // task's start time intact — the readout just freezes until the prompt resolves. + const pendingPromptRef = useRef(null); + pendingPromptRef.current = pendingPrompt; useEffect(() => { if (!busy) return; const start = Date.now(); setTaskStartedAt(start); setNowTick(start); - const iv = setInterval(() => setNowTick(Date.now()), 250); + const iv = setInterval(() => { if (!pendingPromptRef.current) setNowTick(Date.now()); }, 250); return () => { clearInterval(iv); setLastElapsedMs(Date.now() - start); @@ -649,7 +655,9 @@ export function App(props: AppProps): React.ReactElement { )} - {activeTools.map(t => ( + {/* Tool activity hides while a permission prompt is up — the prompt IS the activity, + and every extra dynamic line enlarges the frame Ink re-paints. */} + {!pendingPrompt && activeTools.map(t => ( ))} From 25cff36bb67f93ef06a99e889559cd98c96eb889 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 10:27:08 +0800 Subject: [PATCH 2/9] fix(tokens): count OpenAI cached prompt tokens as cacheRead + age results sooner OpenAI's prompt_tokens INCLUDES automatically-cached prefix tokens; usage now splits prompt_tokens_details.cached_tokens into cacheRead (matching Anthropic semantics), so the budget no longer re-bills the whole context at 1x every turn for OpenAI-provider models. Result aging also tightens (3 turns/8KB -> 2 turns/5KB) so large unique tool outputs stop riding the context full-size. --- src/agent/result-aging.ts | 4 ++-- src/llm/providers/openai.ts | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/agent/result-aging.ts b/src/agent/result-aging.ts index 2394e7b..0273163 100644 --- a/src/agent/result-aging.ts +++ b/src/agent/result-aging.ts @@ -65,8 +65,8 @@ export interface AgingResult { } export function ageToolResults(messages: Message[], opts: AgingOptions = {}): AgingResult { - const minAgeTurns = opts.minAgeTurns ?? 3; - const maxChars = opts.maxChars ?? 8_000; + const minAgeTurns = opts.minAgeTurns ?? 2; + const maxChars = opts.maxChars ?? 5_000; const keepHead = opts.keepHead ?? 1_500; const keepTail = opts.keepTail ?? 2_500; diff --git a/src/llm/providers/openai.ts b/src/llm/providers/openai.ts index 5dde01d..3d9ca8b 100644 --- a/src/llm/providers/openai.ts +++ b/src/llm/providers/openai.ts @@ -241,9 +241,21 @@ export class OpenAIProvider extends Provider { } if (chunk.usage) { + // OpenAI's prompt_tokens INCLUDES server-cached tokens (prompt caching is + // automatic on a byte-stable prefix, billed at a discount and reported in + // prompt_tokens_details.cached_tokens). Splitting them out keeps usage + // semantics aligned with the Anthropic provider — `input` is FRESH tokens + // only, `cacheRead` the cached remainder. Without the split, every turn + // re-counted the whole context at 1× and long sessions looked like they + // burned a 200k budget in minutes. + const cached = (chunk.usage as any).prompt_tokens_details?.cached_tokens ?? 0; yield { type: 'usage', - usage: { input: chunk.usage.prompt_tokens, output: chunk.usage.completion_tokens }, + usage: { + input: Math.max(0, chunk.usage.prompt_tokens - cached), + output: chunk.usage.completion_tokens, + cacheRead: cached, + }, }; } } From ddbfc87e274384eef07cf74b82938ff748259a11 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 16:20:20 +0800 Subject: [PATCH 3/9] fix(tokens): count the token budget in NOVEL tokens, not per-call context re-reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agentic turn makes one API call per tool round and each call re-sends the whole conversation, so charging every call's full input made budget spend grow quadratically with tool rounds — a 21k-context task on a local model (no cache fields at all) burned 216k/200k within two minutes and died. A prompt high-water mark now counts each context token ONCE: consume = output + max(0, seen - mark). Dollar cost stays full-usage per call — bills are bills; the token budget is a runaway guard, not an invoice. --- src/agent/loop.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 8a6b843..137f3a2 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -1158,6 +1158,15 @@ export class AgentLoop { } catch { /* best-effort — never block the task on baseline capture */ } } + // Token-budget accounting is NOVEL-tokens-only. An agentic turn makes one API call per + // tool round, and every call re-sends the whole conversation — so counting each call's + // full input against the budget grows QUADRATICALLY with tool rounds (live: a 21k-context + // task on a local model "spent" 216k/200k within two minutes and died). The high-water + // mark counts every context token ONCE: consume = output + max(0, seenNow - seenBefore). + // Dollar cost stays full-usage per call below — bills are bills; the token budget is a + // runaway guard, not an invoice. + let promptHighWater = 0; + while (true) { try { budget.incrementIteration(); @@ -1580,13 +1589,18 @@ export class AgentLoop { return; } - // Track budget + // Track budget — novel tokens only (see promptHighWater above): the re-sent + // conversation prefix is counted the FIRST time it enters the context, not on + // every subsequent tool round. Works for every provider, including local ones + // that report no cache fields at all. const cost = computeCost(lastUsage, route.modelInfo); - budget.consume({ tokens: lastUsage.input + lastUsage.output, costUsd: cost }); - // Cache hit-rate: cached reads ÷ total input the model saw (fresh + cached). Lets the - // status line PROVE the hierarchical cache is working (and how much it's saving). const cacheRead = (lastUsage as any).cacheRead ?? 0; const totalInputSeen = lastUsage.input + cacheRead; + const freshInput = Math.max(0, totalInputSeen - promptHighWater); + promptHighWater = Math.max(promptHighWater, totalInputSeen); + budget.consume({ tokens: freshInput + lastUsage.output, costUsd: cost }); + // Cache hit-rate: cached reads ÷ total input the model saw (fresh + cached). Lets the + // status line PROVE the hierarchical cache is working (and how much it's saving). const cacheHitRate = totalInputSeen > 0 ? cacheRead / totalInputSeen : 0; yield { type: 'budget_update', From 2be07073847095d7f17cbf9a8522efb0884cc286 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 16:52:14 +0800 Subject: [PATCH 4/9] feat(update): detect available updates in terminal + dashboard, not just apply blindly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qodex already had 'qodex update' and a dashboard button that ran git pull → npm install → build. Both fired blind — no way to know an update EXISTED. Adds checkForUpdate(): a no-merge 'git fetch' + behind-count against the upstream. - terminal: 'qodex update --check' reports availability without applying - dashboard: Health panel now shows 'QodeX vX.Y.Z', an 'update available — N behind' badge (instant local-only check on render), a 'Check for updates' button (networked, app.checkUpdate), and the existing update button Tests build a real local remote→clone pair (no network) to prove the behind count and the fetchRemote:false fast path. --- src/cli/dashboard-control.ts | 5 +++ src/cli/dashboard.ts | 28 +++++++++++++-- src/cli/self-update.ts | 68 ++++++++++++++++++++++++++++++++++++ src/index.ts | 9 ++++- test/self-update.test.ts | 55 ++++++++++++++++++++++++++++- 5 files changed, 161 insertions(+), 4 deletions(-) diff --git a/src/cli/dashboard-control.ts b/src/cli/dashboard-control.ts index c66f804..d4e164a 100644 --- a/src/cli/dashboard-control.ts +++ b/src/cli/dashboard-control.ts @@ -245,6 +245,11 @@ export async function dispatchAction(name: string, params: any, cwd: string): Pr const eg = p.sample.slice(0, 3).map(c => c.name).join(', '); return { ok: true, message: `🔍 ${p.count} unused symbol(s) maintain could clean (e.g. ${eg}). Schedule \`unused-imports\` / \`unused-locals\`.` }; } + case 'app.checkUpdate': { + const { checkForUpdate } = await import('./self-update.js'); + const s = await checkForUpdate(); // networked fetch, on demand + return { ok: s.ok, message: s.message }; + } case 'app.update': { const { selfUpdate } = await import('./self-update.js'); const r = await selfUpdate(); diff --git a/src/cli/dashboard.ts b/src/cli/dashboard.ts index c7b3695..78bf7cb 100644 --- a/src/cli/dashboard.ts +++ b/src/cli/dashboard.ts @@ -35,6 +35,8 @@ export interface DashboardData { extractMetrics?: import('../tools/web/extract-metrics.js').ExtractCounts; roleModels?: { subagent?: string; vision?: string; mainHasVision: boolean }; totals: { sessions: number; tokens: number; cost: number; facts: number; episodes: number; skills: number }; + /** Version + whether the git checkout is behind its remote (populated best-effort). */ + update?: { version?: string; updateAvailable: boolean; behind: number; message: string; ok: boolean }; } const esc = (s: string) => String(s).replace(/[&<>"]/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"' }[c]!)); @@ -119,7 +121,20 @@ export function buildDashboardHtml(d: DashboardData, opts: { token?: string } = // Observability: health badges + a tail of the local log. const healthBadges = d.health.map(h => `${h.ok ? '✓' : '!'} ${esc(h.label)}: ${esc(h.detail)}`).join(''); - const healthPanel = `

Health

${live ? `` : ''}
${healthBadges}
`; + // Version + update status: current version, an "update available" badge when the local + // check knows we're behind, a "Check for updates" button (networked), and the update button. + const up = d.update; + const verLabel = up?.version ? `v${esc(up.version)}` : 'version unknown'; + const updateBadge = up?.updateAvailable + ? `⬆ update available — ${up.behind} behind` + : up?.ok ? `✓ up to date` : ''; + const updateCtl = live + ? `` + + `` + : ''; + const healthPanel = `
` + + `

Health · QodeX ${verLabel} ${updateBadge}

` + + `
${updateCtl}
${healthBadges}
`; const logsPanel = d.logs.length ? `

Recent log

${d.logs.map(esc).join('\n')}
` : ''; // Recall explorer — ask "how did we do X before?" right here: best match + how other attempts // differed (the same visual diff recall_approach returns), rendered in-place without a reload. @@ -444,13 +459,22 @@ export async function gatherDashboardData(cwd: string): Promise { try { const { readExtractMetrics } = await import('../tools/web/extract-metrics.js'); return await readExtractMetrics(); } catch { return undefined; } })(); + // Version + update status — LOCAL only (no network) so the render stays instant; the + // "Check for updates" button runs the networked fetch on demand. + const update = await (async () => { + try { + const { checkForUpdate } = await import('./self-update.js'); + const s = await checkForUpdate({ fetchRemote: false }); + return { version: s.version, updateAvailable: s.updateAvailable, behind: s.behind, message: s.message, ok: s.ok }; + } catch { return undefined; } + })(); return { project, model: defModel, generatedAt: new Date().toISOString().slice(0, 16).replace('T', ' '), providers, sessions, facts, episodes, skills, controls, schedules, models, candidates, runs, bot, health, logs, userModel, maintainStats, maintainWeekly: maintain?.weekly, maintainNext: maintain?.next ?? undefined, maintainTrend: maintain?.trend, maintainProjection: maintain?.projection, maintainForecast: maintain?.forecast, - extractMetrics, roleModels, + extractMetrics, roleModels, update, totals: { sessions: sessions.length, tokens: sessions.reduce((a, s) => a + s.tokens, 0), cost: sessions.reduce((a, s) => a + s.cost, 0), facts: facts.length, episodes: episodes.length, skills: skills.length, diff --git a/src/cli/self-update.ts b/src/cli/self-update.ts index 2c3d502..6fe7e29 100644 --- a/src/cli/self-update.ts +++ b/src/cli/self-update.ts @@ -41,6 +41,74 @@ export async function findRepoRoot(startDir: string): Promise { export interface UpdateResult { ok: boolean; message: string; log: string[]; root?: string } +export interface UpdateStatus { + /** A QodeX git checkout was found and its remote could be queried. */ + ok: boolean; + /** True when the remote tracking branch is ahead of local HEAD. */ + updateAvailable: boolean; + /** How many commits local HEAD is behind the remote (0 when up to date). */ + behind: number; + /** Short local HEAD sha, and the remote's, for display. */ + local?: string; + remote?: string; + /** package.json version string (unchanged until the update actually builds). */ + version?: string; + /** Human summary for the CLI / dashboard. */ + message: string; + root?: string; +} + +/** Best-effort `git` reader that never throws — returns trimmed stdout or ''. */ +function git(root: string, args: string[], timeoutMs = 20_000): string { + const r = spawnSync('git', args, { cwd: root, encoding: 'utf-8', timeout: timeoutMs }); + return r.status === 0 ? (r.stdout ?? '').trim() : ''; +} + +/** + * Check whether newer commits exist on the remote WITHOUT touching the working tree. + * A `git fetch` updates remote-tracking refs (no merge, no checkout), then we count how + * many commits HEAD is behind its upstream. This is what powers "update available" in the + * dashboard badge and `qodex update --check` — the pull itself stays a separate, explicit step. + */ +export async function checkForUpdate(opts: { fetchRemote?: boolean } = {}): Promise { + const fetchRemote = opts.fetchRemote !== false; // default true; false = instant local-only (dashboard render) + const here = path.dirname(fileURLToPath(import.meta.url)); + const root = process.env.QODEX_SRC_DIR ?? (await findRepoRoot(here)); + if (!root) { + return { ok: false, updateAvailable: false, behind: 0, message: 'Not a git checkout — reinstall with install.sh (or set QODEX_SRC_DIR).' }; + } + let version: string | undefined; + try { version = JSON.parse(await fs.readFile(path.join(root, 'package.json'), 'utf-8'))?.version; } catch { /* keep undefined */ } + + // Resolve the upstream ref; fall back to origin/ when no @{u} is set. + const branch = git(root, ['rev-parse', '--abbrev-ref', 'HEAD']) || 'HEAD'; + const upstream = git(root, ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}']) || (branch !== 'HEAD' ? `origin/${branch}` : ''); + if (!upstream) { + return { ok: false, updateAvailable: false, behind: 0, version, root, message: 'No upstream tracking branch — can\'t check for updates.' }; + } + // Network step: refresh remote-tracking refs only (no merge/checkout). Skipped for the + // dashboard's instant render, which reports against whatever the last fetch already knew. + if (fetchRemote) { + const fetch = spawnSync('git', ['fetch', '--quiet', 'origin'], { cwd: root, encoding: 'utf-8', timeout: 45_000 }); + if (fetch.status !== 0) { + return { ok: false, updateAvailable: false, behind: 0, version, root, message: `Couldn't reach the remote (${(fetch.stderr ?? '').trim().slice(-120) || 'offline?'}).` }; + } + } + const local = git(root, ['rev-parse', '--short', 'HEAD']); + const remote = git(root, ['rev-parse', '--short', upstream]); + const behindStr = git(root, ['rev-list', '--count', `HEAD..${upstream}`]); + const behind = Number.parseInt(behindStr, 10) || 0; + return { + ok: true, + updateAvailable: behind > 0, + behind, + local, remote, version, root, + message: behind > 0 + ? `Update available — ${behind} commit${behind === 1 ? '' : 's'} behind ${upstream} (${local} → ${remote}). Run \`qodex update\`.` + : `${fetchRemote ? 'Up to date' : 'Up to date (local check)'} with ${upstream} (${local}).`, + }; +} + /** Run the update pipeline in the QodeX checkout. `onLog` streams progress (for the CLI). */ export async function selfUpdate(onLog: (line: string) => void = () => {}): Promise { const here = path.dirname(fileURLToPath(import.meta.url)); // dist/cli or src/cli diff --git a/src/index.ts b/src/index.ts index 41b5be8..f4e61de 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1003,7 +1003,14 @@ program program .command('update') .description('Self-update the QodeX git checkout (git pull → npm install → npm run build)') - .action(async () => { + .option('--check', 'Only check whether a newer version is available; don\'t apply it') + .action(async (opts: { check?: boolean }) => { + if (opts.check) { + const { checkForUpdate } = await import('./cli/self-update.js'); + const s = await checkForUpdate(); + console.log(`\n${s.updateAvailable ? '⬆' : s.ok ? '✓' : '✗'} ${s.message}\n`); + process.exit(s.ok ? 0 : 1); + } const { selfUpdate } = await import('./cli/self-update.js'); console.log('\n🔄 Updating QodeX…'); const r = await selfUpdate(line => console.log(' ' + line)); diff --git a/test/self-update.test.ts b/test/self-update.test.ts index e217ccf..6c35206 100644 --- a/test/self-update.test.ts +++ b/test/self-update.test.ts @@ -2,7 +2,8 @@ import { describe, it, expect } from 'vitest'; import { promises as fs } from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { findRepoRoot, UPDATE_STEPS } from '../src/cli/self-update.ts'; +import { execFileSync } from 'child_process'; +import { findRepoRoot, UPDATE_STEPS, checkForUpdate } from '../src/cli/self-update.ts'; describe('findRepoRoot', () => { it('walks up to the QodeX checkout (package.json name + .git)', async () => { @@ -31,3 +32,55 @@ describe('findRepoRoot', () => { expect(UPDATE_STEPS[0]!.args).toContain('--ff-only'); // never a merge-commit surprise }); }); + +describe('checkForUpdate', () => { + // Build a tiny local "remote → clone" pair so the fetch/behind-count runs against real git + // without any network. The clone starts one commit behind, so behind === 1. + async function makeBehindClone(): Promise<{ dir: string; clone: string }> { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'upd-git-')); + const remote = path.join(dir, 'remote'); + const g = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, stdio: 'pipe' }); + await fs.mkdir(remote, { recursive: true }); + g(remote, 'init', '-q', '-b', 'main'); + g(remote, 'config', 'user.email', 't@t.co'); g(remote, 'config', 'user.name', 't'); + await fs.writeFile(path.join(remote, 'package.json'), JSON.stringify({ name: '@qodex/cli', version: '2.5.0' })); + g(remote, 'add', '-A'); g(remote, 'commit', '-q', '-m', 'v1'); + const clone = path.join(dir, 'clone'); + g(dir, 'clone', '-q', remote, clone); + await fs.mkdir(path.join(clone, '.git'), { recursive: true }); // findRepoRoot marker (already present) + // advance the remote by one commit so the clone is 1 behind + await fs.writeFile(path.join(remote, 'x.txt'), 'hi'); + g(remote, 'add', '-A'); g(remote, 'commit', '-q', '-m', 'v2'); + return { dir, clone }; + } + + it('reports updateAvailable + behind count against a real upstream (no network)', async () => { + const { dir, clone } = await makeBehindClone(); + const prev = process.env.QODEX_SRC_DIR; + process.env.QODEX_SRC_DIR = clone; + try { + const s = await checkForUpdate(); // fetches from the local file:// remote + expect(s.ok).toBe(true); + expect(s.updateAvailable).toBe(true); + expect(s.behind).toBe(1); + expect(s.version).toBe('2.5.0'); + } finally { + if (prev === undefined) delete process.env.QODEX_SRC_DIR; else process.env.QODEX_SRC_DIR = prev; + await fs.rm(dir, { recursive: true, force: true }).catch(() => {}); + } + }); + + it('fetchRemote:false skips the network step and still reads the local state', async () => { + const { dir, clone } = await makeBehindClone(); + const prev = process.env.QODEX_SRC_DIR; + process.env.QODEX_SRC_DIR = clone; + try { + const s = await checkForUpdate({ fetchRemote: false }); // no fetch → clone hasn't seen v2 yet + expect(s.ok).toBe(true); + expect(s.behind).toBe(0); // local-only view: still even with known origin/main + } finally { + if (prev === undefined) delete process.env.QODEX_SRC_DIR; else process.env.QODEX_SRC_DIR = prev; + await fs.rm(dir, { recursive: true, force: true }).catch(() => {}); + } + }); +}); From fa9df2d1fcf07e0f10426309a234af7a2f6bd15c Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 17:09:45 +0800 Subject: [PATCH 5/9] =?UTF-8?q?fix(budget):=20make=20the=20wall-time=20cei?= =?UTF-8?q?ling=20stall-aware=20=E2=80=94=20stop=20killing=20tasks=20mid-w?= =?UTF-8?q?ork?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkpoint() runs right after a completed model call, so the old absolute ceiling could ONLY ever kill a task that was actively working (live: 'Time budget exceeded: 608s/600s' during an active edit loop on a local model) — a hung task never reaches a checkpoint at all. The ceiling now fires only when the task is ALSO stalled (no completed call in the last 2 minutes); true runaways stay bounded by the iteration and token caps. Default ceiling raised 600s -> 3600s: it was calibrated for cloud latency, and a local model legitimately spends 10 minutes on a handful of long generations. --- src/agent/budget.ts | 16 +++++++++++++++- src/config/defaults.ts | 5 ++++- test/budget.test.ts | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 test/budget.test.ts diff --git a/src/agent/budget.ts b/src/agent/budget.ts index 71e9fa6..4c7c77c 100644 --- a/src/agent/budget.ts +++ b/src/agent/budget.ts @@ -14,6 +14,9 @@ export class BudgetTracker { private costUsd = 0; private iterations = 0; private iterationWarned = false; + /** Bumped on every consume() (= a completed model call). Slow ≠ runaway: the wall-time + * ceiling only fires when the task is ALSO stalled, judged against this timestamp. */ + private lastProgressAt = Date.now(); constructor( private maxTokens: number, @@ -34,6 +37,7 @@ export class BudgetTracker { consume(usage: { tokens?: number; costUsd?: number }): void { this.tokens += usage.tokens ?? 0; this.costUsd += usage.costUsd ?? 0; + this.lastProgressAt = Date.now(); } incrementIteration(): void { @@ -52,7 +56,17 @@ export class BudgetTracker { throw new BudgetExceededError(`Cost budget exceeded: $${this.costUsd.toFixed(4)}/$${this.maxCostUsd}`, 'cost'); } if (this.maxWallSeconds > 0 && wallMs > this.maxWallSeconds * 1000) { - throw new BudgetExceededError(`Time budget exceeded: ${(wallMs / 1000).toFixed(0)}s/${this.maxWallSeconds}s`, 'time'); + // Slow ≠ runaway. checkpoint() runs right AFTER a completed model call, so in the + // old form this ceiling could ONLY ever kill a task that was actively working + // (live: "Time budget exceeded: 608s/600s" mid-edit on a local model) — a hung + // task never reaches a checkpoint at all. The ceiling now fires only when the + // task is ALSO stalled (no completed call in the last 2 minutes); true runaways + // stay bounded by the iteration and token caps. + const idleMs = Date.now() - this.lastProgressAt; + if (idleMs > 120_000) { + throw new BudgetExceededError( + `Time budget exceeded: ${(wallMs / 1000).toFixed(0)}s/${this.maxWallSeconds}s (stalled ${(idleMs / 1000).toFixed(0)}s)`, 'time'); + } } if (this.maxIterations > 0 && this.iterations > this.maxIterations) { throw new BudgetExceededError(`Iteration budget exceeded: ${this.iterations}/${this.maxIterations}`, 'iterations'); diff --git a/src/config/defaults.ts b/src/config/defaults.ts index 11812cd..fd57b78 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -623,7 +623,10 @@ export const DEFAULT_CONFIG: QodexConfig = { dailyLimitUsd: 10.0, perTaskLimitUsd: 1.0, perTaskMaxTokens: 200_000, - perTaskMaxWallSeconds: 600, + // Ceiling, not pace-setter: since the stall-aware checkpoint (budget.ts) it only fires + // when the task ALSO stopped progressing. 600s was calibrated for cloud latency; a local + // model legitimately spends that on a handful of long generations. + perTaskMaxWallSeconds: 3600, toolTimeoutSeconds: 300, }, security: { diff --git a/test/budget.test.ts b/test/budget.test.ts new file mode 100644 index 0000000..129941f --- /dev/null +++ b/test/budget.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { BudgetTracker } from '../src/agent/budget.ts'; + +/** + * The wall-time ceiling is stall-aware: checkpoint() runs right after a completed model + * call, so a plain ceiling could only ever kill tasks that were actively working (live: + * "Time budget exceeded: 608s/600s" mid-edit on a local model). These tests pin the fix: + * over the ceiling + recent progress = keep going; over the ceiling + stalled = stop. + */ +describe('BudgetTracker wall-time', () => { + afterEach(() => vi.useRealTimers()); + + it('does NOT kill an actively-progressing task past the ceiling', () => { + vi.useFakeTimers(); + const b = new BudgetTracker(0, 0, 600, 0); // 600s ceiling, no other limits + vi.advanceTimersByTime(610_000); // 610s elapsed… + b.consume({ tokens: 100 }); // …but a call JUST completed + expect(() => b.checkpoint()).not.toThrow(); + }); + + it('kills a task past the ceiling once it also stalls (>2min without progress)', () => { + vi.useFakeTimers(); + const b = new BudgetTracker(0, 0, 600, 0); + b.consume({ tokens: 100 }); + vi.advanceTimersByTime(610_000); // 610s elapsed, no progress since t=0 + expect(() => b.checkpoint()).toThrow(/Time budget exceeded/); + }); + + it('under the ceiling nothing fires regardless of stall', () => { + vi.useFakeTimers(); + const b = new BudgetTracker(0, 0, 600, 0); + vi.advanceTimersByTime(500_000); + expect(() => b.checkpoint()).not.toThrow(); + }); + + it('token cap still enforces independently', () => { + const b = new BudgetTracker(1000, 0, 0, 0); + b.consume({ tokens: 1500 }); + expect(() => b.checkpoint()).toThrow(/Token budget exceeded/); + }); +}); From ffd60df4713c4be85bd048f52648d4ca42a3ec22 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 17:21:30 +0800 Subject: [PATCH 6/9] feat(status): show the model's real source + live context window in the status bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bar said 'local' whenever cost was zero (a guess) and rendered the STATIC context window from the model table even though loop.ts already live-syncs the real window from LM Studio's native API — the detection existed, unwired. Now budget_update carries providerName/providerIsLocal and the live-synced window: - source badge next to the model: 'ollama·local', 'lmstudio·local', 'anthropic·api' … (LM Studio detected via its /api/v0/models registry, which its OpenAI-compat facade hides) - cloud at zero accrued cost shows 'api · $0.0000', never 'local · free' — billing surfaces the moment real cost accrues - context meter uses the live window, falling back to the static table --- src/agent/loop.ts | 25 +++++++++++++++++++++---- src/cli/ui.tsx | 22 +++++++++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 137f3a2..4995e95 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -123,6 +123,10 @@ export class AgentLoop { * ADDS to this — never drops — so the tools block stays a byte-stable cache prefix * across turns (a sliding per-turn set would flip and invalidate the prompt cache). */ private sessionToolNames = new Set(); + /** Where the active model is actually served from ('ollama'/'lmstudio'/'anthropic'/…) and + * its LIVE context window — set each iteration, forwarded on budget_update for the UI. */ + private lastModelSource = ''; + private lastEffectiveCtxWindow = 0; private totalToolCalls = 0; // running count of executed tool calls (skill-capture eligibility) private currentTaskKey = ''; // stable key for THIS run's task (failure-driven learning) // Ground-truth verification ledger: every checker QodeX actually ran this run + its result. @@ -1410,15 +1414,24 @@ export class AgentLoop { } catch { this.liveCtxWindows = null; } } let effectiveCtxWindow = route.modelInfo.contextWindow; + // Where the model is actually served from, for the status bar. Provider name is the + // base truth (ollama/anthropic/openai/custom name); a hit in LM Studio's native model + // registry upgrades it to 'lmstudio' — the OpenAI-compat facade otherwise hides that. + let modelSource = route.provider.name; if (this.liveCtxWindows) { const id = route.model; const live = this.liveCtxWindows[id] ?? this.liveCtxWindows[Object.keys(this.liveCtxWindows).find(k => k.includes(id) || id.includes(k)) ?? '']; - if (typeof live === 'number' && live > 0 && live !== effectiveCtxWindow) { - logger.info('Context window synced from LM Studio', { model: id, config: effectiveCtxWindow, live }); - effectiveCtxWindow = live; + if (typeof live === 'number' && live > 0) { + modelSource = 'lmstudio'; + if (live !== effectiveCtxWindow) { + logger.info('Context window synced from LM Studio', { model: id, config: effectiveCtxWindow, live }); + effectiveCtxWindow = live; + } } } + this.lastModelSource = modelSource; + this.lastEffectiveCtxWindow = effectiveCtxWindow; const contextBudget = Math.floor(effectiveCtxWindow * 0.75); const estTokens = this.estimateTokens(agedRaw); // ─── Hooks: PreCompact ─────────────────────────────────────────────────── @@ -1608,7 +1621,11 @@ export class AgentLoop { ...budget.getUsage(), lastInputTokens: lastUsage.input, lastOutputTokens: lastUsage.output, lastCostUsd: cost, lastCacheRead: cacheRead, lastCacheCreation: (lastUsage as any).cacheCreation ?? 0, cacheHitRate, - contextWindow: route.modelInfo.contextWindow, + // The LIVE-synced window (LM Studio native API when available), not the static + // table value — plus where the model is actually served from and whether it bills. + contextWindow: this.lastEffectiveCtxWindow || route.modelInfo.contextWindow, + providerName: this.lastModelSource || route.provider.name, + providerIsLocal: (route.provider as any).isLocal === true || this.lastModelSource === 'lmstudio', }, }; diff --git a/src/cli/ui.tsx b/src/cli/ui.tsx index 9331cc1..73b67aa 100644 --- a/src/cli/ui.tsx +++ b/src/cli/ui.tsx @@ -130,7 +130,7 @@ export function App(props: AppProps): React.ReactElement { // finishes (or immediately when motion is disabled / not a TTY). const [booted, setBooted] = useState(false); const [explicitModel, setExplicitModel] = useState(props.explicitModel); - const [budgetStatus, setBudgetStatus] = useState({ tokens: 0, costUsd: 0, contextTokens: 0, contextWindow: 0 }); + const [budgetStatus, setBudgetStatus] = useState({ tokens: 0, costUsd: 0, contextTokens: 0, contextWindow: 0, providerName: '', providerIsLocal: true }); // Live throughput + elapsed readout for the status bar. taskStartedAt marks when // the current task began (busy → true); nowTick is bumped by an interval while // busy so the readout refreshes; lastElapsedMs freezes the finished task's total @@ -528,6 +528,8 @@ export function App(props: AppProps): React.ReactElement { costUsd: event.data.costUsd, contextTokens: event.data.lastInputTokens ?? 0, contextWindow: event.data.contextWindow ?? 0, + providerName: event.data.providerName ?? '', + providerIsLocal: event.data.providerIsLocal !== false, }); break; case 'final': @@ -732,6 +734,8 @@ export function App(props: AppProps): React.ReactElement { costUsd={budgetStatus.costUsd} contextTokens={budgetStatus.contextTokens} contextWindow={budgetStatus.contextWindow} + providerName={budgetStatus.providerName} + providerIsLocal={budgetStatus.providerIsLocal} elapsedMs={busy && taskStartedAt ? Math.max(0, nowTick - taskStartedAt) : lastElapsedMs} busy={busy} /> @@ -792,12 +796,18 @@ function StatusBar(props: { costUsd: number; contextTokens: number; contextWindow: number; + providerName?: string; + providerIsLocal?: boolean; elapsedMs: number; busy: boolean; }): React.ReactElement { - const { width, model, mode, tokens, costUsd, contextTokens, contextWindow, elapsedMs, busy } = props; + const { width, model, mode, tokens, costUsd, contextTokens, contextWindow, providerName, providerIsLocal, elapsedMs, busy } = props; const tok = tokens >= 1000 ? `${(tokens / 1000).toFixed(1)}k` : String(tokens); - const credit = costUsd > 0 ? `$${costUsd.toFixed(4)}` : 'local · free'; + // WHERE the model runs, stated — not guessed from cost: 'ollama·local', 'lmstudio·local', + // 'anthropic·api'… A billing API at $0.0000 still shows ·api so cloud is never mistaken + // for free-local; the dollar figure appears the moment real cost accrues. + const source = providerName ? `${providerName}·${providerIsLocal ? 'local' : 'api'}` : ''; + const credit = costUsd > 0 ? `$${costUsd.toFixed(4)}` : providerIsLocal === false ? 'api · $0.0000' : 'local · free'; // Throughput (token-consumption rate) + elapsed time. Average over the task — // total tokens / elapsed — which is exactly "how fast tokens are being spent". const secs = elapsedMs / 1000; @@ -814,6 +824,12 @@ function StatusBar(props: { {model} + {source !== '' && ( + <> + · + {source} + + )} · {mode} {ctxMeter !== '' && ( From a9796c207166a983273f62737abfa3f652ad5005 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 17:32:08 +0800 Subject: [PATCH 7/9] =?UTF-8?q?fix(subagent):=20create=20the=20child=20ses?= =?UTF-8?q?sion=20row=20before=20the=20sub-agent's=20first=20write=20?= =?UTF-8?q?=E2=80=94=20fabricated=20sub-session=20ids=20(parent/sub-)?= =?UTF-8?q?=20had=20no=20parent=20row=20in=20sessions,=20so=20the=20messag?= =?UTF-8?q?es=20FK=20failed=20instantly=20and=20every=20delegation=20died?= =?UTF-8?q?=20with=20SUBAGENT=5FFAILED?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/agent/loop.ts | 7 ++ src/session/store.ts | 13 ++++ test/subagent-session-fk.test.ts | 113 +++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 test/subagent-session-fk.test.ts diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 4995e95..902ffae 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -307,6 +307,13 @@ export class AgentLoop { sessionId: opts.sessionId, }); + // The child session id is fabricated by the dispatcher (`/sub-`, + // `/fanout-`, …) and has no row in `sessions` yet — but + // messages.session_id carries a FK to sessions.id, so the sub-agent's FIRST + // recordTurn would fail with "FOREIGN KEY constraint failed", killing every + // delegation instantly. Create the parent row up front (idempotent). + getSessionStore().ensureSession(opts.sessionId, this.cwd, modelUsed); + // Role-specific tool restriction (allow-list). Built-in policy: // - vision role: only vision_analyze + read-only browser/file/web tools // - subagent role: everything except `task` (no recursion) — handled by mode=subagent diff --git a/src/session/store.ts b/src/session/store.ts index 3e41235..28ba1ef 100644 --- a/src/session/store.ts +++ b/src/session/store.ts @@ -200,6 +200,19 @@ export class SessionStore { return id; } + /** + * Create a session row with a caller-chosen id if it doesn't exist yet (idempotent). + * Used for sub-agent sessions, whose ids are derived from the parent's + * (`/sub-`, `/fanout-`, …): messages.session_id has a FK to + * sessions.id, so the row MUST exist before the sub-agent's first recordTurn — + * otherwise every child write dies with "FOREIGN KEY constraint failed". + */ + ensureSession(id: string, cwd: string, model: string): void { + this.db.prepare( + `INSERT OR IGNORE INTO sessions (id, cwd, model, title) VALUES (?, ?, ?, ?)`, + ).run(id, cwd, model, null); + } + recordTurn( sessionId: string, messages: Message[], diff --git a/test/subagent-session-fk.test.ts b/test/subagent-session-fk.test.ts new file mode 100644 index 0000000..5edac3b --- /dev/null +++ b/test/subagent-session-fk.test.ts @@ -0,0 +1,113 @@ +/** + * Regression test: dispatching a sub-agent must not die with + * "FOREIGN KEY constraint failed" on its first session-store write. + * + * The bug: dispatchers (task/fanout/gather) fabricate a child session id + * (`/sub-`) that was never INSERTed into `sessions`, while + * messages.session_id carries a FK to sessions.id (and foreign_keys=ON). + * The sub-agent's first recordTurn therefore threw, so delegation NEVER worked. + * + * Fix: AgentLoop.runSubagent calls SessionStore.ensureSession(id, cwd, model) + * before running the child loop. + */ +import { describe, it, expect, vi } from 'vitest'; +import * as os from 'os'; +import * as path from 'path'; +import * as fs from 'fs'; +import { SessionStore, getSessionStore, type Message } from '../src/session/store.js'; +import { AgentLoop } from '../src/agent/loop.js'; + +// Redirect the store singleton to a temp DB so AgentLoop.runSubagent (which uses +// getSessionStore()) never touches the user's real ~/.qodex/sessions.db. +vi.mock('../src/session/store.js', async (importOriginal) => { + const mod: any = await importOriginal(); + const os = await import('os'); + const path = await import('path'); + const fs = await import('fs'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'qodex-subagent-fk-')); + let store: any = null; + return { + ...mod, + getSessionStore: () => (store ??= new mod.SessionStore(path.join(dir, 'sessions.db'))), + }; +}); + +function freshStore(name: string): SessionStore { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `qodex-${name}-`)); + return new SessionStore(path.join(dir, 'sessions.db')); +} + +const assistantMsg: Message = { role: 'assistant', content: 'sub-agent says hi' }; + +describe('SessionStore.ensureSession — sub-agent session FK', () => { + it('documents the bug shape: first write under an unknown session id violates the FK', () => { + const store = freshStore('fk-shape'); + expect(() => + store.recordTurn('parent-abc/sub-1751700000000', [assistantMsg], { input: 1, output: 1, costUsd: 0 }), + ).toThrow(/FOREIGN KEY/i); + }); + + it('ensureSession creates the parent row so the child\'s first write succeeds', () => { + const store = freshStore('fk-fixed'); + const subId = 'parent-abc/sub-1751700000000'; + store.ensureSession(subId, '/tmp/project', 'ollama/glm-5.2'); + expect(() => + store.recordTurn(subId, [assistantMsg], { input: 1, output: 1, costUsd: 0 }), + ).not.toThrow(); + const loaded = store.loadSession(subId); + expect(loaded).not.toBeNull(); + expect(loaded!.messages).toHaveLength(1); + expect(loaded!.meta.model).toBe('ollama/glm-5.2'); + }); + + it('is idempotent and never clobbers an existing session row', () => { + const store = freshStore('fk-idempotent'); + const id = store.createSession('/tmp/project', 'model-a'); + store.recordTurn(id, [{ role: 'user', content: 'hi' }], { input: 5, output: 0, costUsd: 0 }); + store.ensureSession(id, '/somewhere/else', 'model-b'); // must be a no-op + const loaded = store.loadSession(id)!; + expect(loaded.meta.model).toBe('model-a'); + expect(loaded.meta.cwd).toBe('/tmp/project'); + expect(loaded.messages).toHaveLength(1); + }); +}); + +describe('AgentLoop.runSubagent — creates the child session row before the first write', () => { + it('the child loop\'s first recordTurn succeeds (no FK error)', async () => { + const agent: any = new AgentLoop({ + router: { resolveModel: () => null } as any, + registry: { list: () => [] } as any, + permissions: {} as any, + config: { defaults: { provider: 'ollama', model: 'glm-5.2' } } as any, + cwd: '/tmp/project', + }); + + // Mock the LLM side entirely: the child run persists ONE assistant message + // (exactly what the real loop does on its first turn) then finishes. + agent.buildInitialMessages = async (prompt: string): Promise => [ + { role: 'system', content: 'sub-agent system prompt' }, + { role: 'user', content: prompt }, + ]; + agent.run = async function* (_messages: Message[], sessionId: string) { + getSessionStore().recordTurn(sessionId, [assistantMsg], { input: 3, output: 2, costUsd: 0 }); + yield { type: 'final', data: { content: 'done' } }; + }; + + const subSessionId = 'parent-session/sub-1751700000001'; + const result = await agent.runSubagent('do a focused thing', { + maxIterations: 3, + sessionId: subSessionId, + }); + + expect(result.error).toBeUndefined(); + expect(result.ok).toBe(true); + expect(result.finalText).toBe('done'); + + // The child session row was created up front and its first message landed. + const loaded = getSessionStore().loadSession(subSessionId); + expect(loaded).not.toBeNull(); + expect(loaded!.meta.cwd).toBe('/tmp/project'); + expect(loaded!.meta.model).toBe('ollama/glm-5.2'); + expect(loaded!.messages).toHaveLength(1); + }); +}); From 24c70954f296e14e668f02a83dc321f3c7e1e20e Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 17:57:31 +0800 Subject: [PATCH 8/9] fix(budget): completed tool calls count as progress for the stall-aware ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 5-minute shell command is work, not a stall — but only consume() (a model call) bumped lastProgressAt, so the wall ceiling read long tool runs as 'stalled' and killed the task right after the tool returned (live: 'Time budget exceeded: 808s/600s (stalled 300s)' immediately after a 300s shell). noteProgress() now fires when tool executions complete, at all three dispatch sites (parallel read-only, sequential mutating, batched). --- src/agent/budget.ts | 7 +++++++ src/agent/loop.ts | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/agent/budget.ts b/src/agent/budget.ts index 4c7c77c..9e3f324 100644 --- a/src/agent/budget.ts +++ b/src/agent/budget.ts @@ -44,6 +44,13 @@ export class BudgetTracker { this.iterations++; } + /** Record non-model progress (a completed TOOL call). A 5-minute shell command is work, + * not a stall — without this, the wall ceiling read long tool runs as 'stalled' and + * killed the task right after the tool returned (live: 808s/600s, stalled 300s). */ + noteProgress(): void { + this.lastProgressAt = Date.now(); + } + checkpoint(): void { const wallMs = Date.now() - this.startTime; // A value of 0 (or negative) on any limit means "no limit" — useful for local diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 902ffae..a7f99dc 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -2076,6 +2076,7 @@ export class AgentLoop { const results = await Promise.all( readOnlyCalls.map(tc => this.executeToolCall(tc, txn, sessionId, options)), ); + budget.noteProgress(); for (let i = 0; i < readOnlyCalls.length; i++) { const tc = readOnlyCalls[i]!; const r = results[i]!; @@ -2109,6 +2110,7 @@ export class AgentLoop { // Single → execute as before const tc = batch[0]!; const r = await this.executeToolCall(tc, txn, sessionId, options); + budget.noteProgress(); yield { type: 'tool_result', data: { id: tc.id, name: tc.function.name, result: r.content, isError: r.isError, metadata: r.metadata } }; if (r.isError) this.recordToolFailure(tc.function.name, r.content); noteResult(tc.function.name, r); @@ -2127,6 +2129,7 @@ export class AgentLoop { const results = await Promise.all( batch.map(tc => this.executeToolCall(tc, txn, sessionId, options)), ); + budget.noteProgress(); for (let i = 0; i < batch.length; i++) { const tc = batch[i]!; const r = results[i]!; From 5d1a4050e6c853dd75ba253848df25cd39161c56 Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Sun, 5 Jul 2026 21:39:45 +0800 Subject: [PATCH 9/9] =?UTF-8?q?feat(tokens):=20spill=20oversized=20tool=20?= =?UTF-8?q?results=20to=20disk=20=E2=80=94=20context=20gets=20head+tail+pa?= =?UTF-8?q?th?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/agent/loop.ts | 37 +++++- src/agent/tool-spill.ts | 172 ++++++++++++++++++++++++++++ src/config/defaults.ts | 20 ++++ src/tools/web/http-request.ts | 31 ++++- test/tool-spill.test.ts | 205 ++++++++++++++++++++++++++++++++++ 5 files changed, 458 insertions(+), 7 deletions(-) create mode 100644 src/agent/tool-spill.ts create mode 100644 test/tool-spill.test.ts diff --git a/src/agent/loop.ts b/src/agent/loop.ts index a7f99dc..856fd5e 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -40,6 +40,7 @@ import { transformError, explainStreamError, detectStuckLoop, detectErrorLoop, e import { looksLikeBuildTask, isPlanningToolCall, PREFLIGHT_MESSAGE } from './preflight-gate.js'; import { dedupHistory } from './dedup.js'; import { ageToolResults } from './result-aging.js'; +import { applySpillGuard } from './tool-spill.js'; import { efficiencyDefaults, resolveSetting } from './efficiency-profile.js'; import { gatherInfraSignals, deriveAutoDisabledTools, ratchetAutoDisabled } from './tool-profile.js'; import { decideThinking, applyThinkingDecision, countTrailingToolErrors, modelSupportsSoftSwitch } from './thinking-control.js'; @@ -2436,11 +2437,45 @@ export class AgentLoop { } } - const result = await Promise.race([ + let result = await Promise.race([ this.registry.execute(tc.function.name, args, ctx), timeoutPromise, ]); + // ─── Universal spill guard (THE choke point for oversized results) ─── + // Every tool result passes through here before it can become message + // content, so one check covers http_request, web_fetch, shell, grep, + // browser_*, MCP tools — everything. Oversized content is written in + // full to ~/.qodex/tool-spill// and replaced by + // head + "[N chars spilled — full output: ]" + tail, so the model + // keeps status lines and tail errors AND knows how to read the rest + // (read_file with offset/limit). isError and metadata pass through + // untouched. Runs BEFORE the read-only cache store so a cache hit can + // never resurrect the full-size copy. Best-effort: a failed spill must + // never eat the result. + const spillMax = this.config.tools?.maxResultChars ?? 16_000; + if (spillMax > 0 && typeof result.content === 'string' && result.content.length > spillMax) { + try { + const spill = await applySpillGuard(tc.function.name, sessionId, result.content, { + maxResultChars: spillMax, + }); + if (spill.spilled) { + logger.info('Tool result spilled to disk', { + tool: tc.function.name, + chars: result.content.length, + spillPath: spill.spillPath, + }); + uiEvents.push({ + type: 'progress', + message: `💾 Large ${tc.function.name} output (${result.content.length.toLocaleString()} chars) spilled to disk — context keeps head+tail+path`, + } as any); + result = { ...result, content: spill.content }; + } + } catch (e: any) { + logger.warn('Spill guard failed (result kept in full, non-fatal)', { err: e?.message }); + } + } + // Store successful read-only results in cache if (this.toolCache && tool && tool.isReadOnly && !result.isError && typeof result.content === 'string') { this.toolCache.set(tc.function.name, args, result.content); diff --git a/src/agent/tool-spill.ts b/src/agent/tool-spill.ts new file mode 100644 index 0000000..ca963b9 --- /dev/null +++ b/src/agent/tool-spill.ts @@ -0,0 +1,172 @@ +/** + * Universal tool-result spill guard (context diet, applied at the choke point). + * + * The gap this fills: result-aging.ts shrinks large results *2 assistant turns + * later* — so a 278KB http_request page or a 2000-line sitemap still enters the + * context WHOLE, is prefilled at least twice, and only then gets trimmed. For a + * "check these URLs for 404s" task that needed status codes only, that's ~100k + * wasted tokens before the diet even starts. + * + * The structural fix: never let an oversized result into the context at full + * size. At the single point where a tool result becomes message content + * (AgentLoop.executeToolCall), any result whose content exceeds + * `tools.maxResultChars` is: + * + * 1. Written IN FULL to a spill file: ~/.qodex/tool-spill//-.txt + * 2. Replaced in-context by head (~4000 chars) + a marker carrying the REAL + * path and retrieval instructions + tail (~2000 chars). + * + * The model keeps the shape of the output (headers/status at the head, errors + * at the tail) and — crucially — knows exactly HOW to get more: `read_file` + * with offset/limit on the spill path. Nothing is lost, it's just moved out of + * the per-iteration token bill. + * + * Because this runs at ONE choke point, every tool is covered (http_request, + * web_fetch, shell, grep, browser_*, MCP tools…) with zero per-tool code. + * Tools that already cap themselves under the limit are naturally untouched + * (size check first — no double truncation). `isError` and `metadata` are + * preserved by the caller; this module only produces replacement content. + * + * Housekeeping: best-effort at write time — if the spill root grows past + * `maxDirBytes` (50MB default), the oldest files are pruned until under budget. + * Spill files are plain .txt so any session (or the user) can inspect them. + */ + +import { promises as fs } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { writeFileAtomic } from '../utils/atomic-write.js'; +import { logger } from '../utils/logger.js'; + +export const SPILL_MARK = 'chars spilled — full output:'; + +export interface SpillGuardOptions { + /** Results longer than this many chars are spilled. 0 disables the guard entirely. */ + maxResultChars: number; + /** Chars kept in-context from the start (status lines, headers). Default 4000. */ + keepHead?: number; + /** Chars kept in-context from the end (errors live at the tail). Default 2000. */ + keepTail?: number; + /** Spill root. Default ~/.qodex/tool-spill. Overridable for tests. */ + baseDir?: string; + /** Prune-oldest threshold for the spill root. Default 50MB. */ + maxDirBytes?: number; +} + +export interface SpillOutcome { + /** Replacement content if spilled, otherwise the original string, untouched. */ + content: string; + spilled: boolean; + /** Absolute path of the spill file, when spilled. */ + spillPath?: string; +} + +export function defaultSpillDir(): string { + return path.join(os.homedir(), '.qodex', 'tool-spill'); +} + +/** Monotonic per-process sequence so filenames never collide within a session. */ +let spillSeq = 0; + +/** + * Apply the spill guard to one tool result's content. Pure decision + one + * atomic file write. Never throws for the caller's benefit is NOT guaranteed — + * callers should try/catch and fall back to the original content (a failed + * spill must never eat the result). + */ +export async function applySpillGuard( + toolName: string, + sessionId: string, + content: string, + opts: SpillGuardOptions, +): Promise { + const max = opts.maxResultChars; + if (!max || max <= 0) return { content, spilled: false }; // 0 = disabled + if (content.length <= max) return { content, spilled: false }; // under limit — untouched, no double-truncation + if (content.includes(SPILL_MARK)) return { content, spilled: false }; // already spilled once — idempotent + + const keepHead = opts.keepHead ?? 4_000; + const keepTail = opts.keepTail ?? 2_000; + // Degenerate config (head+tail >= max) would "spill" into something no smaller + // than the original — skip rather than produce a bigger message. + if (keepHead + keepTail >= content.length) return { content, spilled: false }; + + const baseDir = opts.baseDir ?? defaultSpillDir(); + const sessionDir = path.join(baseDir, sanitize(sessionId)); + await fs.mkdir(sessionDir, { recursive: true }); + + const seq = String(++spillSeq).padStart(4, '0'); + const spillPath = path.join(sessionDir, `${seq}-${sanitize(toolName)}.txt`); + await writeFileAtomic(spillPath, content); + + const head = content.slice(0, keepHead); + const tail = content.slice(-keepTail); + const replaced = content.length; + // Marker format is STABLE — the model must always learn the same way how to + // retrieve the full output. Keep the phrase `chars spilled — full output:` intact. + const stub = + `${head}\n… [${replaced} ${SPILL_MARK} ${spillPath} — read_file with offset/limit for more] …\n${tail}`; + + // Best-effort housekeeping — never let a prune failure break the tool result. + try { + await pruneSpillDir(baseDir, opts.maxDirBytes ?? 50 * 1024 * 1024); + } catch (e: any) { + logger.debug('Spill-dir prune failed (non-fatal)', { err: e?.message }); + } + + return { content: stub, spilled: true, spillPath }; +} + +/** Filesystem-safe fragment for session ids / tool names. */ +function sanitize(s: string): string { + return s.replace(/[^a-zA-Z0-9._-]/g, '_').slice(0, 80) || 'unknown'; +} + +/** + * If the spill root exceeds `maxBytes`, delete oldest files (by mtime) until + * under budget. Walks one level of session subdirectories — the only layout we + * ever write. Best-effort: callers catch. + */ +export async function pruneSpillDir(baseDir: string, maxBytes: number): Promise { + let entries: string[]; + try { + entries = await fs.readdir(baseDir); + } catch { + return 0; // no spill dir yet — nothing to prune + } + + const files: { path: string; size: number; mtimeMs: number }[] = []; + for (const sub of entries) { + const subPath = path.join(baseDir, sub); + let st; + try { st = await fs.stat(subPath); } catch { continue; } + if (st.isFile()) { + files.push({ path: subPath, size: st.size, mtimeMs: st.mtimeMs }); + } else if (st.isDirectory()) { + let inner: string[] = []; + try { inner = await fs.readdir(subPath); } catch { /* skip */ } + for (const f of inner) { + const fp = path.join(subPath, f); + try { + const fst = await fs.stat(fp); + if (fst.isFile()) files.push({ path: fp, size: fst.size, mtimeMs: fst.mtimeMs }); + } catch { /* skip */ } + } + } + } + + let total = files.reduce((a, f) => a + f.size, 0); + if (total <= maxBytes) return 0; + + files.sort((a, b) => a.mtimeMs - b.mtimeMs); // oldest first + let pruned = 0; + for (const f of files) { + if (total <= maxBytes) break; + try { + await fs.unlink(f.path); + total -= f.size; + pruned++; + } catch { /* file busy/gone — move on */ } + } + return pruned; +} diff --git a/src/config/defaults.ts b/src/config/defaults.ts index fd57b78..3848443 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -216,6 +216,20 @@ export interface QodexConfig { showTokenCount: boolean; showCost: boolean; }; + /** + * Cross-cutting tool-result policies (applied at the registry/loop boundary, + * not per-tool). Optional for back-compat; defaults applied at load. + */ + tools?: { + /** + * Universal spill guard: any tool result whose content exceeds this many + * chars is written IN FULL to ~/.qodex/tool-spill// and enters + * the model context as head + "[N chars spilled — full output: ]" + + * tail instead. Covers every tool (http_request, web_fetch, shell, grep…) + * at one choke point. 0 disables. Default 16000 (~4k tokens). + */ + maxResultChars?: number; + }; /** * Telegram/Discord bot front-end (`qodex bot`). Tokens are NOT here — they're secrets read * from ~/.qodex/.env (TELEGRAM_BOT_TOKEN / DISCORD_TOKEN). Only the (non-secret) allowlists @@ -695,6 +709,12 @@ export const DEFAULT_CONFIG: QodexConfig = { showTokenCount: true, showCost: true, }, + tools: { + // Spill guard threshold. A 278KB http_request page used to enter the context + // whole (~70k tokens) and only get aged 2 turns later; now anything past this + // cap lives on disk and the context gets head+tail+path. 0 = disabled. + maxResultChars: 16_000, + }, bot: { telegram: { enabled: false, allowedUsers: [] }, discord: { enabled: false, allowedUsers: [] }, diff --git a/src/tools/web/http-request.ts b/src/tools/web/http-request.ts index 1e305c5..10d5ca7 100644 --- a/src/tools/web/http-request.ts +++ b/src/tools/web/http-request.ts @@ -32,15 +32,21 @@ const HttpRequestArgs = z.object({ query: z.record(z.string()).optional().describe('Query parameters appended to the URL.'), timeout_seconds: z.number().int().min(1).max(120).optional().describe('Default 30.'), allow_local: z.boolean().optional().describe('Allow localhost/private-IP destinations. Required for dev API testing. Default false.'), - max_response_bytes: z.number().int().min(1024).max(10_000_000).optional().describe('Response body cap. Default 1048576 (1 MB).'), + max_response_bytes: z.number().int().min(1024).max(10_000_000).optional().describe('Response body download cap. Default 1048576 (1 MB).'), follow_redirects: z.boolean().optional().describe('Follow 3xx redirects. Default true.'), + fullBody: z.boolean().describe('Return the ENTIRE response body in the result instead of the default head+tail excerpt (~6KB). Only set true when you actually need the full body content — status/header checks never need it. Default false.').optional(), }); +/** In-result body excerpt sizes (fullBody: false, the default). Status, headers + * and byte counts are ALWAYS reported in full — only the body is excerpted. */ +const BODY_EXCERPT_HEAD = 4_000; +const BODY_EXCERPT_TAIL = 2_000; + const PRIVATE_HOST_RE = /^(localhost|127\.|10\.|192\.168\.|169\.254\.|::1$|fc00::|fe80::)|\.local$/i; export class HttpRequestTool extends Tool> { name = 'http_request'; - description = 'Make an HTTP request to an external or local URL. For local dev APIs pass allow_local=true. Returns status, headers, and response body (up to 1MB). Destructive when method is POST/PUT/PATCH/DELETE — those mutate remote state.'; + description = 'Make an HTTP request to an external or local URL. For local dev APIs pass allow_local=true. Returns status, headers, and a head+tail excerpt (~6KB) of the response body with the total size noted — pass fullBody=true if you genuinely need the whole body (up to 1MB). Destructive when method is POST/PUT/PATCH/DELETE — those mutate remote state.'; isReadOnly = false; // POST/PUT/DELETE mutate isDestructive = true; argsSchema = HttpRequestArgs; @@ -138,12 +144,25 @@ export class HttpRequestTool extends Tool> { out.push(''); out.push(`## Body`); // Pretty-print JSON + let bodyOut = body; if (!isBinary && /application\/json|\+json/.test(ct)) { - try { out.push(JSON.stringify(JSON.parse(body), null, 2)); } - catch { out.push(body); } - } else { - out.push(body); + try { bodyOut = JSON.stringify(JSON.parse(body), null, 2); } + catch { /* not valid JSON — keep raw */ } + } + // Polite-at-the-source default: a status/header check ("is this URL 404?") + // never needs 278KB of HTML in context. Unless the model asked for + // fullBody, keep a head+tail excerpt (~6KB) and say how big the whole + // thing was. The universal spill guard remains the backstop for the + // fullBody path and for every other tool. + const excerptLimit = BODY_EXCERPT_HEAD + BODY_EXCERPT_TAIL; + if (!args.fullBody && bodyOut.length > excerptLimit) { + const omitted = bodyOut.length - excerptLimit; + bodyOut = + bodyOut.slice(0, BODY_EXCERPT_HEAD) + + `\n… [http_request body excerpt: ${received.toLocaleString()} bytes total, middle ${omitted.toLocaleString()} chars omitted — re-run with fullBody: true if you need the complete body] …\n` + + bodyOut.slice(-BODY_EXCERPT_TAIL); } + out.push(bodyOut); return { content: out.join('\n'), diff --git a/test/tool-spill.test.ts b/test/tool-spill.test.ts new file mode 100644 index 0000000..cfc074c --- /dev/null +++ b/test/tool-spill.test.ts @@ -0,0 +1,205 @@ +/** + * Universal tool-result spill guard (src/agent/tool-spill.ts) + the + * http_request head+tail body-excerpt default (src/tools/web/http-request.ts). + * + * The contract under test: an oversized tool result NEVER enters the model + * context whole. The full content lands on disk, the context gets + * head + "[N chars spilled — full output: ]" + tail, and the model is + * told exactly how to read the rest (read_file with offset/limit). Under-limit + * results and config 0 pass through untouched — no double truncation. + */ +import { describe, it, expect, beforeEach, afterEach, afterAll } from 'vitest'; +import { promises as fs } from 'fs'; +import * as http from 'http'; +import * as os from 'os'; +import * as path from 'path'; +import { applySpillGuard, pruneSpillDir, SPILL_MARK } from '../src/agent/tool-spill.js'; +import { DEFAULT_CONFIG } from '../src/config/defaults.js'; +import { HttpRequestTool } from '../src/tools/web/http-request.js'; +import type { ToolContext } from '../src/tools/base.js'; + +let baseDir: string; + +beforeEach(async () => { + baseDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qodex-spill-')); +}); +afterEach(async () => { + await fs.rm(baseDir, { recursive: true, force: true }); +}); + +const OPTS = () => ({ maxResultChars: 16_000, baseDir }); + +describe('applySpillGuard', () => { + it('spills an over-limit result: file exists with COMPLETE content, context gets head+tail+marker with the real path', async () => { + const head = 'STATUS: 200 OK — headers first\n'; + const tail = '\nTRAILING ERROR: sitemap entry 2103 missing'; + const content = head + 'x'.repeat(60_000) + tail; + + const r = await applySpillGuard('http_request', 'sess-1', content, OPTS()); + + expect(r.spilled).toBe(true); + expect(r.spillPath).toBeTruthy(); + // Full, byte-identical content on disk + const onDisk = await fs.readFile(r.spillPath!, 'utf-8'); + expect(onDisk).toBe(content); + // Spill file lives under //-.txt + expect(r.spillPath).toContain(path.join(baseDir, 'sess-1')); + expect(r.spillPath!.endsWith('-http_request.txt')).toBe(true); + + // Context content: head kept, tail kept, marker carries size + REAL path + retrieval hint + expect(r.content.startsWith(head)).toBe(true); + expect(r.content.endsWith(tail)).toBe(true); + expect(r.content).toContain(`${content.length} ${SPILL_MARK} ${r.spillPath}`); + expect(r.content).toContain('read_file with offset/limit'); + // And it actually shrank — that's the whole point + expect(r.content.length).toBeLessThan(8_000); + }); + + it('leaves an under-limit result untouched (no double truncation of self-capping tools)', async () => { + const content = 'y'.repeat(15_999); + const r = await applySpillGuard('shell', 'sess-1', content, OPTS()); + expect(r.spilled).toBe(false); + expect(r.content).toBe(content); + // Nothing written for this session + await expect(fs.readdir(path.join(baseDir, 'sess-1'))).rejects.toThrow(); + }); + + it('maxResultChars 0 disables the guard entirely', async () => { + const content = 'z'.repeat(100_000); + const r = await applySpillGuard('web_fetch', 'sess-1', content, { maxResultChars: 0, baseDir }); + expect(r.spilled).toBe(false); + expect(r.content).toBe(content); + }); + + it('is idempotent: already-spilled content (marker present) is not spilled again', async () => { + const first = await applySpillGuard('grep', 'sess-2', 'A'.repeat(40_000), OPTS()); + expect(first.spilled).toBe(true); + // Force it back through with an absurdly small limit — the marker guards it + const second = await applySpillGuard('grep', 'sess-2', first.content, { maxResultChars: 100, baseDir }); + expect(second.spilled).toBe(false); + expect(second.content).toBe(first.content); + }); + + it('preserves isError/metadata at the loop boundary (guard only replaces content)', async () => { + // The loop applies `result = { ...result, content: spill.content }` — mirror + // that here to pin the contract: isError and metadata survive a spill. + const result = { + content: '[TOOL_ERROR] giant stack trace\n' + 'frame\n'.repeat(10_000), + isError: true, + metadata: { status: 500, bytes: 12345 }, + }; + const spill = await applySpillGuard('shell', 'sess-3', result.content, OPTS()); + expect(spill.spilled).toBe(true); + const rewritten = { ...result, content: spill.content }; + expect(rewritten.isError).toBe(true); + expect(rewritten.metadata).toEqual({ status: 500, bytes: 12345 }); + expect(rewritten.content).toContain(SPILL_MARK); + expect(rewritten.content.startsWith('[TOOL_ERROR]')).toBe(true); + }); + + it('config default is 16000 chars', () => { + expect(DEFAULT_CONFIG.tools?.maxResultChars).toBe(16_000); + }); + + it('prunes oldest spill files when the dir exceeds the byte budget', async () => { + const dir = path.join(baseDir, 'sess-old'); + await fs.mkdir(dir, { recursive: true }); + const now = Date.now(); + for (let i = 0; i < 5; i++) { + const p = path.join(dir, `000${i}-shell.txt`); + await fs.writeFile(p, 'k'.repeat(1_000)); + // Stagger mtimes so "oldest" is well-defined + await fs.utimes(p, new Date(now - (10 - i) * 60_000), new Date(now - (10 - i) * 60_000)); + } + const pruned = await pruneSpillDir(baseDir, 2_500); // 5KB present, 2.5KB budget + expect(pruned).toBeGreaterThanOrEqual(2); + const left = await fs.readdir(dir); + // Newest files survive + expect(left).toContain('0004-shell.txt'); + expect(left).not.toContain('0000-shell.txt'); + }); +}); + +describe('http_request body excerpt (polite at the source)', () => { + let server: http.Server; + let port: number; + const BIG_BODY = '' + 'chunk-'.repeat(50_000) + 'THE-VERY-END'; // ~300KB + + const mkCtx = (): ToolContext => ({ + cwd: process.cwd(), + sessionId: 'test', + transaction: {} as any, + permissions: { check: () => ({ ok: true }) } as any, + askUser: async () => 'allow', + emit: () => {}, + signal: new AbortController().signal, + } as ToolContext); + + beforeEach(async () => { + server = http.createServer((_req, res) => { + res.writeHead(200, { 'content-type': 'text/html' }); + res.end(BIG_BODY); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as any).port; + }); + afterEach(async () => { + await new Promise(resolve => server.close(() => resolve())); + }); + + it('defaults to a head+tail excerpt with total-bytes note — status and headers stay complete', async () => { + const tool = new HttpRequestTool(); + const r = await tool.execute( + { url: `http://127.0.0.1:${port}/page`, allow_local: true } as any, + mkCtx(), + ); + expect(r.isError).toBeFalsy(); + expect(r.content).toContain('Status: 200'); + expect(r.content).toContain(''); // head kept + expect(r.content).toContain('THE-VERY-END'); // tail kept + expect(r.content).toContain('http_request body excerpt'); + expect(r.content).toContain('fullBody: true'); + expect(r.content).toContain(`${BIG_BODY.length.toLocaleString()} bytes total`); + // The excerpt keeps the whole result comfortably small (~6KB body + envelope) + expect(r.content.length).toBeLessThan(10_000); + expect((r.metadata as any).bytes).toBe(BIG_BODY.length); + }); + + it('fullBody: true bypasses the excerpt and returns the entire body', async () => { + const tool = new HttpRequestTool(); + const r = await tool.execute( + { url: `http://127.0.0.1:${port}/page`, allow_local: true, fullBody: true } as any, + mkCtx(), + ); + expect(r.isError).toBeFalsy(); + expect(r.content).toContain(BIG_BODY); + expect(r.content).not.toContain('http_request body excerpt'); + }); + + it('small bodies are returned whole with no excerpt marker', async () => { + await new Promise(resolve => server.close(() => resolve())); + server = http.createServer((_req, res) => { + res.writeHead(404, { 'content-type': 'text/plain' }); + res.end('not found'); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as any).port; + + const tool = new HttpRequestTool(); + const r = await tool.execute( + { url: `http://127.0.0.1:${port}/missing`, allow_local: true } as any, + mkCtx(), + ); + expect(r.content).toContain('Status: 404'); + expect(r.content).toContain('not found'); + expect(r.content).not.toContain('http_request body excerpt'); + }); + + it('schema exposes fullBody as a boolean with guidance', () => { + const schema = new HttpRequestTool().schema(); + const props = (schema.function.parameters as any).properties; + expect(props.fullBody).toBeDefined(); + expect(props.fullBody.type).toBe('boolean'); + expect(props.fullBody.description).toMatch(/head\+tail excerpt/); + }); +});