mirror of
https://github.com/ruvnet/RuVector.git
synced 2026-10-24 15:20:59 +00:00
776921b3dc
* fix(brain): UTF-8 char-boundary panics in every byte-budget truncation
`&s[..N]` on a `str` indexes by BYTES, and slicing at an offset that is
not a char boundary panics. Every place this crate bounded a
user-supplied string by a fixed byte budget was a latent panic: a title
whose Nth byte lands mid-emoji (or mid-CJK, or mid-accented-letter)
aborts the request task. This is what took down the production
`ruvbrain` service (panic in `notify_digest`'s row builder → graceful
shutdown → restart).
`str::floor_char_boundary` is still unstable, so add
`text::truncate_at_char_boundary`, which rounds the byte budget DOWN to
the nearest char boundary. Byte-budget semantics are preserved exactly:
the result is never longer than `max_bytes`, and it differs from the old
code only in the cases where the old code would have panicked.
Audited every `[..N]` in the crate, not just the reported sites. Fixed
21 call sites across routes.rs, gist.rs, trainer.rs and notify.rs,
covering both `&s[..N]` and the equally-unsafe `&s[..s.len().min(N)]`
idiom. `web_ingest::truncate` already implemented this rule correctly
but privately — it now delegates, so there is one implementation.
Left alone, verified safe by construction (documented in the ADR):
- auth.rs:32 — `HeaderValue::to_str` only succeeds on visible ASCII
- bin/local.rs — hex digests
- routes.rs wasm magic, graph.rs f64, web_ingest.rs `Vec<Page>` — not str
- every `&s[..pos]` where `pos` comes from `.find()` with an ASCII
needle, which always lands on a boundary
- notify.rs `inject_tracking` was safe but fragile; now `strip_suffix`
`notify_digest`'s row builder is extracted to `format_digest_rows` so
the exact crash site is testable without an `AppState` or a live Resend
key. Both new test groups were confirmed to FAIL against the pre-fix
code (reproducing "byte index 120 is not a char boundary; it is inside
'é'") and pass after.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* fix(brain): authenticate the pipeline-inject and /internal/* write paths
The Cloud Run service `ruvbrain` grants `roles/run.invoker` to `allUsers`
(verified read-only against ruv-dev), and no `READ_ONLY` env var is set,
so `check_read_only` was the only gate on several write endpoints —
i.e. no gate at all. Anyone on the internet could write 100 memories per
request.
Changed:
- `/v1/pipeline/inject` and `/v1/pipeline/inject/batch` now take
`AuthenticatedContributor` and are subject to the same write rate
limit as every other write route. The live `brain-pubmed-daily`
scheduler job already sends a Bearer token to the batch path, so it
is unaffected.
- `/internal/queue/push`, `/internal/queue/drain`,
`/internal/session/create` and `/internal/session/:id` now require
`BRAIN_SYSTEM_KEY` via the existing `verify_system_key` helper.
"Internal" was never a network fact: these are as publicly reachable
as `/v1/*`.
- `internal_session_create` is capped at `MAX_SSE_SESSIONS` (1024).
Each call permanently added an entry to two DashMaps plus an mpsc
channel, and the draining task only cleans up once the sender drops
— but the sender lives in `state.sessions`, so uncapped `create`
calls were an unbounded memory leak. Re-creating an existing id is
still allowed so a reconnecting proxy is never locked out.
- `ruvbrain-sse` sends the system key on its three `/internal/*` calls.
- `delete_memory_as(.., system)` lets a `BRAIN_SYSTEM_KEY` holder remove
pipeline rows. Injected rows are owned by `pipeline:{source}`, which
no 32-hex-char pseudonym can equal, so they were previously
undeletable through the API by anyone.
DEPLOY DEPENDENCY: `ruvbrain-sse` must have `BRAIN_SYSTEM_KEY` in its
environment before this reaches production. It does not today. Deploying
the API without that breaks the SSE transport.
Deliberately NOT changed, with reasons recorded in the ADR:
- `/v1/pipeline/pubsub` — the live `brain-inject-push` subscription
sends a Google OIDC JWT, which is far over the contributor
extractor's 256-byte ceiling; applying that extractor would reject
every legitimate push. The correct fix is in-handler OIDC
verification. The misleading comment claiming Cloud Run validates the
token (it does not, because of `allUsers`) is corrected in place.
- `/v1/email/inbound` (Resend/Svix signature) and `/v1/chat/google`
(Google Chat issuer JWT) — both need a webhook secret / audience that
does not exist in the service config, so the right value cannot be
determined from here.
- The length-only API-key check in auth.rs is untouched; it is a
separate design decision.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* perf(brain): take the CSR rebuild off the graph write lock in ranked_search
`ranked_search` needed `&mut self` for exactly one reason: `ensure_csr`.
Everything else it touches, including `pagerank_scores`, was already
`&self`. So the search path took `state.graph.write()` — and the comment
there claimed the lock was held "briefly".
It was not. Every inject sets `csr_dirty`, so the next search rebuilt the
CSR while holding a write lock on the whole graph. In production that is
~1.2M edges, and it excluded every concurrent reader for the duration.
This is one of the three independent mechanisms behind the 2026-09-19
504s.
The CSR cache now lives behind its own `RwLock<Option<Arc<CsrMatrix>>>`
with an `AtomicBool` dirty flag, so `ensure_csr`, `rebuild_csr`,
`ranked_search` and `pagerank_search` all take `&self` and the route uses
`state.graph.read()`. A `csr_build` mutex double-checks the flag so a
burst of concurrent searches after an inject performs one rebuild, not
one per thread. `rebuild_csr` builds the matrix before taking the cache
write lock, so readers only ever wait for the pointer swap.
STALENESS: none. The rebuild is still synchronous with respect to the
search that triggers it, so a search never runs against a stale CSR.
Decoupling further — rebuilding in the background and letting searches
run one inject behind — was considered and deliberately NOT done: it
trades a correctness property for latency we have not measured, and the
lock-contention problem is already solved without it. Writers still wait
for in-flight readers; that is the remaining, much smaller, interaction.
No speedup figure is claimed here. The change is justified by the lock
structure, not by a benchmark, and none was run.
Tests: two readers hold shared read guards and search concurrently (this
fails to COMPILE if `ranked_search` ever regresses to `&mut self` —
verified by reverting the signature); a search immediately after an
insert sees the new node, proving the lazy rebuild still has no staleness
window; repeated searches are stable.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* perf(brain): build the cold-start sparsifier without holding the graph lock
`main.rs` wrapped `graph.write().rebuild_sparsifier()` in `spawn_blocking`.
That is easy to read as "already mitigated" and it is not: `spawn_blocking`
keeps the build off the two tokio worker threads, but the write guard is
taken INSIDE the closure and held for the whole build, so every reader
still stalls behind it. The live graph is ~1.2M edges — inside the
100k..5M band that triggers this — so it runs on every cold start, 60s
in. The comment directly above it already predicted this 504; the fix
applied at the time addressed the wrong half of the problem.
`rebuild_sparsifier` is now split into three pieces:
- `sparsifier_snapshot(&self)` — takes the COO entries plus node/edge
counts under a read lock;
- `build_sparsifier_from(entries, n)` — an associated fn over plain
data. No `self`, so no lock can be held across it. This is what runs
in `spawn_blocking`;
- `install_sparsifier(&mut self, ..)` — a brief write lock at the end.
The install is guarded. `add_memory` only feeds the sparsifier while it
is `Some`, and it is `None` for the entire build window, so edges
appended during the build would otherwise be silently dropped — they are
replayed on install. And if the graph SHRANK during the build,
`remove_memory` has reassigned every node position, so the built matrix
refers to the wrong nodes; the install refuses rather than corrupting
analytics. Both behaviours have tests that were verified to fail when the
guard and the replay are removed.
The >5M skip is left as-is. Its stated reason (a write lock held across
the build) no longer applies, but the CPU cost at that size has not been
measured here, so widening the band would be a guess. Noted in the ADR.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* perf(brain): make list_memories proportional to the page, not the corpus
`list_memories` cloned every matching `BrainMemory`, sorted all of them,
and then discarded all but `limit`. `BrainMemory` carries a 384-dim
embedding plus several Strings, so `limit=5` bounded the response and
nothing else — the work scaled with the corpus.
Now it collects 24-byte `(key, Uuid)` pairs, partitions with
`select_nth_unstable_by` at `offset + limit`, sorts only that prefix, and
clones only the rows actually returned.
Ordering is unchanged apart from being strictly more deterministic: rows
with equal sort keys were previously ordered by DashMap iteration order,
which can differ between two identical requests. They now tie-break on
id. A test asserts the new pagination matches a full-sort reference
across all three sort modes and seven limit/offset combinations —
including `offset` past the end — on data built with heavy tie density,
and a second test asserts repeated identical requests return identical
pages. The first was verified to fail against an off-by-one in the
partition point.
No speedup figure is claimed; no benchmark was run. The justification is
the asymptotic shape of the work and the removal of the per-row clone.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* docs(adr): ADR-348 — pi.ruv.io 504 mechanisms, UTF-8 crash, write-path auth
Records the three INDEPENDENT 504 mechanisms behind the 2026-09-19
incident (search taking a graph write lock, the cold-start sparsifier
holding one for its whole build, list_memories scaling with the corpus),
why the existing `spawn_blocking` on the sparsifier reads as a fix and
is not, the per-endpoint authentication decisions and the three
deliberately deferred ones, the CSR staleness trade-off and why it was
declined rather than accepted, and the ~3-month deploy gap.
Numbering was checked across all refs first: ADR-346 is claimed by eight
in-flight branches and ADR-347 by another, so 348 was the first free
number.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* fix(brain): guard sparsifier install on an index generation, not just counts
Follow-up to the off-lock sparsifier build. The first version of the
install guard compared node/edge counts, and that is not sufficient —
two live paths reshuffle node positions at equal or greater count and
would have sailed straight through it:
- `remove_memory(X)` followed by any `add_memory` during the build
window restores both counts while every index past X has shifted;
- `rebuild_from_batch` reassigns EVERY position even for an identical
memory set, because it iterates a DashMap whose order is arbitrary.
`GRAPH_AUTO_REBUILD=true` in production, and `routes.rs` calls
`rebuild_sparsifier` inline right after it on the small-graph path —
so the background install could also overwrite a freshly correct
sparsifier with an older one.
Adds `index_generation: u64`, bumped wherever `node_index` is rebuilt
rather than appended to, returned from `sparsifier_snapshot` and compared
for equality on install. Also refuses when a sparsifier is already
installed.
Three new tests, each verified to fail without the generation check:
remove-plus-add with restored counts, a full `rebuild_from_batch`, and
the overwrite case. The shrink and overwrite tests still pass without it,
confirming the guards cover distinct cases rather than overlapping.
ADR-348 updated: the P3 section previously claimed two guarded failure
modes, which was not true of the shipped code. Also corrected there —
the P0 site count (22, verified by grepping the diff, not 21), the
`UpdatedAt` key now comparing microseconds rather than nanoseconds, write
rate limiting recorded as an addition beyond the brief with the measured
headroom, `verify_system_key`'s fail-open semantics named as load-bearing
for the SSE deploy, the drain endpoint's empty-array-over-401
debuggability trade, and the Pub/Sub OIDC audience (the subscription sets
none, so it is the push URL) which is no longer an open question.
Co-Authored-By: RuFlo <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_018CvN6AQ5ChzjSQ42g8YbMn
* docs(adr): renumber ruvbrain 504 ADR to 349; record M4 graph-rebuild gap; SSE deploy secret
- ADR-348 -> ADR-349: 347 (PR #989) and 348 (PR #946, TwinKV) were claimed
first. INDEX.md gets one row and next number 350 (not regenerated).
- ADR-349 "Deliberately not decided here" #6: the O(n^2) rebuild_from_batch
under graph.write() (rebuild_graph optimize action via the brain-graph and
brain-full-optimize scheduler jobs, plus the post-hydration cold-start
rebuild) is the dominant 504 signature in prod logs and is not fixed here.
- deploy_brain_services.sh: ruvbrain-sse now gets BRAIN_SYSTEM_KEY, which the
API requires on /internal/* after this branch.
Co-Authored-By: claude-flow <ruv@ruv.net>
73 lines
2.5 KiB
Bash
Executable file
73 lines
2.5 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
# Deploy ADR-130 brain service split
|
|
# Usage: ./scripts/deploy_brain_services.sh [api|sse|worker|all]
|
|
|
|
PROJECT=ruv-dev
|
|
REGION=us-central1
|
|
|
|
deploy_api() {
|
|
echo "=== Deploying ruvbrain-api ==="
|
|
gcloud builds submit --config=crates/mcp-brain-server/cloudbuild.yaml --project=$PROJECT
|
|
gcloud run deploy ruvbrain \
|
|
--image=gcr.io/$PROJECT/ruvbrain:latest \
|
|
--region=$REGION \
|
|
--memory=4Gi --cpu=2 \
|
|
--concurrency=80 --max-instances=15 --min-instances=1 \
|
|
--timeout=300 --session-affinity \
|
|
--allow-unauthenticated
|
|
}
|
|
|
|
deploy_sse() {
|
|
echo "=== Deploying ruvbrain-sse ==="
|
|
# Build SSE image
|
|
gcloud builds submit \
|
|
--config=crates/mcp-brain-server/cloudbuild-sse.yaml \
|
|
--project=$PROJECT
|
|
# Deploy SSE service
|
|
gcloud run deploy ruvbrain-sse \
|
|
--image=gcr.io/$PROJECT/ruvbrain-sse:latest \
|
|
--region=$REGION \
|
|
--memory=512Mi --cpu=1 \
|
|
--concurrency=500 --max-instances=10 --min-instances=0 \
|
|
--timeout=3600 --session-affinity \
|
|
--allow-unauthenticated \
|
|
--set-env-vars="BRAIN_API_URL=https://ruvbrain-HASH.us-central1.run.app,RUST_LOG=info" \
|
|
--set-secrets=BRAIN_SYSTEM_KEY=BRAIN_SYSTEM_KEY:latest
|
|
# Note: BRAIN_API_URL needs the actual Cloud Run URL of ruvbrain-api
|
|
# BRAIN_SYSTEM_KEY is required (ADR-349): the API gates /internal/* on it.
|
|
# Without it the SSE stream connects but never delivers MCP responses.
|
|
}
|
|
|
|
deploy_worker() {
|
|
echo "=== Deploying ruvbrain-worker ==="
|
|
# Build worker image
|
|
gcloud builds submit \
|
|
--config=crates/mcp-brain-server/cloudbuild-worker.yaml \
|
|
--project=$PROJECT
|
|
# Create Cloud Run Job (not a service)
|
|
gcloud run jobs create ruvbrain-worker \
|
|
--image=gcr.io/$PROJECT/ruvbrain-worker:latest \
|
|
--region=$REGION \
|
|
--memory=4Gi --cpu=2 \
|
|
--max-retries=1 --task-timeout=3600s \
|
|
--set-env-vars="RUST_LOG=info" \
|
|
--set-secrets=BRAIN_API_KEY=brain-api-key:latest,BRAIN_SIGNING_KEY=brain-signing-key:latest \
|
|
2>/dev/null || \
|
|
gcloud run jobs update ruvbrain-worker \
|
|
--image=gcr.io/$PROJECT/ruvbrain-worker:latest \
|
|
--region=$REGION \
|
|
--memory=4Gi --cpu=2
|
|
}
|
|
|
|
# Parse argument
|
|
case "${1:-all}" in
|
|
api) deploy_api ;;
|
|
sse) deploy_sse ;;
|
|
worker) deploy_worker ;;
|
|
all) deploy_api && deploy_sse && deploy_worker ;;
|
|
*) echo "Usage: $0 [api|sse|worker|all]"; exit 1 ;;
|
|
esac
|
|
|
|
echo "=== Done ==="
|