fix(rvf): version-gate + pid-guard the corrupt-lock detector (retires on aqe 3.12.3) - #31
Merged
Merged
Conversation
… on aqe 3.12.3) 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.
This was referenced Jul 18, 2026
pacphi
added a commit
that referenced
this pull request
Jul 18, 2026
…ing, docs audit (#35) * feat(sync): live progress tickers; close the statusline footer wipe; strip retired RVF lock scan - output: withProgress elapsed-time ticker (TTY-only, injectable seam) wrapped around every slow managed heal — the ~512 MB brain KB refresh previously sat silent for 4m25s with the prompt frozen - statusline: root cause of the recurring footer wipe identified — @claude-flow/cli's version-stamped helper auto-refresh pristine-copies statusline.cjs on the FIRST ruflo command after an upgrade (observed: daemon start + statusline.cjs + .helpers-version mtimes matching at 12:20:35). fixStatusline now triggers that refresh first (refreshRufloHelpers, subprocess, alsoRefreshGlobal), then injects onto the freshly-stamped copy; helperStampStale lets status flag the armed wipe before it fires. Verified live: footer survives ruflo commands - rvf: strip the version-gated legacy corrupt-lock branch (PR #31's remnant) — dead code now that sync keeps aqe >= 3.12.3 everywhere. Only the oversized backstop (#495 runaway append) remains; the FLVR/SFVR history stays in the module header as a guard against reintroducing bytes-on-disk corruption heuristics - tests: helper-stamp (9), output-progress (5), rvf rewritten for the reduced contract (8) — 125 kit tests green * fix: remediate 4-lens brutal-honesty review findings (PR #34 wave 1) Adversarial review (Linus/security/Ramsay/Bach lenses, independent agents) over the branch diff. Everything actionable, remediated with proof: Ramsay (test quality): - F1: the refresh-before-inject ordering test used a sentinel-only fake that passed BOTH orderings; the fake now models the real pristine-copy wipe. Mutation-verified: moving the refresh after inject turns the test RED - F2: the ticker never ticked under test — mocked clock now drives the interval, minute rollover (1m01s), and real caller labels - F3/L2: refresh child now exits nonzero on rejection AND on upstream's resolved {blocked} (signature gate) — true means ran-unblocked; hung-child timeout injectable and tested - F4: v-prefixed stamp no longer arms a permanent false-stale (latent bug); empty/garbage stamps pinned stale-by-design (self-correcting) - F5: cap boundary (=== excluded, +1 flagged), only-.rvf candidacy pinned Linus (correctness): - scanRvf per-entry stat guard: a store renamed mid-scan by the aqe daemon no longer crashes ak status/sync (dangling-symlink test pins the race) - staleness-oracle divergence: rufloCliVersion gains a createRequire resolver fallback, and the sync statusline step now also runs on providers-only plans (any ruflo-invoking heal is a potential wiper) - statusline step wrapped in withProgress; ticker stdout invariant documented; stale "corrupt/oversized" wording in x/verify fixed Bach (claims): - sync no longer claims victory when the helper stamp is still stale after the heal (silent upstream-API no-op now warns instead of false success) - rvf.mjs header rationale corrected: the stripped branch was reachable (status/--no-upgrade/offline), removed because its false-positive data-loss risk outweighed its protection — not because it was dead code Security: clean (empirical: argv-safe subprocess, no shell, upstream Ed25519 gate preserved; rmSync does not follow symlinks). One LOW disclosure (alsoRefreshGlobal touches ~/.claude/helpers) → PR body. 137 tests green (was 125 at review start). * fix: remediate holistic full-source review findings (wave 2) Second adversarial wave: 4 independent agents (correctness/security/ coverage/docs-vs-reality) over all 35 source files, templates, bin, docs. Correctness (holistic-linus): - nativeSlots regression metric counted agentdb DIRECTORIES, not native bindings — the native→WASM backslide it exists to catch was invisible, and a benign tree reshape faked an alarm. Now counts native bindings (incl. the aqe slot) - provider scope resolved by cwd-only .git probe: any run from a repo SUBDIR silently wrote ENABLE_*/AQE_LLM_PROVIDER into machine-wide user settings (and undo, run at the root, couldn't find them) while the aqe router/dual-agent gates skipped. All three gates now share a bounded repo-root walk (paths.repoRoot), project files anchor at the root - upsertBlock/stripBlock ran an orphaned BEGIN "to end-of-file" — one corrupt sentinel deleted everything below it in ~/.claude/CLAUDE.md. Missing END now means append-fresh/leave-untouched; syncBlocks takes a one-time .bak like every other writer - brain plugin-version fallback sorted lexically (0.9.0 > 0.10.0) → semver Security (holistic-security; 2 LOW, 4 INFO, rest cleared with evidence): - daemon reap confirms the live pid's cmdline is a ruflo daemon before SIGTERM (pidfile pid-reuse guard; POSIX ps, unconfirmable → never kill) - dashboard rejects non-loopback Host headers (DNS-rebinding read guard) - improvement-eval execSync shell strings → execFileSync argv (the one violation of the exec.mjs binding rule); setup.mjs SQL now uses bound params; .mcp.json deleted only when mcpServers was its sole content Docs-vs-reality (holistic-bach): - --no-security was INERT: documented, persisted, read by nothing. Now gates the aidefence heal in setup, the status security row (info, no fix), and sync's security branch - dead updateHost/hostDrift removed (drift rides versions.mjs driftReport) - KB size claims corrected to ~2 GB (disk truth: 1.9 GB); bin verify list gains harvest; README uninstall row shows the real flags Coverage (holistic-ramsay): daemons.mjs was the only zero-coverage module that kills processes — now pinned (kill selection, dead-pid degradation, pid-reuse guard incl. a live spawned "daemon start" child, sweep parsing). Deferred with eyes open: mcp.mjs deny-rule tests, uninstall.mjs fixture tests, heal.mjs untested tail, dashboard client-esc DOM test. 149 tests green (was 137). * docs: full documentation audit — accuracy, dead references, GitHub alerts Walked every .md in the repo (37 files; none exceeds 1500 lines, so no decomposition was warranted; docs/archive/* audited as frozen history, content untouched by policy). Accuracy (claims corrected to disk/code truth): - TROUBLESHOOTING: RVF row no longer teaches the debunked FLVR-corruption story — oversized-only guard, aqe >= 3.12.3 self-heal cited (aqe #563), historical note explains .corrupt-<pid> droppings; statusline row now names the real wiper (helper auto-refresh on the first ruflo command after an upgrade) and the refresh-then-inject heal; adds the --no-security row and the harvest verify suite - KB size ~512 MB → ~2 GB everywhere (disk truth: 1.9 GB) — MAINTAINER, README, setup help, heal/ruvnet-brain comments - ruflo-reference stamp 3.28.x/2026-07-14 → 3.32.x/2026-07-18; dead `install.sh` attributions in three templates → ak-era commands - project CLAUDE.md Build & Test: stale single-file command → pnpm test / pnpm run check (the real gates) - archive index: living-docs pointers referenced two files that do not exist (BACKGROUND.md, CONDITIONAL-BLOCKS.md — both are archived); three "survives in bin//shell/" cells updated to v4 paths Readability/labeling: GitHub alert syntax ([!IMPORTANT]/[!TIP]/[!NOTE]/ [!WARNING]) on the load-bearing callouts in README, TROUBLESHOOTING, PROVIDERS (incl. the new repo-root scope note). markdownlint clean. Also: branch renamed feat/sync-progress-statusline-convergence → feat/ux-hardening-docs-audit (PR #34 retargeted); drops the unused cmpVersions import stranded by the wave-2 dead-code removal. * fix(ci): pid-reuse guard killed the Windows test runner — make it real on Windows isDaemonProcess() returned true unconditionally on win32 ("no cheap sync probe"), so the pid-reuse guard test — which feeds reap() the test runner's own live pid to prove non-daemons are never killed — SIGTERM'd the runner itself on windows-latest, failing daemons.test.mjs at file level across the whole Node matrix (macOS/Linux green). Fix the cause, not the test: the guard now probes the cmdline on Windows too, via the same Win32_Process CIM query the process sweep already trusts (pid numeric-coerced into the filter; unconfirmable → never kill). The guard test now runs on every platform; only the kill-true path stays POSIX-only (CI spawn/CIM timing, not a behavior difference).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Our RVF corrupt-lock detector (
src/lib/rvf.mjs) flagged any.rvf.lockwhose first four bytes areFLVRand then deleted the store beside it. Measured against@ruvector/rvf-node0.1.8 — and confirmed across every RVF lock on the dev machine (12/12) — that signal is unsound:.rvf.lock(incl. next to healthy stores)FLVR.rvfstoreSFVRFLVRis the normal lock-record magic (the store's own magic isSFVR); 162 bytes is a valid empty store. So the detector was firing on "a lock exists" and could delete a healthy store — includingbrain.rvf, a dual-write target that can hold writes never flushed tomemory.db(real data loss). This is the red herring aqe #564 calls out, independently reproduced here.The real fix is upstream
aqe 3.12.3 (#564) makes exports atomic (tmp+rename) and self-heals genuinely-unusable stores on the next run — non-destructively, 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 the kit should defer to aqe, not second-guess it.
What this PR does
aqeSelfHealsRvfgate). A 3.12.3 prerelease sorts below the release and keeps the legacy scan — the safe direction. No release-day code change needed; it flips on version detection.Verified on the installed 3.12.2: gate reads
false→ scan ACTIVE (pid-guarded); it flips to RETIRED at 3.12.3.Scope note
The residual pre-3.12.3 false positive — a stale lock beside a healthy populated store — is pre-existing and moot on upgrade. Fully fixing it would require the native open+create probe (out of scope); the real fix is adopting 3.12.3. Tracked in #30, which also carries the release-adoption checklist (delete the now-dead branch once aqe ≥ 3.12.3 is in).
Tests
tests/kit/rvf.test.mjs— 12 hermetic tests (syntheticFLVRlock records + injected version/pid/cap; no aqe install, no binding, no network): version gate across 3.12.2 / 3.12.3 / 3.13.0 / prerelease / null; stale-lock flagged pre-fix; ignored post-retirement; live-peer never flagged; non-FLVR ignored; oversized survives retirement; missing dir safe. The retirement gate and live-peer guard both fail on the old logic, verified by reverting.Gates: 112 mjs + 43 cjs tests pass, eslint 0, typecheck 0.
Closes part of #30.