diff --git a/skills/story-lifecycle/scripts/ledger.js b/skills/story-lifecycle/scripts/ledger.js index 925d4dbe6..303e08d0a 100644 --- a/skills/story-lifecycle/scripts/ledger.js +++ b/skills/story-lifecycle/scripts/ledger.js @@ -33,6 +33,41 @@ function parseArgs(argv) { function sleep(ms) { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); } +function identity(stat) { return { dev: stat.dev, ino: stat.ino }; } + +function sameFile(file, expected) { + try { + const stat = fs.lstatSync(file); + return stat.isFile() && stat.dev === expected.dev && stat.ino === expected.ino; + } catch (err) { if (err.code === 'ENOENT') return false; throw err; } +} + +// Bind writes to an exclusively created descriptor, never a checked path. +function writeNew(file, content) { + const fd = fs.openSync(file, fs.constants.O_WRONLY | fs.constants.O_CREAT | + fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, 0o600); + try { + fs.writeFileSync(fd, content); + fs.fsyncSync(fd); + return identity(fs.fstatSync(fd)); + } finally { fs.closeSync(fd); } +} + +function readSnapshot(file, encoding) { + const fd = fs.openSync(file, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + try { + const stat = fs.fstatSync(fd); + if (!stat.isFile()) throw new LedgerError(`not a regular file: ${file}`); + return { content: fs.readFileSync(fd, encoding), owner: identity(stat) }; + } finally { fs.closeSync(fd); } +} + +function readRegular(file, encoding) { return readSnapshot(file, encoding).content; } + +function removeOwned(file, owner) { + if (sameFile(file, owner)) fs.unlinkSync(file); +} + function pidAlive(pid) { try { process.kill(pid, 0); return true; } catch (err) { return err.code === 'EPERM'; } } @@ -79,7 +114,13 @@ function registryDir(root, dir) { } function readLock(dir) { - try { return JSON.parse(fs.readFileSync(path.join(dir, '.lock'), 'utf8')); } catch { return null; } + let fd; + try { + fd = fs.openSync(path.join(dir, '.lock'), fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + const stat = fs.fstatSync(fd); + if (!stat.isFile()) return null; + return { ...JSON.parse(fs.readFileSync(fd, 'utf8')), owner: identity(stat) }; + } catch { return null; } finally { if (fd !== undefined) fs.closeSync(fd); } } function isStale(holder) { return holder.host === os.hostname() && !pidAlive(holder.pid); } @@ -89,18 +130,19 @@ function isStale(holder) { return holder.host === os.hostname() && !pidAlive(hol // Returns false if another waiter (or a crashed one) holds .lock.break. function breakStaleLock(dir) { const brk = path.join(dir, '.lock.break'); + let owner; try { - fs.writeFileSync(brk, String(process.pid), { flag: 'wx' }); + owner = writeNew(brk, String(process.pid)); } catch (err) { if (err.code === 'EEXIST') return false; throw err; } try { const holder = readLock(dir); - if (holder && isStale(holder)) fs.rmSync(path.join(dir, '.lock'), { force: true }); + if (holder && isStale(holder)) removeOwned(path.join(dir, '.lock'), holder.owner); return true; } finally { - fs.rmSync(brk, { force: true }); + removeOwned(brk, owner); } } @@ -109,8 +151,8 @@ function acquireLock(dir, waitMs) { const deadline = Date.now() + waitMs; for (;;) { try { - fs.writeFileSync(lock, JSON.stringify({ pid: process.pid, host: os.hostname(), at: new Date().toISOString() }), { flag: 'wx' }); - return () => fs.rmSync(lock, { force: true }); + const owner = writeNew(lock, JSON.stringify({ pid: process.pid, host: os.hostname(), at: new Date().toISOString() })); + return () => removeOwned(lock, owner); } catch (err) { if (err.code !== 'EEXIST') throw err; } @@ -130,7 +172,7 @@ function recover(dir) { const journalPath = path.join(txn, 'journal.json'); let rolledBack = 0; if (fs.existsSync(journalPath)) { - const { entries } = JSON.parse(fs.readFileSync(journalPath, 'utf8')); + const { entries } = JSON.parse(readRegular(journalPath, 'utf8')); for (const entry of entries) { resolveLedgerPath(dir, entry.path); if (entry.backup && !(BACKUP.test(entry.backup) && fs.lstatSync(path.join(txn, entry.backup)).isFile())) { @@ -140,13 +182,22 @@ function recover(dir) { for (const entry of entries) { const target = resolveLedgerPath(dir, entry.path); if (entry.backup) { + if (entry.original && !sameFile(target, entry.original) && !sameFile(target, entry.owner) && + !(entry.backupOwner && sameFile(target, entry.backupOwner))) { + throw new LedgerError(`interrupted update changed ownership: ${entry.path}`); + } // Fresh temp file, then rename: neither step writes through a planted symlink. const tmp = path.join(txn, 'restore.tmp'); fs.rmSync(tmp, { force: true }); - fs.copyFileSync(path.join(txn, entry.backup), tmp, fs.constants.COPYFILE_EXCL); + // Preserve the backup inode so a crash during recovery is retryable. + readRegular(path.join(txn, entry.backup)); // no-follow, regular-file validation + fs.linkSync(path.join(txn, entry.backup), tmp); fs.renameSync(tmp, target); } else { - fs.rmSync(target, { force: true }); + // A failed exclusive publication may leave a competing owner's file. + // Only delete the inode staged by this transaction, never that file. + if (entry.owner) removeOwned(target, entry.owner); + else if (fs.existsSync(target)) throw new LedgerError(`cannot prove interrupted create ownership: ${entry.path}`); } rolledBack++; } @@ -185,7 +236,7 @@ function replaceTable(md, rel, header, rows) { // Read the ledger with pending changes overlaid; return regenerated epic/sprint files that differ. function project(dir, pending = {}) { - const read = (rel) => (rel in pending ? pending[rel] : fs.readFileSync(resolveLedgerPath(dir, rel), 'utf8')); + const read = (rel) => (rel in pending ? pending[rel] : readRegular(resolveLedgerPath(dir, rel), 'utf8')); const list = (sub, re) => { const abs = path.join(dir, sub); const onDisk = fs.existsSync(abs) ? fs.readdirSync(abs).map((f) => (sub ? `${sub}/${f}` : f)) : []; @@ -220,7 +271,10 @@ function project(dir, pending = {}) { function init(dir) { for (const sub of ['epics', 'sprints']) fs.mkdirSync(path.join(dir, sub), { recursive: true }); const ignore = path.join(dir, '.gitignore'); - if (!fs.existsSync(ignore)) fs.writeFileSync(ignore, '.lock\n.lock.break\n.txn/\n.ids/\n'); + try { writeNew(ignore, '.lock\n.lock.break\n.txn/\n.ids/\n'); } catch (err) { + if (err.code !== 'EEXIST') throw err; + if (!fs.lstatSync(ignore).isFile()) throw new LedgerError('refusing non-regular ledger ignore file'); + } return { ok: true, dir }; } @@ -242,7 +296,7 @@ function allocate(root, dir, epic, count) { if (!Number.isSafeInteger(n)) throw new LedgerError('story ID exceeds safe integer range'); const id = `${epic}-${n}`; try { - fs.writeFileSync(path.join(reg, id), '', { flag: 'wx' }); + writeNew(path.join(reg, id), ''); ids.push(id); } catch (err) { if (err.code !== 'EEXIST') throw err; @@ -273,7 +327,7 @@ function apply(root, dir, changes) { const abs = resolveLedgerPath(dir, rel); if (!fs.existsSync(abs)) throw new LedgerError(`cannot update missing ${rel}`); if (!STORY_FILE.test(rel)) continue; - const from = parseStory(rel, fs.readFileSync(abs, 'utf8')).status; + const from = parseStory(rel, readRegular(abs, 'utf8')).status; const to = parseStory(rel, update[rel]).status; if (to !== from && STATUSES.indexOf(to) !== STATUSES.indexOf(from) + 1) { throw new LedgerError(`${rel}: status ${from} -> ${to} is not allowed; status moves forward one step at a time`); @@ -282,19 +336,32 @@ function apply(root, dir, changes) { const pending = { ...create, ...update }; const all = { ...pending, ...project(dir, pending).derived }; const txn = path.join(dir, '.txn'); - fs.mkdirSync(txn); + fs.mkdirSync(txn, { mode: 0o700 }); const entries = Object.keys(all).map((rel, i) => { const existed = fs.existsSync(resolveLedgerPath(dir, rel)); - if (existed) fs.copyFileSync(resolveLedgerPath(dir, rel), path.join(txn, `backup-${i}`)); - fs.writeFileSync(path.join(txn, `new-${i}`), all[rel]); - return { path: rel, backup: existed ? `backup-${i}` : null, staged: `new-${i}` }; + if (rel in create && existed) throw new LedgerError(`refusing to overwrite existing ${rel}`, 4); + const previous = existed ? readSnapshot(resolveLedgerPath(dir, rel)) : null; + const backupOwner = previous ? writeNew(path.join(txn, `backup-${i}`), previous.content) : null; + const owner = writeNew(path.join(txn, `new-${i}`), all[rel]); + return { path: rel, backup: existed ? `backup-${i}` : null, staged: `new-${i}`, + owner, original: previous ? previous.owner : null, backupOwner }; }); - fs.writeFileSync(path.join(txn, 'journal.tmp'), JSON.stringify({ entries })); + writeNew(path.join(txn, 'journal.tmp'), JSON.stringify({ entries })); fs.renameSync(path.join(txn, 'journal.tmp'), path.join(txn, 'journal.json')); const crashAfter = Number(process.env.STORY_LEDGER_CRASH_AFTER || -1); entries.forEach((entry, i) => { if (i === crashAfter) process.exit(70); // test-only fault injection - fs.renameSync(path.join(txn, entry.staged), resolveLedgerPath(dir, entry.path)); + const staged = path.join(txn, entry.staged); + const target = resolveLedgerPath(dir, entry.path); + if (entry.backup) { + if (!sameFile(target, entry.original)) throw new LedgerError(`update changed ownership: ${entry.path}`, 4); + fs.renameSync(staged, target); + } else { + // link publishes the staged inode exclusively; rename would clobber a + // file created after validation by an independent owner. + fs.linkSync(staged, target); + fs.unlinkSync(staged); + } }); fs.rmSync(path.join(txn, 'journal.json')); // commit point fs.rmSync(txn, { recursive: true, force: true }); @@ -310,9 +377,11 @@ function main() { if (cmd === 'init') return init(dir); if (!fs.existsSync(path.join(dir, 'epics'))) throw new LedgerError('no .stories/ ledger; run `init` first'); if (cmd === 'status') { - const { stories, derived } = project(dir); const holder = readLock(dir); - const recoveryPending = fs.existsSync(path.join(dir, '.txn')) && (!holder || isStale(holder)); + const transactionPending = fs.existsSync(path.join(dir, '.txn')); + const recoveryPending = transactionPending && (!holder || isStale(holder)); + if (transactionPending) return { stories: [], drift: [], recoveryPending, projectionAvailable: false }; + const { stories, derived } = project(dir); return { stories, drift: Object.keys(derived), recoveryPending }; } const waitMs = Number(opts['wait-ms'] ?? 5000); diff --git a/src/llm/providers/ollama.py b/src/llm/providers/ollama.py index 4571fdb73..0a7434daa 100644 --- a/src/llm/providers/ollama.py +++ b/src/llm/providers/ollama.py @@ -42,9 +42,10 @@ def _messages_payload(messages: list[Message]) -> list[dict[str, Any]]: for msg in messages: item = msg.to_dict() for tc in msg.tool_calls or []: - names[tc.id] = tc.name + if tc.id: + names[tc.id] = tc.name if msg.role == Role.TOOL: - tool_name = msg.name or names.get(msg.tool_call_id or "") + tool_name = msg.name or (names.get(msg.tool_call_id) if msg.tool_call_id else None) if tool_name: item["tool_name"] = tool_name payload.append(item) diff --git a/tests/docs/story-lifecycle-ledger.test.js b/tests/docs/story-lifecycle-ledger.test.js index c9d0a7491..f96bafb88 100644 --- a/tests/docs/story-lifecycle-ledger.test.js +++ b/tests/docs/story-lifecycle-ledger.test.js @@ -43,8 +43,15 @@ function writeJson(root, name, value) { return file; } +const ownedTemps = new Set(); +function tempDir(prefix) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + ownedTemps.add(dir); + return dir; +} + function newRepo() { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'story-ledger-')); + const root = tempDir('story-ledger-'); execFileSync('git', ['init', '-q', root]); assert.strictEqual(run(root, ['init']).code, 0); const input = writeJson(root, 'epic.json', { create: { 'epics/auth-flow.md': EPIC } }); @@ -183,7 +190,7 @@ const tests = [ ['git worktrees of one clone share ID allocation', () => { const root = newRepo(); execFileSync('git', ['-C', root, '-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-q', '--allow-empty', '-m', 'init']); - const wt = `${root}-wt`; + const wt = path.join(tempDir('story-worktree-'), 'checkout'); execFileSync('git', ['-C', root, 'worktree', 'add', '-q', wt]); run(wt, ['init']); run(wt, ['apply', '--input', writeJson(wt, 'epic.json', { create: { 'epics/auth-flow.md': EPIC } })]); @@ -195,7 +202,7 @@ const tests = [ ['concurrent allocation from two worktrees never hands out the same ID', async () => { const root = newRepo(); execFileSync('git', ['-C', root, '-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-q', '--allow-empty', '-m', 'init']); - const wt = `${root}-wt`; + const wt = path.join(tempDir('story-worktree-'), 'checkout'); execFileSync('git', ['-C', root, 'worktree', 'add', '-q', wt]); run(wt, ['init']); run(wt, ['apply', '--input', writeJson(wt, 'epic.json', { create: { 'epics/auth-flow.md': EPIC } })]); @@ -208,7 +215,7 @@ const tests = [ ['symlinked ledger directories are refused before any write', () => { const root = newRepo(); - const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'story-outside-')); + const outside = tempDir('story-outside-'); fs.rmSync(path.join(root, '.stories', 'sprints'), { recursive: true }); symlink(outside, path.join(root, '.stories', 'sprints'), 'dir'); const input = writeJson(root, 's.json', { create: { 'sprints/sprint-1.md': '# Sprint 1\n' } }); @@ -290,7 +297,7 @@ const tests = [ ['symlinked .ids registry is refused outside git', () => { const root = newRepo(); fs.rmSync(path.join(root, '.git'), { recursive: true }); - const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'story-ids-')); + const outside = tempDir('story-ids-'); symlink(outside, path.join(root, '.stories', '.ids'), 'dir'); assert.strictEqual(run(root, ['allocate', '--epic', 'auth-flow']).code, 2); assert.deepStrictEqual(fs.readdirSync(outside), []); @@ -311,9 +318,9 @@ const tests = [ fs.writeFileSync(lock, JSON.stringify({ pid: 2 ** 22 + 1, host: os.hostname(), at: 'stale' })); // Preload: right after the waiter reads the stale lock, another writer wins and holds a live lock. const preload = path.join(root, 'race.js'); - fs.writeFileSync(preload, `const fs = require('fs'); const orig = fs.readFileSync; let raced = false; -fs.readFileSync = function (p, ...a) { const r = orig.call(fs, p, ...a); - if (!raced && String(p).endsWith('.lock')) { raced = true; fs.unlinkSync(p); + fs.writeFileSync(preload, `const fs = require('fs'); const orig = fs.openSync; let raced = false; +fs.openSync = function (p, ...a) { const r = orig.call(fs, p, ...a); + if (!raced && String(p).endsWith('.lock') && a[0] === (fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0))) { raced = true; fs.unlinkSync(p); fs.writeFileSync(p, JSON.stringify({ pid: ${process.pid}, host: require('os').hostname(), at: 'live' }), { flag: 'wx' }); } return r; };`); const r = spawnSync(process.execPath, ['-r', preload, LEDGER, 'allocate', '--epic', 'auth-flow', '--root', root, '--wait-ms', '200'], { encoding: 'utf8' }); @@ -363,7 +370,7 @@ fs.readFileSync = function (p, ...a) { const r = orig.call(fs, p, ...a); ['git-common allocation registry refuses a symlinked parent', () => { const root = newRepo(); - const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'story-outside-')); + const outside = tempDir('story-outside-'); symlink(outside, path.join(root, '.git', 'story-lifecycle'), 'dir'); const result = run(root, ['allocate', '--epic', 'auth-flow']); assert.strictEqual(result.code, 2, result.err); @@ -396,6 +403,131 @@ fs.readFileSync = function (p, ...a) { const r = orig.call(fs, p, ...a); } }], + ['init never follows a symlink planted at the ignore-file write boundary', () => { + const root = tempDir('story-init-race-'); + const victim = path.join(root, 'victim.txt'); + fs.writeFileSync(victim, 'ORIGINAL'); + const preload = path.join(root, 'race.js'); + fs.writeFileSync(preload, `const fs = require('fs'); let planted = false; +const plant = (p) => { if (!planted && String(p).endsWith('/.stories/.gitignore')) { + planted = true; fs.symlinkSync(${JSON.stringify(victim)}, p); } }; +const exists = fs.existsSync; fs.existsSync = function (p) { const result = exists.call(fs, p); + if (!result) plant(p); return result; }; +const open = fs.openSync; fs.openSync = function (p, ...args) { plant(p); return open.call(fs, p, ...args); };`); + const result = spawnSync(process.execPath, ['-r', preload, LEDGER, 'init', '--root', root], { encoding: 'utf8' }); + assert.strictEqual(fs.readFileSync(victim, 'utf8'), 'ORIGINAL', result.stderr); + assert.notStrictEqual(result.status, 0, 'a planted symlink must fail closed'); + }], + + ['interrupted story-before-new-sprint batch reports recovery without projecting partial state', () => { + for (const interrupted of [false, true]) { + const root = newRepo(); + const [id] = run(root, ['allocate', '--epic', 'auth-flow']).out.ids; + const before = read(root, 'epics/auth-flow.md'); + const input = writeJson(root, 'sprint-batch.json', { create: { + [`${id}.md`]: story(id, 'Assigned', { sprint: '1' }), + 'sprints/sprint-1.md': '# Sprint 1\n\n\n| ID | Title | Points | Status |\n| --- | --- | --- | --- |\n\n## Total Points: 0\n', + } }); + const result = run(root, ['apply', '--input', input], interrupted ? { STORY_LEDGER_CRASH_AFTER: '1' } : {}); + assert.strictEqual(result.code, interrupted ? 70 : 0, result.err); + if (interrupted) { + const partialStory = read(root, `${id}.md`); + const status = run(root, ['status']); + assert.strictEqual(status.code, 0, status.err); + assert.strictEqual(status.out.recoveryPending, true); + assert.strictEqual(status.out.projectionAvailable, false); + assert.strictEqual(read(root, `${id}.md`), partialStory, 'status must be read-only'); + assert.strictEqual(run(root, ['recover']).code, 0); + assert.deepStrictEqual(storyFiles(root), []); + assert.strictEqual(read(root, 'epics/auth-flow.md'), before); + assert.strictEqual(run(root, ['apply', '--input', input]).code, 0); + } + const status = run(root, ['status']); + assert.strictEqual(status.code, 0, status.err); + assert.strictEqual(status.out.recoveryPending, false); + assert.deepStrictEqual(status.out.drift, []); + assert.ok(read(root, 'sprints/sprint-1.md').includes(`| ${id} | Assigned | 3 | todo |`)); + } + }], + + ['a competing create at publication is preserved through failure and recovery', () => { + const root = newRepo(); + const { ids, input } = createStories(root, ['Ours']); + const target = path.join(root, '.stories', `${ids[0]}.md`); + const preload = path.join(root, 'publish-race.js'); + fs.writeFileSync(preload, `const fs = require('fs'); let planted = false; +const plant = (target) => { if (!planted && target === ${JSON.stringify(target)}) { + planted = true; fs.writeFileSync(target, 'COMPETING OWNER', {flag:'wx'}); } }; +for (const name of ['renameSync', 'linkSync']) { const original = fs[name]; fs[name] = function (source, target, ...a) { + if (String(source).includes('/.txn/new-')) plant(target); return original.call(fs, source, target, ...a); }; }`); + const result = spawnSync(process.execPath, ['-r', preload, LEDGER, 'apply', '--input', input, '--root', root], { encoding: 'utf8' }); + assert.notStrictEqual(result.status, 0, 'publication must not overwrite another owner'); + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'COMPETING OWNER'); + assert.strictEqual(run(root, ['recover']).code, 0); + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'COMPETING OWNER', 'rollback must not delete another owner'); + }], + + ['staged writes refuse symlinks planted just before open', () => { + const root = newRepo(); + const { input } = createStories(root, ['Ours']); + const victim = path.join(root, 'victim.txt'); fs.writeFileSync(victim, 'ORIGINAL'); + const preload = path.join(root, 'stage-race.js'); + fs.writeFileSync(preload, `const fs = require('fs'); let planted = false; +const plant = (p) => { if (!planted && String(p).endsWith('/.txn/new-0')) { + planted = true; fs.symlinkSync(${JSON.stringify(victim)}, p); } }; +const open = fs.openSync; fs.openSync = function (p, ...a) { plant(p); return open.call(fs, p, ...a); }; +const write = fs.writeFileSync; fs.writeFileSync = function (p, ...a) { plant(p); return write.call(fs, p, ...a); };`); + const result = spawnSync(process.execPath, ['-r', preload, LEDGER, 'apply', '--input', input, '--root', root], { encoding: 'utf8' }); + assert.notStrictEqual(result.status, 0); + assert.strictEqual(fs.readFileSync(victim, 'utf8'), 'ORIGINAL'); + assert.strictEqual(run(root, ['recover']).code, 0); + assert.deepStrictEqual(storyFiles(root), []); + }], + + ['concurrent attempts to publish one allocated story have one owner', async () => { + const root = newRepo(); + const { ids, input } = createStories(root, ['Single owner']); + const results = await Promise.all([runAsync(root, ['apply', '--input', input]), runAsync(root, ['apply', '--input', input])]); + assert.deepStrictEqual(results.map((r) => r.code).sort(), [0, 4]); + assert.deepStrictEqual(storyFiles(root), [`${ids[0]}.md`]); + assert.ok(read(root, `${ids[0]}.md`).includes('Single owner')); + assert.deepStrictEqual(run(root, ['status']).out.drift, []); + }], + + ['release leaves a replacement lock owned by another writer intact', () => { + const root = newRepo(); + const { input } = createStories(root, ['Ours']); + const lock = path.join(root, '.stories', '.lock'); + const preload = path.join(root, 'lock-owner-race.js'); + fs.writeFileSync(preload, `const fs = require('fs'); const original = fs.rmSync; let replaced = false; +fs.rmSync = function (p, ...args) { const result = original.call(fs, p, ...args); + if (!replaced && String(p).endsWith('/.stories/.txn')) { replaced = true; fs.unlinkSync(${JSON.stringify(lock)}); + fs.writeFileSync(${JSON.stringify(lock)}, JSON.stringify({pid:${process.pid},host:require('os').hostname(),at:'replacement'}), {flag:'wx'}); } + return result; };`); + const result = spawnSync(process.execPath, ['-r', preload, LEDGER, 'apply', '--input', input, '--root', root], { encoding: 'utf8' }); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(fs.readFileSync(lock, 'utf8').includes('replacement')); + }], + + ['recovery can itself be interrupted and retried without losing rollback ownership', () => { + const root = newRepo(); + const { ids, input } = createStories(root, ['Before']); + assert.strictEqual(run(root, ['apply', '--input', input]).code, 0); + const before = read(root, `${ids[0]}.md`); + const change = writeJson(root, 'update.json', { update: { [`${ids[0]}.md`]: story(ids[0], 'After', { status: 'in-progress' }) } }); + assert.strictEqual(run(root, ['apply', '--input', change], { STORY_LEDGER_CRASH_AFTER: '1' }).code, 70); + const preload = path.join(root, 'recover-crash.js'); + fs.writeFileSync(preload, `const fs = require('fs'); const original = fs.renameSync; +fs.renameSync = function (source, ...args) { const result = original.call(fs, source, ...args); + if (String(source).endsWith('/.txn/restore.tmp')) process.exit(70); return result; };`); + const recovery = spawnSync(process.execPath, ['-r', preload, LEDGER, 'recover', '--root', root], { encoding: 'utf8' }); + assert.strictEqual(recovery.status, 70); + const retry = run(root, ['recover']); + assert.strictEqual(retry.code, 0, retry.err); + assert.strictEqual(read(root, `${ids[0]}.md`), before); + assert.deepStrictEqual(run(root, ['status']).out.drift, []); + }], + ['ledger paths are confined to .stories/ file shapes', () => { const dir = path.join(os.tmpdir(), '.stories'); for (const bad of ['../x-1.md', 'epics/../../x.md', 'Auth-1.md', 'a/b-1.md', 'epics/a.txt', 'sprints/sprint-0.md', '/abs-1.md']) { @@ -422,6 +554,9 @@ fs.readFileSync = function (p, ...a) { const r = orig.call(fs, p, ...a); console.log(` ✗ ${name}`); console.log(` Error: ${error.message}`); failed++; + } finally { + for (const dir of ownedTemps) fs.rmSync(dir, { recursive: true, force: true }); + ownedTemps.clear(); } } console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); diff --git a/tests/test_provider_tools.py b/tests/test_provider_tools.py index 5f4960b81..1b2199fc2 100644 --- a/tests/test_provider_tools.py +++ b/tests/test_provider_tools.py @@ -502,3 +502,21 @@ def test_ollama_malformed_capability_declaration_is_not_authority(monkeypatch, c provider.generate(request) assert ['tools' in chat for chat in server.chats] == [False, True] assert server.shows == ['custom', 'custom'] + + +def test_ollama_mixed_history_does_not_infer_tool_names_from_empty_ids(monkeypatch: pytest.MonkeyPatch) -> None: + server = _FakeOllama(monkeypatch, [_ollama_answer("ok")]) + messages = [ + Message(role=Role.ASSISTANT, content="", tool_calls=[ToolCall(id="", name="first", arguments={}), ToolCall(id="", name="last", arguments={}), ToolCall(id="real-id", name="identified", arguments={})]), + Message(role=Role.TOOL, content="unnamed missing"), + Message(role=Role.TOOL, content="unnamed empty", tool_call_id=""), + Message(role=Role.TOOL, content="explicit empty", tool_call_id="", name="explicit"), + Message(role=Role.TOOL, content="identified result", tool_call_id="real-id"), + Message(role=Role.TOOL, content="explicit override", tool_call_id="real-id", name="override"), + ] + OllamaProvider().generate(LLMInput(messages=messages)) + results = server.chats[0]["messages"][1:] + assert "tool_name" not in results[0] + assert "tool_name" not in results[1] + assert [item["tool_name"] for item in results[2:]] == ["explicit", "identified", "override"] + assert results[3]["tool_call_id"] == "real-id"