From e8850da5f1cf2e4009e241e29e2971a7cd51f318 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Fri, 17 Jul 2026 08:14:40 -0700 Subject: [PATCH] fix(rvf): version-gate + pid-guard the corrupt-lock detector (retires on aqe 3.12.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our RVF corrupt-lock detector fired on any `.rvf.lock` whose first four bytes are `FLVR`, then DELETED the store beside it. Measured against @ruvector/rvf-node 0.1.8 and confirmed across every RVF lock on the dev machine (12/12), that signal is unsound: `FLVR` is the NORMAL lock-record magic (the store's own magic is `SFVR`), so every lock matches — healthy or not — and a 162-byte store is a valid EMPTY store, not a truncated one. The detector was effectively firing on "a lock exists" and could delete a healthy store, including `brain.rvf`, a dual-write target holding writes never flushed to memory.db (real data loss). aqe PR #564 (shipping in 3.12.3) fixes the actual bug (#563) at the root: exports become atomic (tmp+rename) and genuinely-unusable stores self-heal on the next run, non-destructively and pid-guarded. The sound corruption signal is "open fails AND create fails", which needs the native binding — the kit can't observe it from the filesystem. So: - Retire the corrupt-lock scan automatically once installed aqe >= 3.12.3 (aqeSelfHealsRvf gate; a 3.12.3 prerelease sorts below the release and keeps the legacy scan, the safe direction). - Until then, guard the legacy scan with the same pid-liveness check aqe added: only a STALE lock (dead owner pid) can trip it, so it can never quarantine a store a live process is holding — closes that hazard today, on 3.12.2, not just after the upgrade. - Keep the oversized backstop (#495 runaway-append mode) — a different, still-sound signal. - Status/heal wording no longer claims "rebuilt from memory.db"; it points at aqe 3.12.3 self-heal. The residual pre-3.12.3 false positive (a stale lock beside a healthy POPULATED store) is pre-existing and moot on upgrade; the sound fix is adopting 3.12.3, tracked separately. 12 hermetic tests (synthetic FLVR lock records + injected version/pid/cap; no aqe install, no binding, no network). The retirement gate and live-peer guard both fail on the old logic, verified by reverting. --- src/commands/status.mjs | 7 ++- src/lib/heal.mjs | 4 +- src/lib/rvf.mjs | 92 ++++++++++++++++++++++++++----- tests/kit/rvf.test.mjs | 118 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+), 17 deletions(-) create mode 100644 tests/kit/rvf.test.mjs diff --git a/src/commands/status.mjs b/src/commands/status.mjs index 0ff4e6eb..d61c121a 100644 --- a/src/commands/status.mjs +++ b/src/commands/status.mjs @@ -164,9 +164,12 @@ export async function collect({ pkgRoot, cwd = process.cwd() }) { if (fs.existsSync(aqeDir)) { const findings = scanRvf(aqeDir); if (findings.length) { + // On aqe < 3.12.3 only: a stale-lock or oversized artifact aqe can't yet + // self-heal. aqe >= 3.12.3 makes scanRvf return nothing here (it repairs + // its own stores non-destructively), so this row goes quiet on upgrade. rows.push(row('aqe', 'fail', - `${findings.length} corrupt/oversized RVF artifact(s) — aqe will drop OFF ruvector (FsyncFailed)`, - 'sync quarantines them (rebuilt from memory.db)')); + `${findings.length} stale/oversized RVF artifact(s) — aqe will drop OFF ruvector (FsyncFailed)`, + 'sync quarantines them (aqe 3.12.3 self-heals this)')); } else { rows.push(row('aqe', 'ok', 'agentic-qe initialized here; RVF store healthy')); } diff --git a/src/lib/heal.mjs b/src/lib/heal.mjs index 51296349..3a486bbe 100644 --- a/src/lib/heal.mjs +++ b/src/lib/heal.mjs @@ -104,7 +104,9 @@ export async function healAqeSolver() { return { ok: true, detail: r.code === 0 ? 'installed' : 'unavailable (TS fallback is fine <50K nodes)' }; } -/** Quarantine corrupt/oversized RVF artifacts in a project. */ +/** Quarantine stale-lock / oversized RVF artifacts in a project. No-op once the + * installed aqe self-heals its own stores (>= 3.12.3): scanRvf returns nothing, + * so this reports healthy without touching disk. See src/lib/rvf.mjs. */ export function healRvf(projectAqeDir) { const findings = scanRvf(projectAqeDir); const removed = findings.flatMap((f) => quarantine(f)); diff --git a/src/lib/rvf.mjs b/src/lib/rvf.mjs index 5718adf5..3952a6a1 100644 --- a/src/lib/rvf.mjs +++ b/src/lib/rvf.mjs @@ -1,28 +1,92 @@ -// RVF pattern-store health: the two corruption modes the shell kit repairs. -// 1. FLVR-magic .rvf.lock — store bytes written into the lock by an -// interrupted write (aqe does NOT self-heal this; FsyncFailed on init). -// 2. Oversized .rvf (runaway append after a hard exit; seen at ~277GB). -// Stores are derived caches (rebuilt from memory.db) — quarantine = delete. +// RVF pattern-store health — the corruption modes the kit repaired before +// agentic-qe learned to heal itself. +// +// HISTORY (issue #563 → aqe PR #564, shipped in 3.12.3). The kit used to flag a +// `.rvf.lock` whose first four bytes are `FLVR` as a "corrupt lock" and delete +// the store next to it. Measured against @ruvector/rvf-node 0.1.8 — and +// confirmed across every RVF lock on the dev machine — that signal is UNSOUND: +// · `FLVR` is the NORMAL lock-record magic (the store's own magic is `SFVR`), +// so every lock on disk matches, healthy or not; +// · a 162-byte store is a VALID EMPTY store, not the "truncated header" the +// issue reported. +// The only sound signal for this corruption is "open fails AND create fails", +// which needs the native binding — the kit cannot observe it from the +// filesystem. So the old detector was really firing on "a lock exists", then +// deleting a store that may have been perfectly healthy (and, for `brain.rvf`, +// a dual-write target holding writes never flushed to memory.db — real data). +// +// aqe >= 3.12.3 makes exports atomic (tmp+rename) and self-heals genuinely +// unusable stores on the next run, non-destructively (quarantine-aside, not +// delete), gated on the lock's owner pid. Once that version is installed the +// kit stops second-guessing it: the corrupt-lock scan RETIRES itself and only +// the oversized backstop (a different mode — #495 runaway append, seen ~277 GB) +// remains. Until then the legacy scan stays active but is now guarded by the +// same pid-liveness check aqe added, so it can never quarantine a store a LIVE +// process is holding. Tracking the upgrade: pacphi/agentic-kit follow-up issue. import fs from 'node:fs'; import path from 'node:path'; +import { installedVersion, cmpVersions } from './versions.mjs'; -const CAP_BYTES = Number(process.env.RUFLO_AQE_RVF_MAX_BYTES ?? 2 * 1024 ** 3); +// First agentic-qe release with atomic RVF export + self-healing stores (#564). +const AQE_SELFHEAL_VERSION = '3.12.3'; -export function scanRvf(dir) { +const defaultCapBytes = () => Number(process.env.RUFLO_AQE_RVF_MAX_BYTES ?? 2 * 1024 ** 3); + +/** True once the installed agentic-qe atomically exports and self-heals RVF + * stores (>= 3.12.3), making the kit's corrupt-lock quarantine both redundant + * and unsound to keep. A prerelease of the target (3.12.3-rc.1) sorts below + * the release and so keeps the legacy scan active — the safe direction. */ +export function aqeSelfHealsRvf(installed = installedVersion('agentic-qe')) { + return !!installed && cmpVersions(installed, AQE_SELFHEAL_VERSION) >= 0; +} + +/** True when `pid` names a running process (signal 0 probes without delivering). + * EPERM = exists but owned by another user → still alive. */ +function isPidAlive(pid) { + try { process.kill(pid, 0); return true; } + catch (e) { return e.code === 'EPERM'; } +} + +/** The owner pid recorded in an rvf-node 0.1.8 lock record — `FLVR` magic then + * the pid as u32 LE at offset 4 — or null if the file is not a readable lock. */ +function readLockPid(file) { + let fd; + try { + fd = fs.openSync(file, 'r'); + const buf = Buffer.alloc(8); + if (fs.readSync(fd, buf, 0, 8, 0) < 8) return null; + if (buf.subarray(0, 4).toString('latin1') !== 'FLVR') return null; + const pid = buf.readUInt32LE(4); + return pid > 0 ? pid : null; + } catch { + return null; + } finally { + if (fd !== undefined) { try { fs.closeSync(fd); } catch { /* best-effort */ } } + } +} + +/** Scan a project's `.agentic-qe/` for RVF artifacts the kit should quarantine. + * `selfHeals`/`capBytes`/`isAlive` are injectable for hermetic tests. */ +export function scanRvf(dir, { + selfHeals = aqeSelfHealsRvf(), + capBytes = defaultCapBytes(), + isAlive = isPidAlive, +} = {}) { const findings = []; if (!fs.existsSync(dir)) return findings; for (const name of fs.readdirSync(dir)) { const file = path.join(dir, name); - if (name.endsWith('.rvf.lock')) { - const fd = fs.openSync(file, 'r'); - const buf = Buffer.alloc(4); - try { fs.readSync(fd, buf, 0, 4, 0); } finally { fs.closeSync(fd); } - if (buf.toString('latin1') === 'FLVR') { + if (!selfHeals && name.endsWith('.rvf.lock')) { + // Legacy pre-3.12.3 path (see file header). Only a STALE lock — one whose + // owner pid is gone — can trip this; a live owner means a store in use, + // never ours to quarantine. On 3.12.3+ this branch is skipped entirely. + const pid = readLockPid(file); + if (pid !== null && !isAlive(pid)) { findings.push({ kind: 'corrupt-lock', file, sibling: file.replace(/\.lock$/, '') }); } - } else if (name.endsWith('.rvf') && CAP_BYTES > 0) { + } else if (name.endsWith('.rvf') && capBytes > 0) { const size = fs.statSync(file).size; - if (size > CAP_BYTES) findings.push({ kind: 'oversized', file, size }); + if (size > capBytes) findings.push({ kind: 'oversized', file, size }); } } return findings; diff --git a/tests/kit/rvf.test.mjs b/tests/kit/rvf.test.mjs new file mode 100644 index 00000000..6dc5c3ae --- /dev/null +++ b/tests/kit/rvf.test.mjs @@ -0,0 +1,118 @@ +// scanRvf / aqeSelfHealsRvf — the RVF corrupt-lock detector and its +// version-gated retirement (issue #563 → aqe PR #564). Hermetic: a synthetic +// .agentic-qe fixture with hand-built lock records + injected version/pid/cap, +// so no aqe install, no rvf-node binding, no network. +// +// The load-bearing facts, measured against @ruvector/rvf-node 0.1.8 and +// confirmed on the dev machine: a lock record is `FLVR` magic + owner pid as +// u32 LE at offset 4; the store's own magic is `SFVR`; an empty store is 162 +// bytes. The old detector fired on the mere presence of an `FLVR` lock, which +// is EVERY lock — the retirement + pid-guard fix that. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { scanRvf, aqeSelfHealsRvf } from '../../src/lib/rvf.mjs'; + +const DEAD_PID = 0x7fffffff; // no process; process.kill(_, 0) throws ESRCH → dead + +function aqeDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'ak-rvf-')); +} + +/** Write an rvf-node-shaped lock: 4-byte 'FLVR' magic, pid u32 LE at offset 4. */ +function writeLock(dir, name, pid) { + const buf = Buffer.alloc(104); + buf.write('FLVR', 0, 'latin1'); + buf.writeUInt32LE(pid, 4); + fs.writeFileSync(path.join(dir, name), buf); +} + +/** Write a store file of `size` bytes with the real `SFVR` store magic. */ +function writeStore(dir, name, size) { + const buf = Buffer.alloc(size); + buf.write('SFVR', 0, 'latin1'); + fs.writeFileSync(path.join(dir, name), buf); +} + +const kinds = (findings) => findings.map((f) => f.kind).sort(); + +// ── aqeSelfHealsRvf: the retirement gate ──────────────────────────────────── + +test('aqeSelfHealsRvf is false below 3.12.3 — the legacy scan stays active', () => { + assert.equal(aqeSelfHealsRvf('3.12.2'), false); +}); + +test('aqeSelfHealsRvf is true at exactly 3.12.3 — the scan retires', () => { + assert.equal(aqeSelfHealsRvf('3.12.3'), true); +}); + +test('aqeSelfHealsRvf is true above 3.12.3', () => { + assert.equal(aqeSelfHealsRvf('3.13.0'), true); +}); + +test('aqeSelfHealsRvf treats a 3.12.3 prerelease as not-yet-healed (safe direction)', () => { + assert.equal(aqeSelfHealsRvf('3.12.3-rc.1'), false); +}); + +test('aqeSelfHealsRvf is false when no aqe version resolves', () => { + assert.equal(aqeSelfHealsRvf(null), false); +}); + +// ── scanRvf: corrupt-lock detection, pre- and post-fix ────────────────────── + +test('a stale FLVR lock is flagged on aqe < 3.12.3 (legacy behavior preserved)', () => { + const dir = aqeDir(); + writeStore(dir, 'brain.rvf', 162); + writeLock(dir, 'brain.rvf.lock', DEAD_PID); + const findings = scanRvf(dir, { selfHeals: false }); + assert.deepEqual(kinds(findings), ['corrupt-lock']); + assert.equal(findings[0].sibling, path.join(dir, 'brain.rvf')); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('the same stale FLVR lock is IGNORED once aqe self-heals (>= 3.12.3)', () => { + const dir = aqeDir(); + writeStore(dir, 'brain.rvf', 162); + writeLock(dir, 'brain.rvf.lock', DEAD_PID); + assert.deepEqual(scanRvf(dir, { selfHeals: true }), []); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('a lock held by a LIVE process is never flagged — no quarantining a peer in use', () => { + const dir = aqeDir(); + writeStore(dir, 'patterns.rvf', 162); + writeLock(dir, 'patterns.rvf.lock', process.pid); // this test process is alive + assert.deepEqual(scanRvf(dir, { selfHeals: false }), []); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('a non-FLVR .rvf.lock is not treated as a lock record', () => { + const dir = aqeDir(); + fs.writeFileSync(path.join(dir, 'brain.rvf.lock'), Buffer.from('not-a-lock-record')); + assert.deepEqual(scanRvf(dir, { selfHeals: false }), []); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +// ── scanRvf: the oversized backstop (a different mode; survives retirement) ── + +test('an oversized store is flagged even after the corrupt-lock scan retires', () => { + const dir = aqeDir(); + writeStore(dir, 'patterns.rvf', 4096); + const findings = scanRvf(dir, { selfHeals: true, capBytes: 1024 }); + assert.deepEqual(kinds(findings), ['oversized']); + assert.equal(findings[0].size, 4096); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('a normal-sized store is not flagged', () => { + const dir = aqeDir(); + writeStore(dir, 'patterns.rvf', 162); + assert.deepEqual(scanRvf(dir, { selfHeals: true, capBytes: 2 * 1024 ** 3 }), []); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('scanRvf on a missing .agentic-qe dir returns empty, never throws', () => { + assert.deepEqual(scanRvf(path.join(os.tmpdir(), 'ak-rvf-nope-123'), { selfHeals: false }), []); +});