mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-07-10 01:29:41 +00:00
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.
This commit is contained in:
AgentSeal
parent
9c2ac6e22a
commit
926dcdb7fc
2 changed files with 150 additions and 71 deletions
|
|
@ -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<string>): 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<string>): 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<string>): 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<string>): 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)
|
||||
|
|
|
|||
|
|
@ -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<string> {
|
||||
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<void> {
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue