diff --git a/lib/agent/driver.js b/lib/agent/driver.js index 8ebe3e8..54e6e16 100644 --- a/lib/agent/driver.js +++ b/lib/agent/driver.js @@ -171,7 +171,8 @@ export function createDriver({ profile, ports, transcripts, env = process.env, l const at = { sessionId, projectDir: profile.payload.projectDir(payload, context) }; const waitLeft = (budgetOf = 'prompt') => waitBudgetOf(profile.budgets, budgetOf, { lifetime, startedAt }); return await HANDLERS[event]({ profile, ports, hostEvent, payload, context, at, waitLeft, startedAt, options }); - } catch { + } catch (error) { + options.onError?.(error); return profile.reply.empty(hostEvent, payload); } }, diff --git a/lib/channels/hook/runner.js b/lib/channels/hook/runner.js index a591080..fca96d4 100644 --- a/lib/channels/hook/runner.js +++ b/lib/channels/hook/runner.js @@ -13,7 +13,7 @@ import { readPayload } from './stdin.js'; * One command-hook invocation: the host's payload on stdin, the agent's answer as JSON on stdout, exit 0. Inside * this plugin's own headless distillation the hook answers the agent's empty reply and does nothing else. */ -export async function runHookProcess(agent, hostEvent, { stdin = process.stdin, stdout = process.stdout, env = process.env, exit = process.exit, profileOf = profileNamed } = {}) { +export async function runHookProcess(agent, hostEvent, { stdin = process.stdin, stdout = process.stdout, stderr = process.stderr, env = process.env, exit = process.exit, profileOf = profileNamed } = {}) { const startedAt = Date.now(); const write = (answer) => stdout.write(JSON.stringify(answer ?? {}), () => exit(0)); const profile = await profileOf(agent).catch(() => null); @@ -22,5 +22,14 @@ export async function runHookProcess(agent, hostEvent, { stdin = process.stdin, if (isHeadlessRun(env)) return write(profile.reply.empty(hostEvent)); const { ports } = environmentFor({ profile, env, lifetime: EPHEMERAL, runBackground: runBackgroundTasks }); const driver = createDriver({ profile, ports, transcripts, env, lifetime: EPHEMERAL, startedAt }); - return write(await driver.handle(hostEvent, payload)); + let diagnostic; + const answer = await driver.handle(hostEvent, payload, { + onError(error) { + if (['EINVALIDSTATE', 'ESTATEIO', 'ELOCKED'].includes(error?.code)) { + diagnostic = `Evolver hook skipped a state operation (${error.code}). Check the plugin state directory and retry.\n`; + } + }, + }); + if (diagnostic) return stderr.write(diagnostic, () => write(answer)); + return write(answer); } diff --git a/lib/runtime/store/file-lock.js b/lib/runtime/store/file-lock.js index f730098..2463b2b 100644 --- a/lib/runtime/store/file-lock.js +++ b/lib/runtime/store/file-lock.js @@ -15,22 +15,41 @@ const CLAIM_STALE_MS = 60 * 1000; export function readState(target, fallback) { try { const parsed = JSON.parse(fs.readFileSync(target, 'utf8')); - return parsed && typeof parsed === 'object' && Array.isArray(parsed) === Array.isArray(fallback) ? parsed : fallback; - } catch { - return fallback; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed) !== Array.isArray(fallback)) throw Object.assign(new Error(`Invalid state shape in ${target}; repair it before retrying.`), { code: 'EINVALIDSTATE' }); + return parsed; + } catch (error) { + if (error?.code === 'ENOENT') return fallback; + if (error instanceof SyntaxError) throw Object.assign(new Error(`Invalid JSON in ${target}; repair it before retrying.`), { code: 'EINVALIDSTATE', cause: error }); + if (error?.code === 'EINVALIDSTATE') throw error; + throw Object.assign(new Error(`Could not read state in ${target}; check access and retry.`), { code: 'ESTATEIO', cause: error }); } } export function writeState(target, value) { fs.mkdirSync(path.dirname(target), { recursive: true, mode: 0o700 }); const temporary = `${target}.${process.pid}.${crypto.randomBytes(6).toString('hex')}.tmp`; - fs.writeFileSync(temporary, JSON.stringify(value), { mode: 0o600, flag: 'wx' }); + const deadline = Date.now() + LOCK_WAIT_MS; + let descriptor; try { - fs.renameSync(temporary, target); - } catch (error) { - if (!['EEXIST', 'EPERM'].includes(error?.code)) throw error; - fs.rmSync(target, { force: true }); - fs.renameSync(temporary, target); + descriptor = fs.openSync(temporary, 'wx', 0o600); + try { + fs.writeFileSync(descriptor, JSON.stringify(value)); + } catch (error) { + try { fs.closeSync(descriptor); } catch {} + throw error; + } + fs.closeSync(descriptor); + for (;;) { + try { + fs.renameSync(temporary, target); + return; + } catch (error) { + if (!['EEXIST', 'EACCES', 'EPERM', 'EBUSY'].includes(error?.code) || Date.now() >= deadline) throw error; + pause(LOCK_RETRY_MS); + } + } + } finally { + if (descriptor !== undefined) removeState(temporary); } } @@ -92,23 +111,34 @@ function acquire(lockPath, deadline) { } function release(lockPath, token) { - if (contentOf(lockPath) === token) removeState(lockPath); + const deadline = Date.now() + LOCK_WAIT_MS; + for (;;) { + try { + if (fs.readFileSync(lockPath, 'utf8') === token) fs.unlinkSync(lockPath); + return; + } catch (error) { + if (!['EACCES', 'EPERM', 'EBUSY'].includes(error?.code) || Date.now() >= deadline) return; + pause(LOCK_RETRY_MS); + } + } } /** * Applies `mutate` to a state file under an exclusive lock, so hook processes sharing it cannot overwrite each - * other's entries. A dead owner's lock is taken over; one that cannot be had within LOCK_WAIT_MS is given up on - * and the update runs unguarded, which never blocks a session. `mutate` returns undefined to leave the file. + * other's entries. A dead owner's lock is taken over; an unavailable lock refuses the update before reading + * or mutating state. `mutate` returns undefined to leave the file. */ export function updateState(target, fallback, mutate) { const lockPath = `${target}.lock`; - const token = acquire(lockPath, Date.now() + LOCK_WAIT_MS); + let token, cause; + try { token = acquire(lockPath, Date.now() + LOCK_WAIT_MS); } catch (error) { cause = error; } + if (!token) throw Object.assign(new Error('State is busy or unavailable; no update was made', { cause }), { code: 'ELOCKED' }); try { const next = mutate(readState(target, fallback)); if (next !== undefined) writeState(target, next); return next; } finally { - if (token) release(lockPath, token); + release(lockPath, token); } } diff --git a/test/channels/hook.test.js b/test/channels/hook.test.js index ac20184..29cf3b2 100644 --- a/test/channels/hook.test.js +++ b/test/channels/hook.test.js @@ -1,4 +1,7 @@ import assert from 'node:assert/strict'; +import { createHash } from 'node:crypto'; +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; import { Readable } from 'node:stream'; import { after, before, test } from 'node:test'; @@ -19,12 +22,12 @@ let proxy; before(async () => { proxy = await fakeProxy(); }); after(() => proxy.close()); -function invoke(hostEvent, payload, { env = proxy.env, raw } = {}) { +function invoke(hostEvent, payload, { env = proxy.env, raw, stderr } = {}) { return new Promise((resolve) => { let written = ''; const stdout = { write: (text, done) => { written += text; done(); } }; const stdin = Readable.from([raw ?? JSON.stringify(payload)]); - runHookProcess('testhook', hostEvent, { stdin, stdout, env, exit: () => resolve(JSON.parse(written)), profileOf: async () => profile }); + runHookProcess('testhook', hostEvent, { stdin, stdout, stderr, env, exit: () => resolve(JSON.parse(written)), profileOf: async () => profile }); }); } @@ -44,3 +47,18 @@ test('inside the plugin\'s own headless run, a hook answers the empty reply and test('a payload that is not JSON still gets an answer', async () => { assert.deepEqual(await invoke('UserPromptSubmit', null, { raw: '{not json' }), { empty: 'UserPromptSubmit' }); }); + +test('corrupt hook state stays intact and emits a diagnostic alongside the neutral reply', async () => { + const dir = profile.stateDir(proxy.env); + const key = createHash('sha256').update('s-corrupt').digest('hex').slice(0, 16); + const file = join(dir, `testhook-session-${key}.json`); + mkdirSync(dir, { recursive: true }); + writeFileSync(file, '{"private text":'); + let diagnostic = ''; + const stderr = { write: (text, done) => { diagnostic += text; done(); } }; + + assert.deepEqual(await invoke('UserPromptSubmit', { session_id: 's-corrupt', cwd: '/proj', prompt: PROMPT }, { stderr }), { empty: 'UserPromptSubmit' }); + assert.match(diagnostic, /EINVALIDSTATE/); + assert.doesNotMatch(diagnostic, /private text/); + assert.equal(readFileSync(file, 'utf8'), '{"private text":'); +}); diff --git a/test/runtime/file-stores.test.js b/test/runtime/file-stores.test.js index 7f52193..0f56c33 100644 --- a/test/runtime/file-stores.test.js +++ b/test/runtime/file-stores.test.js @@ -1,11 +1,13 @@ import assert from 'node:assert/strict'; import { spawn } from 'node:child_process'; -import { mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import crypto from 'node:crypto'; +import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { after, test } from 'node:test'; -import { readState } from '../../lib/runtime/store/file-lock.js'; +import { readState, updateState } from '../../lib/runtime/store/file-lock.js'; import { fileStoresOf } from '../../lib/runtime/store/file-stores.js'; const dirs = []; @@ -42,6 +44,232 @@ test('the outbox takes queued reports and keeps what each delivery attempt settl assert.equal(seen[0].sentAt, 5); }); +test('an unreadable or invalid draft table is preserved and never passed to an update as empty', async t => { + for (const failure of ['EBUSY', 'EACCES', 'invalid-json', 'wrong-shape']) { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const original = failure === 'invalid-json' ? '[unfinished' : failure === 'wrong-shape' ? '{}' : '[{"title":"keep this draft"}]'; + writeFileSync(target, original); + const originalRead = fs.readFileSync; + t.mock.method(fs, 'readFileSync', (file, ...args) => { + if (file === target && ['EBUSY', 'EACCES'].includes(failure)) throw Object.assign(new Error('fixture read denial'), { code: failure }); + return originalRead(file, ...args); + }); + let mutated = false; + await assert.rejects(cursor.drafts.update(drafts => { mutated = true; return [...drafts, { title: 'new draft' }]; }), failure); + assert.equal(mutated, false, failure); + t.mock.restoreAll(); + assert.equal(fs.readFileSync(target, 'utf8'), original, failure); + assert.equal(existsSync(`${target}.lock`), false, failure); + } + const { stores: cursor } = stores(); + assert.deepEqual(await cursor.drafts.read(), [], 'an absent table is still a fresh store'); + await cursor.drafts.update(() => [{ title: 'first draft' }]); + assert.deepEqual(await cursor.drafts.read(), [{ title: 'first draft' }]); +}); + +test('a corrupt draft file is preserved for every packaged host', async () => { + for (const agent of readdirSync(new URL('../../hosts/', import.meta.url))) { + if (!existsSync(new URL(`../../hosts/${agent}/profile.js`, import.meta.url))) continue; + const { dir, stores: host } = stores(agent); + const target = join(dir, `${agent}-distill-drafts.json`); + writeFileSync(target, '[unfinished'); + await assert.rejects(host.drafts.update(() => assert.fail('mutator must not run')), { code: 'EINVALIDSTATE' }); + assert.equal(fs.readFileSync(target, 'utf8'), '[unfinished', agent); + } +}); + +test('a refused state lock preserves drafts and never runs their mutator', async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const lock = `${target}.lock`; + const original = '[{"title":"keep this draft"}]'; + writeFileSync(target, original); + const write = fs.writeFileSync; + t.mock.method(fs, 'writeFileSync', (file, ...args) => { + if (file === lock) throw Object.assign(new Error('fixture lock creation denial'), { code: 'EROFS' }); + return write(file, ...args); + }); + let mutations = 0; + try { + await assert.rejects(cursor.drafts.update(drafts => { mutations += 1; return [...drafts, { title: 'new draft' }]; }), { code: 'ELOCKED' }); + } finally { + t.mock.restoreAll(); + } + assert.equal(mutations, 0); + assert.equal(fs.readFileSync(target, 'utf8'), original); + assert.deepEqual(readdirSync(dir), ['cursor-distill-drafts.json']); +}); + +test('a held live state lock preserves drafts and never runs their mutator after waiting', async () => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const lock = `${target}.lock`; + const original = '[{"title":"keep this draft"}]'; + const held = `${process.pid}:fixture-live-owner`; + writeFileSync(target, original); + writeFileSync(lock, held); + let mutations = 0; + await assert.rejects(cursor.drafts.update(drafts => { mutations += 1; return [...drafts, { title: 'new draft' }]; }), { code: 'ELOCKED' }); + assert.equal(mutations, 0); + assert.equal(fs.readFileSync(target, 'utf8'), original); + assert.equal(fs.readFileSync(lock, 'utf8'), held); +}); + +for (const closeFails of [false, true]) test(`a partial temporary state write preserves drafts and cleans up before a retry${closeFails ? ' even when close also fails' : ''}`, async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const previous = [{ title: 'keep this draft' }]; + await cursor.drafts.update(() => previous); + const original = fs.readFileSync(target, 'utf8'); + const temporary = file => typeof file === 'string' && file.startsWith(target + '.') && file.endsWith('.tmp'); + const descriptors = new Set(); + const open = fs.openSync, write = fs.writeFileSync, close = fs.closeSync; + t.mock.method(fs, 'openSync', (file, ...args) => { + const descriptor = open(file, ...args); + if (temporary(file)) descriptors.add(descriptor); + return descriptor; + }); + let failed = false; + let closeFailed = false; + const writeFailure = Object.assign(new Error('fixture storage exhausted after a partial write'), { code: 'ENOSPC' }); + if (closeFails) t.mock.method(fs, 'closeSync', descriptor => { + const owned = descriptors.delete(descriptor); + const result = close(descriptor); + if (owned) { + closeFailed = true; + throw Object.assign(new Error('fixture close failure after closing the descriptor'), { code: 'EIO' }); + } + return result; + }); + t.mock.method(fs, 'writeFileSync', (file, ...args) => { + if (!failed && (temporary(file) || descriptors.has(file))) { + failed = true; + write(file, '[{"partial":', ...args.slice(1)); + throw writeFailure; + } + return write(file, ...args); + }); + try { + await assert.rejects(cursor.drafts.update(drafts => [...drafts, { title: 'new draft' }]), error => { + assert.equal(error, writeFailure); + assert.equal(error.code, 'ENOSPC'); + return true; + }); + } finally { + t.mock.restoreAll(); + } + assert.equal(failed, true); + assert.equal(closeFailed, closeFails); + assert.equal(fs.readFileSync(target, 'utf8'), original); + assert.deepEqual(readdirSync(dir), ['cursor-distill-drafts.json']); + await cursor.drafts.update(drafts => [...drafts, { title: 'retry succeeded' }]); + assert.deepEqual(await cursor.drafts.read(), [...previous, { title: 'retry succeeded' }]); +}); + +test('a temporary descriptor close failure preserves drafts without replacing the target and permits a retry', async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const previous = [{ title: 'keep this draft' }]; + await cursor.drafts.update(() => previous); + const original = fs.readFileSync(target, 'utf8'); + const descriptors = new Set(); + const open = fs.openSync, close = fs.closeSync, rename = fs.renameSync; + t.mock.method(fs, 'openSync', (file, ...args) => { + const descriptor = open(file, ...args); + if (typeof file === 'string' && file.startsWith(target + '.') && file.endsWith('.tmp')) descriptors.add(descriptor); + return descriptor; + }); + let failed = false; + t.mock.method(fs, 'closeSync', descriptor => { + const owned = descriptors.delete(descriptor); + const result = close(descriptor); + if (owned) { + failed = true; + throw Object.assign(new Error('fixture close failure after closing the descriptor'), { code: 'EIO' }); + } + return result; + }); + let replacements = 0; + t.mock.method(fs, 'renameSync', (from, to) => { + if (to === target) replacements += 1; + return rename(from, to); + }); + try { + await assert.rejects(cursor.drafts.update(drafts => [...drafts, { title: 'new draft' }]), { code: 'EIO' }); + } finally { + t.mock.restoreAll(); + } + assert.equal(failed, true); + assert.equal(replacements, 0); + assert.equal(fs.readFileSync(target, 'utf8'), original); + assert.deepEqual(readdirSync(dir), ['cursor-distill-drafts.json']); + await cursor.drafts.update(drafts => [...drafts, { title: 'retry succeeded' }]); + assert.deepEqual(await cursor.drafts.read(), [...previous, { title: 'retry succeeded' }]); +}); + +test('a temporary file collision preserves the existing file and releases the state lock', async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + await cursor.drafts.update(() => [{ title: 'keep this draft' }]); + const original = fs.readFileSync(target, 'utf8'); + const suffix = '010203040506'; + const collision = `${target}.${process.pid}.${suffix}.tmp`; + const collisionContents = 'another invocation owns these bytes'; + writeFileSync(collision, collisionContents); + const random = crypto.randomBytes; + t.mock.method(crypto, 'randomBytes', (size, ...args) => size === 6 ? Buffer.from(suffix, 'hex') : random(size, ...args)); + try { + await assert.rejects(cursor.drafts.update(drafts => [...drafts, { title: 'new draft' }]), { code: 'EEXIST' }); + } finally { + t.mock.restoreAll(); + } + assert.equal(fs.readFileSync(target, 'utf8'), original); + assert.equal(fs.readFileSync(collision, 'utf8'), collisionContents); + assert.equal(existsSync(`${target}.lock`), false); + await cursor.drafts.update(drafts => [...drafts, { title: 'retry succeeded' }]); + assert.deepEqual(await cursor.drafts.read(), [{ title: 'keep this draft' }, { title: 'retry succeeded' }]); + assert.equal(fs.readFileSync(collision, 'utf8'), collisionContents); +}); + +test('a failed draft replacement preserves the previous file, releases its lock and removes its temporary file', async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + await cursor.drafts.update(() => [{ title: 'keep this draft' }]); + const before = fs.readFileSync(target, 'utf8'); + const rename = fs.renameSync; + t.mock.method(fs, 'renameSync', (from, to) => { + if (to === target) throw Object.assign(new Error('fixture replacement denial'), { code: 'EPERM' }); + return rename(from, to); + }); + await assert.rejects(cursor.drafts.update(drafts => [...drafts, { title: 'new draft' }]), { code: 'EPERM' }); + t.mock.restoreAll(); + assert.equal(fs.readFileSync(target, 'utf8'), before); + assert.deepEqual(readdirSync(dir), ['cursor-distill-drafts.json']); + await cursor.drafts.update(drafts => [...drafts, { title: 'retry succeeded' }]); + assert.deepEqual(await cursor.drafts.read(), [{ title: 'keep this draft' }, { title: 'retry succeeded' }]); +}); + +test('a transient replacement denial retries while readers can still see the previous drafts', async t => { + const { dir, stores: cursor } = stores(); + const target = join(dir, 'cursor-distill-drafts.json'); + const previous = [{ title: 'keep this draft' }]; + await cursor.drafts.update(() => previous); + const rename = fs.renameSync; + let attempts = 0; + t.mock.method(fs, 'renameSync', (from, to) => { + if (to === target) { + assert.deepEqual(JSON.parse(fs.readFileSync(target, 'utf8')), previous); + if (++attempts === 1) throw Object.assign(new Error('fixture sharing denial'), { code: 'EBUSY' }); + } + return rename(from, to); + }); + await cursor.drafts.update(drafts => [...drafts, { title: 'new draft' }]); + assert.equal(attempts, 2); + assert.deepEqual(await cursor.drafts.read(), [...previous, { title: 'new draft' }]); + assert.deepEqual(readdirSync(dir), ['cursor-distill-drafts.json']); +}); + test('a report appended after an interrupted queue write survives draining and commit', async () => { const { dir, stores: cursor } = stores(); const earlier = { report: { task_id: 'earlier', asset_id: 'a' }, sessionId: 's1' }; @@ -74,11 +302,36 @@ test('parallel processes updating one state file through the lock lose no update const lock = new URL('../../lib/runtime/store/file-lock.js', import.meta.url).href; const script = `import { updateState } from ${JSON.stringify(lock)}; for (let i = 0; i < 25; i += 1) updateState(${JSON.stringify(target)}, {}, (state) => ({ count: (state.count ?? 0) + 1 }));`; - const run = () => new Promise((resolve) => spawn(process.execPath, ['--input-type=module', '-e', script]).on('close', resolve)); - await Promise.all([run(), run(), run(), run()]); + const run = () => new Promise((resolve) => { + const child = spawn(process.execPath, ['--input-type=module', '-e', script]); + let stderr = ''; + child.stderr.on('data', chunk => { stderr += chunk; }); + child.on('close', code => resolve({ code, stderr })); + }); + const results = await Promise.all([run(), run(), run(), run()]); + assert.deepEqual(results.map(result => result.code), [0, 0, 0, 0], results.map(result => result.stderr).join('\n')); assert.equal(readState(target, {}).count, 100); }); +test('transient sharing denial while releasing a state lock leaves no orphaned lock', t => { + for (const operation of ['readFileSync', 'unlinkSync']) { + const { dir } = stores(); + const target = join(dir, 'counter.json'), lock = `${target}.lock`; + const original = fs[operation]; + let denied = false; + t.mock.method(fs, operation, (file, ...args) => { + if (file === lock && !denied) { denied = true; throw Object.assign(new Error('sharing violation'), { code: 'EPERM' }); } + return original(file, ...args); + }); + updateState(target, {}, state => ({ count: (state.count ?? 0) + 1 })); + assert.equal(denied, true); + assert.equal(existsSync(lock), false, operation); + updateState(target, {}, state => ({ count: state.count + 1 })); + assert.equal(readState(target, {}).count, 2); + t.mock.restoreAll(); + } +}); + test('a session file outlives the outbox: a session resumed within a month still counts its turns on', async () => { const dir = mkdtempSync(join(tmpdir(), 'evolver-prune-')); dirs.push(dir);