From 8f80de5975345fc31e6ce3e8cb5460023e1680df Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:19:35 -0400 Subject: [PATCH] fix(ledger): retain lock descriptors across ownership checks and release --- .../commands/context-budget.md | 2 +- manifests/context-packs/skill-triggers@1.json | 2 +- skills/story-lifecycle/scripts/ledger.js | 36 ++++++++++++------- tests/docs/story-lifecycle-ledger.test.js | 28 +++++++++++++++ tests/lib/command-plugin-root.test.js | 4 +-- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/legacy-command-shims/commands/context-budget.md b/legacy-command-shims/commands/context-budget.md index c96d9161c..0c241d8c9 100644 --- a/legacy-command-shims/commands/context-budget.md +++ b/legacy-command-shims/commands/context-budget.md @@ -19,6 +19,6 @@ $ARGUMENTS Apply the `context-budget` skill. - Without an explicit audit request, return the harness's native live-context command and do not scan files or call tools. -- Treat `--audit`, “audit installed components,” or equivalent unambiguous audit wording as opt-in. `--verbose` alone stays on the native live-context path. +- Treat an explicit `--audit`, “audit installed components,” or equivalent unambiguous audit wording as opt-in. `--verbose` alone stays on the native live-context path. - Pass through `--audit` and `--verbose` only when the user supplied them. - Keep any explicit audit bounded to the scope and safety rules in the canonical skill. diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 9aedd318d..3deeb9379 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "5dd7259b7c6706ef2d6223330654a7dd1bf84893655fddb3d003a56b35b8c18a", + "registryDigest": "abe7be71a90fe080fee4e253b9ed30a258c9e051122f49803aa9633309846e59", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/story-lifecycle/scripts/ledger.js b/skills/story-lifecycle/scripts/ledger.js index 303e08d0a..833b2d1be 100644 --- a/skills/story-lifecycle/scripts/ledger.js +++ b/skills/story-lifecycle/scripts/ledger.js @@ -43,14 +43,17 @@ function sameFile(file, expected) { } // Bind writes to an exclusively created descriptor, never a checked path. -function writeNew(file, content) { +function writeNew(file, content, { keepOpen = false } = {}) { const fd = fs.openSync(file, fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, 0o600); + let retained = false; try { fs.writeFileSync(fd, content); fs.fsyncSync(fd); - return identity(fs.fstatSync(fd)); - } finally { fs.closeSync(fd); } + const owner = identity(fs.fstatSync(fd)); + if (keepOpen) { owner.fd = fd; retained = true; } + return owner; + } finally { if (!retained) fs.closeSync(fd); } } function readSnapshot(file, encoding) { @@ -113,14 +116,16 @@ function registryDir(root, dir) { } } -function readLock(dir) { - let fd; +function readLock(dir, { keepOpen = false } = {}) { + let fd, retained = false; 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); } + const holder = { ...JSON.parse(fs.readFileSync(fd, 'utf8')), owner: identity(stat) }; + if (keepOpen) { holder.owner.fd = fd; retained = true; } + return holder; + } catch { return null; } finally { if (fd !== undefined && !retained) fs.closeSync(fd); } } function isStale(holder) { return holder.host === os.hostname() && !pidAlive(holder.pid); } @@ -130,19 +135,22 @@ 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; + let owner, holder; try { - owner = writeNew(brk, String(process.pid)); + owner = writeNew(brk, String(process.pid), { keepOpen: true }); } catch (err) { if (err.code === 'EEXIST') return false; throw err; } try { - const holder = readLock(dir); + holder = readLock(dir, { keepOpen: true }); if (holder && isStale(holder)) removeOwned(path.join(dir, '.lock'), holder.owner); return true; } finally { - removeOwned(brk, owner); + try { removeOwned(brk, owner); } finally { + fs.closeSync(owner.fd); + if (holder?.owner.fd !== undefined) fs.closeSync(holder.owner.fd); + } } } @@ -151,8 +159,10 @@ function acquireLock(dir, waitMs) { const deadline = Date.now() + waitMs; for (;;) { try { - const owner = writeNew(lock, JSON.stringify({ pid: process.pid, host: os.hostname(), at: new Date().toISOString() })); - return () => removeOwned(lock, owner); + // A live descriptor keeps this inode allocated, even if a foreign writer + // replaces the pathname. dev/ino alone can alias after an early close. + const owner = writeNew(lock, JSON.stringify({ pid: process.pid, host: os.hostname(), at: new Date().toISOString() }), { keepOpen: true }); + return () => { try { removeOwned(lock, owner); } finally { fs.closeSync(owner.fd); } }; } catch (err) { if (err.code !== 'EEXIST') throw err; } diff --git a/tests/docs/story-lifecycle-ledger.test.js b/tests/docs/story-lifecycle-ledger.test.js index f96bafb88..13d3edab4 100644 --- a/tests/docs/story-lifecycle-ledger.test.js +++ b/tests/docs/story-lifecycle-ledger.test.js @@ -509,6 +509,34 @@ fs.rmSync = function (p, ...args) { const result = original.call(fs, p, ...args) assert.ok(fs.readFileSync(lock, 'utf8').includes('replacement')); }], + ['lock release survives inode reuse after an early descriptor close', () => { + const root = newRepo(); + const { input } = createStories(root, ['Inode owner']); + const lock = path.join(root, '.stories', '.lock'); + const preload = path.join(root, 'lock-inode-reuse.js'); + fs.writeFileSync(preload, `const fs = require('fs'); let fd, oldStat, closed = false, replaced = false; +const open = fs.openSync; fs.openSync = function (p, flags, ...a) { + const result = open.call(fs, p, flags, ...a); + if (String(p) === ${JSON.stringify(lock)} && (flags & fs.constants.O_EXCL)) { + fd = result; oldStat = fs.fstatSync(result); closed = false; + } return result; }; +const close = fs.closeSync; fs.closeSync = function (value) { + if (value === fd) closed = true; return close.call(fs, value); }; +const remove = fs.rmSync; fs.rmSync = function (p, ...a) { + const result = remove.call(fs, p, ...a); + 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 stat = fs.lstatSync; fs.lstatSync = function (p, ...a) { + const result = stat.call(fs, p, ...a); + return replaced && closed && String(p) === ${JSON.stringify(lock)} ? oldStat : 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']); diff --git a/tests/lib/command-plugin-root.test.js b/tests/lib/command-plugin-root.test.js index 133528a03..93f7390ce 100644 --- a/tests/lib/command-plugin-root.test.js +++ b/tests/lib/command-plugin-root.test.js @@ -115,8 +115,8 @@ for (const name of ['auto-update', 'instinct-status', 'skill-health', 'skill-hos const selected = { ...process.env, HOME: temp, USERPROFILE: temp, CLAUDE_CONFIG_DIR: root }; delete selected.CLAUDE_PLUGIN_ROOT; for (const env of [selected, { ...selected, CLAUDE_PLUGIN_ROOT: root }]) { - const result = spawnSync('bash', ['-c', 'source /dev/stdin; printf "%s" "$ECC_ROOT"'], - { input: assignment + '\n', env, cwd: temp, encoding: 'utf8' }); + const result = spawnSync('bash', ['-s'], + { input: assignment + '\nprintf "%s" "$ECC_ROOT"\n', env, cwd: temp, encoding: 'utf8' }); assert.strictEqual(result.status, 0, result.stderr); assert.strictEqual(result.stderr, ''); assert.strictEqual(result.stdout, root);