From 926dcdb7fceb63243a79f49c213992c1722799da Mon Sep 17 00:00:00 2001 From: AgentSeal Date: Sun, 5 Jul 2026 15:02:15 +0200 Subject: [PATCH] fix(kiro): harden ws-session parsing and cover the stale-cache path (#619) Maintainer follow-up on top of the parsing fix: - Replace the placeholder cache tests with a regression test that runs the full parseAllSessions() pipeline against a seeded session-cache.json: a zero-turn entry at the current fingerprint is honored (control), and a pre-fix fingerprint forces the re-parse that recovers the missed calls. - Treat history items carrying an executionId as execution-backed regardless of their text, so a stub-text change can never reintroduce double-counting; the 'On it.' check remains for stubs whose ref rides a separate item. - Resolve the workspace-session timestamp before consuming the dedup key, and drop the call when stat fails instead of fabricating a current time. - Read selectedModel through stringField instead of an unchecked cast. - Import stat statically. --- src/providers/kiro.ts | 27 ++-- tests/kiro-cache-invalidation.test.ts | 194 ++++++++++++++++++-------- 2 files changed, 150 insertions(+), 71 deletions(-) diff --git a/src/providers/kiro.ts b/src/providers/kiro.ts index e5d4c9c..1488f4b 100644 --- a/src/providers/kiro.ts +++ b/src/providers/kiro.ts @@ -1,6 +1,6 @@ import type { Dirent } from 'fs' import { existsSync } from 'fs' -import { readdir, readFile } from 'fs/promises' +import { readdir, readFile, stat } from 'fs/promises' import { basename, dirname, extname, join } from 'path' import { homedir } from 'os' @@ -585,7 +585,7 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars const historyArr = record['history'] if (Array.isArray(historyArr) && typeof record['sessionId'] === 'string') { const sessionId = record['sessionId'] as string - const modelRaw = (record['selectedModel'] as string) ?? '' + const modelRaw = stringField(record, ['selectedModel']) let modelId = normalizeModelId(modelRaw) if (modelId === 'auto' || !modelId) modelId = 'kiro-auto' @@ -601,7 +601,8 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars if (!rec) continue // Track if this session references execution files (which are parsed separately) - if (typeof rec['executionId'] === 'string') hasExecutionRefs = true + const execBacked = typeof rec['executionId'] === 'string' + if (execBacked) hasExecutionRefs = true const msg = asRecord(rec['message']) if (!msg) continue @@ -610,7 +611,11 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars if (role === 'user' && text) { inputChars += text.length pendingUserMessage = text.slice(0, 500) - } else if (role === 'assistant' && text && text !== 'On it.') { + } else if (role === 'assistant' && !execBacked && text && text !== 'On it.') { + // An item carrying an executionId is execution-backed: its content is + // counted from the execution file, so counting it here would double-count. + // 'On it.' is the observed placeholder text Kiro writes for such stubs + // when the executionId rides a separate history item. outputChars += text.length hasRealAssistantContent = true } @@ -625,20 +630,20 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars // Skip sessions with no meaningful content if (inputChars === 0 && outputChars === 0) return - const dedupKey = `kiro:ws-session:${sessionId}` - if (seenKeys.has(dedupKey)) return - seenKeys.add(dedupKey) - - // Use file mtime as timestamp (workspace-session files don't carry startTime) - const { stat } = await import('fs/promises') + // Use file mtime as timestamp (workspace-session files don't carry startTime). + // No stat means no usable timestamp: drop the call like the other parse paths. let timestamp: string try { const s = await stat(source.path) timestamp = new Date(s.mtimeMs).toISOString() } catch { - timestamp = new Date().toISOString() + return } + const dedupKey = `kiro:ws-session:${sessionId}` + if (seenKeys.has(dedupKey)) return + seenKeys.add(dedupKey) + const inputTokens = Math.ceil(inputChars / CHARS_PER_TOKEN) const outputTokens = Math.ceil(outputChars / CHARS_PER_TOKEN) const costUSD = calculateCost(modelId, inputTokens, outputTokens, 0, 0, 0) diff --git a/tests/kiro-cache-invalidation.test.ts b/tests/kiro-cache-invalidation.test.ts index 91b79ae..725ba5b 100644 --- a/tests/kiro-cache-invalidation.test.ts +++ b/tests/kiro-cache-invalidation.test.ts @@ -1,78 +1,152 @@ -import { mkdir, mkdtemp, rm, stat, writeFile } from 'fs/promises' -import { tmpdir } from 'os' +// Regression test for the Kiro stale-cache path (#618, #619). +// +// Before this fix the Kiro parser returned 0 turns for every IDE execution +// file that stores content under `context.messages[].entries`. Those empty +// results were cached in session-cache.json keyed by file fingerprint, so +// shipping a fixed parser alone is not enough: unchanged files would keep +// their cached `turns: []` forever. The fix registers kiro in +// PROVIDER_PARSE_VERSIONS, which changes the provider envFingerprint and +// makes `parseProviderSources` discard the stale section on first run. +// +// This test exercises the full `parseAllSessions` pipeline against a seeded +// session-cache.json, in both directions: +// - a cache seeded with the CURRENT fingerprint is honored (zero-turn entry +// stays, proving the seed is structurally valid and actually trusted) +// - a cache seeded with the PRE-FIX fingerprint is discarded and the file +// is re-parsed, recovering the calls the broken parser missed + +import { describe, it, expect, beforeEach, afterAll, vi } from 'vitest' +import { mkdir, rm, writeFile } from 'fs/promises' +import { createHash } from 'crypto' import { join } from 'path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { clearSessionCache, parseAllSessions } from '../src/parser.js' +import { + CACHE_VERSION, + computeEnvFingerprint, + fingerprintFile, + type SessionCache, +} from '../src/session-cache.js' -import { createKiroProvider } from '../src/providers/kiro.js' -import { CACHE_VERSION, computeEnvFingerprint } from '../src/session-cache.js' -import type { ParsedProviderCall } from '../src/providers/types.js' - -let tmpDir: string - -beforeEach(async () => { - tmpDir = await mkdtemp(join(tmpdir(), 'kiro-cache-inv-')) +// The kiro provider singleton captures homedir() when its module is first +// imported, so HOME must point at the test root before ../src/parser.js is +// evaluated. vi.hoisted runs ahead of the static imports above (but after +// tests/setup/env-isolation.ts, whose per-test beforeEach re-sandboxes env +// vars — anything read at *call* time, like CODEBURN_CACHE_DIR, must be +// re-asserted in this file's own beforeEach). +const testRoot = vi.hoisted(() => { + const root = `${process.env['TMPDIR'] || '/tmp'}/kiro-cache-inv-${process.pid}-${Date.now()}` + process.env['HOME'] = `${root}/home` + process.env['USERPROFILE'] = `${root}/home` + return root }) -afterEach(async () => { - await rm(tmpDir, { recursive: true, force: true }) +const HOME = join(testRoot, 'home') +const CACHE_DIR = join(testRoot, 'cache') + +function kiroAgentDir(): string { + if (process.platform === 'darwin') { + return join(HOME, 'Library', 'Application Support', 'Kiro', 'User', 'globalStorage', 'kiro.kiroagent') + } + if (process.platform === 'win32') { + return join(HOME, 'AppData', 'Roaming', 'Kiro', 'User', 'globalStorage', 'kiro.kiroagent') + } + return join(HOME, '.config', 'Kiro', 'User', 'globalStorage', 'kiro.kiroagent') +} + +// What computeEnvFingerprint('kiro') returned before kiro had an entry in +// PROVIDER_PARSE_VERSIONS: no env vars, no parser version, i.e. a hash of +// zero parts. This is the fingerprint sitting in every pre-fix cache. +function preFixFingerprint(): string { + return createHash('sha256').update([].join('\0')).digest('hex').slice(0, 16) +} + +// Writes one IDE execution file in the context.messages[].entries format that +// the pre-fix parser turned into 0 turns, and returns its path. +async function seedExecutionFile(): Promise { + const dir = join(kiroAgentDir(), 'a'.repeat(32), 'b'.repeat(32)) + await mkdir(dir, { recursive: true }) + const path = join(dir, 'exec-stale-001') + await writeFile(path, JSON.stringify({ + executionId: 'exec-stale-001', + workflowType: 'chat-agent', + status: 'succeed', + startTime: 1780000000000, + chatSessionId: 'session-stale-001', + context: { + messages: [ + { role: 'human', entries: [{ type: 'text', text: 'What is TypeScript?' }] }, + { role: 'bot', entries: [{ type: 'text', text: 'TypeScript is a typed superset of JavaScript.' }] }, + ], + }, + })) + return path +} + +async function seedCache(execPath: string, envFingerprint: string): Promise { + const fp = await fingerprintFile(execPath) + if (!fp) throw new Error('failed to fingerprint seeded execution file') + const cache: SessionCache = { + version: CACHE_VERSION, + providers: { + kiro: { + envFingerprint, + files: { + [execPath]: { fingerprint: fp, mcpInventory: [], turns: [] }, + }, + }, + }, + } + await mkdir(CACHE_DIR, { recursive: true }) + await writeFile(join(CACHE_DIR, 'session-cache.json'), JSON.stringify(cache)) +} + +async function parseKiroCalls() { + const projects = await parseAllSessions(undefined, 'kiro') + return projects + .flatMap(p => p.sessions) + .flatMap(s => s.turns) + .flatMap(t => t.assistantCalls) +} + +beforeEach(async () => { + // Runs after env-isolation's global beforeEach, which cleared this var. + process.env['CODEBURN_CACHE_DIR'] = CACHE_DIR + clearSessionCache() + await rm(testRoot, { recursive: true, force: true }) +}) + +afterAll(async () => { + clearSessionCache() + await rm(testRoot, { recursive: true, force: true }) }) describe('Kiro session cache invalidation', () => { - it('kiro parser version is registered — stale cache fingerprints will mismatch', () => { - // The cache invalidation mechanism works by comparing envFingerprints. - // When kiro is NOT in PROVIDER_PARSE_VERSIONS, its fingerprint matches - // any unknown provider (no parser version component). With the entry added, - // existing caches that were computed without the parser version will have a - // different fingerprint → triggering a full re-parse. - const kiroFp = computeEnvFingerprint('kiro') - const unknownFp = computeEnvFingerprint('nonexistent-provider') - expect(kiroFp).not.toBe(unknownFp) + it('registers a kiro parser version in the env fingerprint', () => { + expect(computeEnvFingerprint('kiro')).not.toBe(preFixFingerprint()) }) - it('stale zero-turn cache entry is invalidated by parser version change', () => { - // Simulate the scenario: old cache was computed without parser version - const oldFingerprint = 'abcdef0123456789' // would have been computed before kiro had a parser version - const currentFingerprint = computeEnvFingerprint('kiro') + it('control: a zero-turn cache entry at the current fingerprint is honored', async () => { + const execPath = await seedExecutionFile() + await seedCache(execPath, computeEnvFingerprint('kiro')) - // The fingerprints differ, which causes parseProviderSources() to discard - // the cached section and re-parse all files from scratch - expect(oldFingerprint).not.toBe(currentFingerprint) + const calls = await parseKiroCalls() + + // The seeded cache is structurally valid and trusted: the unchanged file + // is not re-parsed, so the stale zero-turn entry yields no calls. This + // guards the regression test below against passing for the wrong reason + // (an invalid or unread seed being silently ignored). + expect(calls).toHaveLength(0) }) - it('file with context.messages.entries parses correctly after cache miss', async () => { - // This verifies that after cache invalidation forces a re-parse, - // the fixed parser correctly extracts content from context.messages.entries - const wsHash = 'a'.repeat(32) - const subDir = 'b'.repeat(32) - await mkdir(join(tmpDir, wsHash, subDir), { recursive: true }) + it('regression: a pre-fix cache fingerprint forces a re-parse that recovers the calls', async () => { + const execPath = await seedExecutionFile() + await seedCache(execPath, preFixFingerprint()) - await writeFile(join(tmpDir, wsHash, subDir, 'exec-001'), JSON.stringify({ - executionId: 'exec-cache-miss', - workflowType: 'chat-agent', - status: 'succeed', - startTime: 1780000000000, - chatSessionId: 'session-cache-miss', - context: { - messages: [ - { role: 'human', entries: [{ type: 'text', text: 'What is TypeScript?' }] }, - { role: 'bot', entries: [{ type: 'text', text: 'TypeScript is a typed superset of JavaScript that compiles to plain JavaScript.' }] }, - ], - }, - })) + const calls = await parseKiroCalls() - const provider = createKiroProvider(tmpDir, tmpDir, '/nonexistent') - const sessions = await provider.discoverSessions() - const calls: ParsedProviderCall[] = [] - for (const source of sessions) { - for await (const call of provider.createSessionParser(source, new Set()).parse()) { - calls.push(call) - } - } - - // Parser correctly extracts content — this is what happens after cache invalidation expect(calls).toHaveLength(1) - expect(calls[0]!.inputTokens).toBeGreaterThan(0) - expect(calls[0]!.outputTokens).toBeGreaterThan(0) + expect(calls[0]!.usage.inputTokens).toBeGreaterThan(0) + expect(calls[0]!.usage.outputTokens).toBeGreaterThan(0) }) })