Cleanup/hardening: fix five open correctness/security defects, adopt ADR-341 invariants - #933
Cleanup/hardening: fix five open correctness/security defects, adopt ADR-341 invariants#933ruvnet wants to merge 3 commits into
Conversation
…nts (ADR-340) Fixes five open defects as one hardening pass, and records the underlying invariants in ADR-340 so the defect classes stop recurring: - #901 ruvector-tiny-dancer-core: route() now rejects non-finite model output at a single choke point on BOTH the gated and ungated paths (tripping the circuit breaker, matching the VoI path's contract), and the decision sort uses total_cmp as defense-in-depth. Regression test poisons the model weights with NaN and asserts an error, not a panic. - #825 ruvector-delta-index: DeltaHnsw::connect_node's prune branch held a write lock on a neighbor while re-locking the same node through self.distance() — parking_lot locks are not reentrant, so inserts deadlocked whenever a neighbor list overflowed m0 (nondeterministic under the unseeded test RNG; burned a 3h52m CI job). The neighbor's vector now comes from the held guard via a split borrow, prune_neighbors drops self-loops before measuring distances, greedy_search carries an explicit progress bound, and the tests are seeded. A prune-heavy regression test (400 inserts, dim 8) completes in <1s. ruvector-delta-index rejoins the core-platform CI shard. - #907 ruvector-graph: the redb pool now holds Weak per-path slots with Drop-under-guard eviction and slot reaping, ported from ruvector-core's post-#902 reference shape. Regression tests: erased-path recreation never reuses the unlinked database; reopening after last drop preserves data and reaps the pool entry; 40-iteration barrier-synchronized concurrent drop-vs-open probe with zero tolerated failures. - #908 ruvector-context: require_private_root now requires the root to be owned by the process euid (rustix::process::geteuid — no unsafe, no new transitive dependency) in addition to mode 0700. Rejects only roots that already failed later with bare EACCES. - #903 harness: canonical() in benchmark.ts and research.ts sorts keys by code unit (RFC 8785) instead of localeCompare, making cache keys and digests locale-independent. NOTE: this changes the benchmark cache key, invalidating existing .metaharness/cache entries once (documented in the issue as the accepted cost). Regression test stubs Intl.Collator with a reversed comparator and asserts the encoding is unchanged. ADR-340 records the six invariants (total float orderings, no lock re-entry, Weak+Drop pool shape, identity-not-just-mode trust checks, code-unit canonical encodings, seeded nondeterministic tests). Closes #825, #901, #903, #907, #908. Co-Authored-By: claude-flow <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01L5ffi8NiKAQNabK3D5t2gK
Co-Authored-By: claude-flow <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01L5ffi8NiKAQNabK3D5t2gK
|
CI note: It is the known toolchain drift from #900: the link of third-party Root cause is now pinned down empirically (details in the fix PR): stable 1.98.0 (2026-08-18) began emitting the Cortex-A53 erratum flag for The fix is #934 (pin Generated by Claude Code |
|
CI note: It is exactly the pre-existing #928 compile-size problem: the job ran 02:53→06:53 UTC and was killed at the cap during compilation, the same outcome #928 documents on PR #926's run 32702332738 (cancelled mid-compile of Not re-running: the cap-out already reproduced on an independent PR (#926, per #928), so a re-run would spend another four runner-hours to confirm what is already confirmed. Not fixing here: #928 explicitly reserves the shard split/caching change for maintainer review as a CI-policy decision, so porting a workflow change into this PR would preempt that. Signal coverage note: everything this PR actually changes is tested by green shards — Generated by Claude Code |
Main allocated ADR-340 to the signed retrieval-receipt anchoring ADR (PR #949) while this branch held the number for the correctness-hardening invariants ADR. Main merged first, so its allocation stands: this branch's ADR renumbers to ADR-341, INDEX.md keeps main's ADR-340 row and gains the ADR-341 row (next available: 342; the numbering guard passes). Also corrects two comments main's graph-node Cypher fix added citing 'ADR-340 invariant 1' for the NaN-comparison rule — that content lives in the hardening ADR, which is now ADR-341. No semantic conflicts: main's ruvector-graph changes touch cypher/parser.rs and graph.rs only, not the storage pool this branch reworked; full ruvector-graph suite passes on the merge (264 tests). Co-Authored-By: claude-flow <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01L5ffi8NiKAQNabK3D5t2gK
Summary
One hardening pass over the open defect backlog: five issues fixed with regression tests, the underlying invariants recorded as ADR-341, and three new issues filed for defects found during the work (#930, #931, #932).
Closes #825, closes #901, closes #903, closes #907, closes #908.
Fixes
#825 —
ruvector-delta-index: DeltaHnsw insert hang (root-caused: self-deadlock)The hang was not an unterminated traversal:
connect_node's prune branch held a write lock on a neighbor node whileprune_neighborsre-locked the same node throughself.distance(). parking_lot locks are not reentrant, so the inserting thread deadlocked — but only when a neighbor list overflowedm0, which the unseeded test RNG made nondeterministic. Reproduced deterministically (seeded, 400 inserts, dim 8 — hung pre-fix, <1s post-fix).prune_neighborsdrops self-loops before measuring distances.greedy_searchcarries an explicit progress bound (strictly-decreasing distance ⇒ ≤nodes.len()moves) so a malformed graph terminates loudly.ruvector-delta-indexrejoins thecore-platformCI shard (checklist item from ruvector-delta-index test_insert_and_search hangs indefinitely (DeltaHnsw insert/search) #825) — and passed there in 7 minutes on this PR's runs.#901 —
ruvector-tiny-dancer-core: NaN confidence panickingroute()Non-finite score/uncertainty now rejected at a single choke point covering both the gated and ungated paths, tripping the circuit breaker (matching the VoI path's contract); the decision sort uses
total_cmpas defense-in-depth. Regression test poisons the model weights with NaN (the issue's corrupted-model scenario) and asserts error-not-panic.#907 —
ruvector-graph: strong-Arc pool (unlinked-database reuse + unbounded growth)Ported
ruvector-core's post-#902 reference shape wholesale:Weakper-path slots,Drop-under-the-slot-guard eviction, slot reaping. Regression tests: erased-path recreation observes an empty database; reopen-after-last-drop preserves data and reaps the pool entry; 40-iteration barrier-synchronized concurrent drop-vs-open probe, zero tolerated failures. The non-canonicalized-key tail note from #907 is split out as #932 so it survives this close.#908 —
ruvector-context:require_private_rootowner checkThe root must now be owned by the process euid in addition to being 0700, via
rustix::process::geteuid()(nounsafe, already inCargo.lockthroughfs4— one line of lockfile churn, no new transitive dependency). As the issue argued, this rejects only roots that already failed later with a bareEACCES.#903 — harness: locale-dependent benchmark cache key
canonical()inbenchmark.tsnow sorts keys by code unit (RFC 8785), with theIntl.Collator-stub regression test copied from the #890 fix shape. The same defect inresearch.ts(feedingsha256/embeddingSpaceIddigests) is fixed in the same way.ADR-341 — Correctness-Hardening Invariants for Hot-Path Primitives
Records the six invariants these fixes instantiate so the defect classes stop recurring: total float orderings on untrusted values, no lock re-entry (read through the held guard), the Weak+Drop pool shape as the only sanctioned pool pattern, identity-not-just-mode filesystem trust checks, code-unit canonical encodings for anything hashed, and seeded RNGs in tests of nondeterministically-triggered behavior.
node scripts/adr-index.mjs --checkpasses post-merge (372 ADR files, next available 342).Follow-up issues filed from this pass
DeltaHnsw::delete()of the entry point leaves search anchored to a phantom node; verified empirically: the whole graph becomes unreachable and search returns onlyid="".sort_by(partial_cmp().unwrap())) across 27 crates — NaN-reachable in core query paths (ADR-340 invariant 1) #931 — inventory of 91 remainingsort_by(partial_cmp().unwrap())sites across 27 crates, triaged user-reachable-first (ruvector-core query paths, ruvector-postgres backend-abort risk).PathBuf(two spellings of one file ⇒ redb lock error), split out of ruvector-graph: connection pool still holds strong Arcs with no eviction (unlinked-database reuse + unbounded growth) #907.CI status (see PR comments for detail)
Green everywhere the diff reaches, including
core-platformwith delta-index restored. Two pre-existing, tracked reds inherited from main, both commented on this PR:Tests (core-and-rest)caps out at 240 min compiling (#928, maintainer CI-policy decision), andBuild Tiny Dancer linux-arm64-musl(#900, toolchain drift — fix ready in #934).Validation
cargo test -p ruvector-tiny-dancer-core— 49 passedcargo test -p ruvector-context— 44 passedcargo test -p ruvector-delta-index— 15 passed (pre-fix: hung; suite now <1s)cargo test -p ruvector-graph— 264 passed post-merge (both sides' changes together)cargo clippyover the four touched crates — clean;cargo fmt --all --check— cleannpm test— 167 pass / 0 failnode scripts/adr-index.mjs --check— OK🤖 Generated with claude-flow
https://claude.ai/code/session_01L5ffi8NiKAQNabK3D5t2gK