Repository navigation
fix(cli): refuse cascade sync while a server holds the memory root - #458
Merged
Merged
Conversation
`cascade rebuild` already refuses to run next to a live server (the OME jobstore lock, exit code 3); `cascade sync` was deliberately left out in #384 because draining the queue from a second process was believed safe. The 10-hour Windows soak (two `cascade sync` processes alongside the server) shows it is not: the server reads its own LanceDB snapshot (`read_consistency_seconds` defaults to None) and cannot see what the CLI process just committed, so both decide a row is new and both insert it. After 10 h every table carried 4-5 % duplicate global ids, same md_path, same content, `updated_at` about 14 s apart. With a server running the CLI is redundant anyway — the daemon's watcher already syncs every change — so `sync` now takes the same lock probe and exits 3 with a message that says so. Without a server nothing changes. The runbook's "unlike cascade sync" sentence and the "run sync after batch edits" advice are corrected to match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adversarial review of the first commit found the guard too narrow to deliver a single writer: it probed the lock once and released it, so a server started mid-drain became a second writer anyway; `cascade fix --apply` drains through the same orchestrator and was not gated; and the refusal text claimed the server "already syncs", which is false for a server started with EVEROS_DISABLE_CASCADE=1 or one that has been quiesced. `_runtime(exclusive=True)` now acquires the OME jobstore lock — the same file and flags the engine uses — and holds it until the command's runtime tears down, so a late server fails at its own lock instead of joining in. `sync`, `fix --apply` and `rebuild` pass it; `status` and `fix` (listing) stay read-only and keep working next to a server. The refusal names the lock holder and the two exceptions, and exits 3 before anything is opened. Tests hold the real lock file the way a server does: `sync` and `fix --apply` exit 3 with `get_engine` patched to fail if reached; `fix` and `status` still run; `sync` observes the lock held during its own drain and released after. The runbook's "safe to run in parallel with a live server" paragraph, the rebuild note, the reference-file workaround and how-memory-works.md's "force the queue" advice are corrected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
0xKT
approved these changes
Sep 24, 2026
arelchan
self-requested a review
September 24, 2026 08:44
arelchan
approved these changes
Sep 24, 2026
This was referenced Sep 24, 2026
Merged
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
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.
Summary
Every cascade command that writes the index —
sync,fix --apply,rebuild— now acquires the OME jobstore lock (the same file and flagseveros serveruses) and holds it for its whole run. While a server or another exclusive CLI phase holds the root, the command exits 3 before opening anything; a server that starts mid-run fails at its own lock instead of becoming a second writer.statusandfix(listing) are read-only and keep working next to a server.Why:
syncwas deliberately exempted from the lock in #384 on the belief that draining from a second process is safe. The 10-hour Windows soak (PR #454) ran twocascade syncprocesses next to the server and ended with 1 158 / 1 219 / 1 241 duplicate global ids in episode / atomic_fact / foresight (4.3–4.6 % extra rows): samemd_path, same content,updated_at~14 s apart. Mechanism: the server's LanceDB connection keeps its own snapshot (read_consistency_seconds = None), so when another process inserts a row the server'smerge_insertmatch phase still sees "no row" and inserts it again; two insert-only transactions do not conflict in Lance and no unique-key constraint exists. No consistency interval closes that window — a single writer does.Behaviour change
everos cascade syncandeveros cascade fix --applyrefuse (exit 3) while a server holds the memory root. Forcing a path to re-index while a server runs is no longer possible from the CLI; the watcher covers edits, and an API re-enqueue endpoint would be the way to bring that back.EVEROS_DISABLE_CASCADE=1, quiesced) so the operator knows to stop it first.cascade syncprocesses next to the server,.work_context/lancedb_soak/harness) is retired by this change: every iteration exits 3. Its purpose — exercising cross-process commit races — is what this PR removes on purpose.Verification
syncandfix --applyexit 3 withget_enginepatched to fail if reached (refusal precedes any DB open);fixandstatusstill run;syncobserves the lock held during its own drain (ome_lock_is_free()is False insidesync_once) and released after. Red without the held lock (3 failed), green with it.tests/integration/test_cascade_cli_integration.py+tests/unit/test_entrypoints/test_cli+tests/unit/test_memory/test_cascade: 295 passed.make lintgreen..work_context/lancedb_soak/results/windows_run1/summary.md(local), probedup_probe.py.Docs
docs/cascade_runbook.md: thesyncsection no longer says "safe to run in parallel with a live server"; the rebuild note no longer calls rebuild "the one command not safe next to a server"; the reference-file workaround says to re-saveSKILL.md(or stop the server);docs/how-memory-works.mdno longer suggests forcing the queue withsyncfor read-your-write.Review round
An adversarial review of the first commit found: probe-and-release instead of a held lock,
fix --applyungated, the refusal claiming the server "already syncs" (false forEVEROS_DISABLE_CASCADE=1/ quiesced), a test that could not tell where the guard sat, and five doc passages still prescribingsyncnext to a server. All addressed in2660d43.🤖 Generated with Claude Code