Skip to content

Cleanup/hardening: fix five open correctness/security defects, adopt ADR-340 invariants - #933

Draft
ruvnet wants to merge 2 commits into
mainfrom
claude/ruvector-cleanup-hardening-m2ax54
Draft

Cleanup/hardening: fix five open correctness/security defects, adopt ADR-340 invariants#933
ruvnet wants to merge 2 commits into
mainfrom
claude/ruvector-cleanup-hardening-m2ax54

Conversation

@ruvnet

@ruvnet ruvnet commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

One hardening pass over the open defect backlog: five issues fixed with regression tests, the underlying invariants recorded as ADR-340, 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

#825ruvector-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 while prune_neighbors re-locked the same node through self.distance(). parking_lot locks are not reentrant, so the inserting thread deadlocked — but only when a neighbor list overflowed m0, which the unseeded test RNG made nondeterministic. Reproduced deterministically (seeded, 400 inserts, dim 8 — hung pre-fix, <1s post-fix).

  • 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 (strictly-decreasing distance ⇒ ≤ nodes.len() moves) so a malformed graph terminates loudly.
  • Tests seeded; prune-heavy regression test added; ruvector-delta-index rejoins the core-platform CI shard (checklist item from ruvector-delta-index test_insert_and_search hangs indefinitely (DeltaHnsw insert/search) #825).

#901ruvector-tiny-dancer-core: NaN confidence panicking route()

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_cmp as defense-in-depth. Regression test poisons the model weights with NaN (the issue's corrupted-model scenario) and asserts error-not-panic.

#907ruvector-graph: strong-Arc pool (unlinked-database reuse + unbounded growth)

Ported ruvector-core's post-#902 reference shape wholesale: Weak per-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.

#908ruvector-context: require_private_root owner check

The root must now be owned by the process euid in addition to being 0700, via rustix::process::geteuid() (no unsafe, already in Cargo.lock through fs4 — one line of lockfile churn, no new transitive dependency). As the issue argued, this rejects only roots that already failed later with a bare EACCES.

#903 — harness: locale-dependent benchmark cache key

canonical() in benchmark.ts now sorts keys by code unit (RFC 8785), with the Intl.Collator-stub regression test copied from the #890 fix shape. The same defect in research.ts (feeding sha256/embeddingSpaceId digests) is fixed in the same way.

⚠ One-time cache invalidation (called out per the issue): changing the ordering changes the benchmark cache key, so existing .metaharness/cache entries no longer match and benchmarks recompute once. This is the accepted cost of a locale-independent key and the reason this change ships in its own commit path rather than riding a feature PR.

ADR-340 — 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. docs/adr/INDEX.md updated minimally (generator's full regeneration churns every row's git date on a fresh clone; node scripts/adr-index.mjs --check passes).

Follow-up issues filed from this pass

Validation

  • cargo test -p ruvector-tiny-dancer-core — 49 passed
  • cargo test -p ruvector-context — 44 passed
  • cargo test -p ruvector-delta-index — 15 passed (pre-fix: hung; suite now <1s)
  • cargo test -p ruvector-graph — full suite green
  • cargo clippy over the four touched crates — clean (pre-existing workspace warnings only)
  • harness: npm test — 167 pass / 0 fail (the one prior failure was a missing local Python jsonschema, unrelated)
  • node scripts/adr-index.mjs --check — OK, next available ADR-341

🤖 Generated with claude-flow

https://claude.ai/code/session_01L5ffi8NiKAQNabK3D5t2gK


Generated by Claude Code

claude and others added 2 commits August 26, 2026 02:45
…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

ruvnet commented Aug 26, 2026

Copy link
Copy Markdown
Owner Author

CI note: Build Tiny Dancer linux-arm64-musl is red here, and the failure is not this PR's.

It is the known toolchain drift from #900: the link of third-party redb fails with unsupported linker arg: --fix-cortex-a53-843419 before any first-party code is reached. This PR surfaces it only because it touches crates/ruvector-tiny-dancer-core/, which re-triggers the path-filtered workflow that hadn't run since its last green pass on 2026-08-03.

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 aarch64-unknown-linux-musl, and no zig release accepts it — verified against zig 0.13.0, 0.14.1, 0.15.2, and 0.16.0. The failure reproduces with a minimal cdylib link on 1.98.0 + zig 0.13.0 and disappears on 1.97.0 with the same zig.

The fix is #934 (pin toolchain: "1.97.0" beside the existing zig 0.13.0 pin), kept as the separate CI PR that #900 explicitly asked for rather than folded into this diff. Once #934 merges, this check goes green here on re-run; every other failure signal on this PR (the initial Rustfmt red) is already fixed on the current head.


Generated by Claude Code

ruvnet commented Aug 26, 2026

Copy link
Copy Markdown
Owner Author

CI note: Tests (core-and-rest) was cancelled at the 240-minute cap, and this failure is not this PR's.

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 ruvector-temporal-tensor-wasm, 16/17 other jobs green) — filed the day before this PR existed. This PR's diff adds only test code and small fixes to crates in that shard, nothing that moves a four-hour compile wall.

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 — core-platform (which now includes ruvector-delta-index again, passing in 7 minutes), core-and-rest-heavy, core-and-rest-wasm, both Clippy jobs, Rustfmt, and the harness job all pass on this head. The two remaining reds are both pre-existing and tracked: this one (#928) and Tiny Dancer arm64-musl (#900, fix in #934).


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment