mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-18 02:24:27 +02:00
98 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ragnor Comerford
|
bcd0d9c867
|
feat(mcp): MCP server surface — Streamable-HTTP transport + tool/resource projection (RFC-003)
Add the `omnigraph-mcp` crate (stateless Streamable-HTTP transport, `McpBackend`
seam, fail-closed Host/Origin policy) and the server backend projecting built-in
operations and the per-graph stored-query registry as MCP tools + resources over
`POST /graphs/{id}/mcp`. Every tool delegates to the same engine/handler
functions the REST routes use and is gated by the same Cedar `authorize` path;
reads/writes carry structured output.
Includes three correctness fixes from review + live testing:
- tools/list is a faithful relaxation of the per-call gate: a built-in whose
authorization depends on a caller-chosen branch is shown iff the actor could
invoke it on some branch, via PolicyEngine::permits_on_any_branch (capability
probe through the same Cedar authorizer). A fabricated-`main` probe wrongly
hid graph_mutate under the canonical "protect main, write unprotected" policy.
- The stored-query surface honors mode + `expose` on call as well as on list:
resolve_stored_tool is the single membership test, so the meta pair
(stored_query_list/stored_query_run) is callable only in `meta` mode and
stored_query_run resolves exposed-only. An `expose:false` query is unreachable
by name on the agent surface (it stays HTTP/service-callable).
- The loopback Host allow-list is the full set [127.0.0.1, ::1, localhost]
(matches rmcp's default), so an IPv6 loopback `Host: [::1]` is accepted
regardless of which stack the server bound.
The protocol-version contract is documented (initialize negotiates the version
in its body, so the MCP-Protocol-Version header is validated on non-init
requests only) and pinned by a test.
Tests: omnigraph-mcp/tests/standalone.rs, omnigraph-server/tests/mcp.rs,
omnigraph-policy permits_on_any_branch unit test, omnigraph-api-types schema
projection. Full workspace gate green.
|
||
|
Ragnor Comerford
|
c43b81d318
|
docs(rfc-013): add reader/writer scaling (§5.8) — split read fleet vs write tier
The substrate makes read/write scaling asymmetric: reads are object-store-backed, snapshot-isolated, stateless -> horizontal to N replicas with zero coordination; writes are serialized per (table,branch) by one-winner manifest CAS -> scale by partitioning (branches/graphs/cells), with a single active coordinator per (cell,graph,branch). Adds the CQRS deployment split (read fleet / write tier / maintenance / heavy reads), read-your-write via commit_id/snapshot_id pinning, and gates pooled WRITES (not reads) on closing the cross-process-CAS gap. |
||
|
Ragnor Comerford
|
0f58329ab7
|
docs(rfc-013): tenancy model — cluster-as-tenant cells, pooled compute
General server/topology/auth/deployment RFC resolving the half-built tenancy ambiguity (cluster-only server vs pooled tenant_id scaffolding). Decision: the cluster is the tenant is the cell — silo the data (own storage/catalog/ policy/tokens), pool the compute (one process : N cells). No row-level pooling (no engine RLS). - §5.1 CellRuntime lifts today's per-cluster runtime into a value. - §5.2/§5.3 AppState holds a CellRegistry; resolve_cell is one new outer middleware hop before auth; the per-graph + Cedar + MCP stack is unchanged. - §5.4 per-cell CellAuth (Static | Oidc TokenVerifier); WorkOS org -> cell 1:1 with per-cell OAuth audience (cross-tenant token replay fails on aud). - §5.5 Cedar stays per-graph/per-cell; default-deny-read becomes safe; no tenant dimension needed. - §5.6 control plane = Cell Registry (metadata only) + provisioning-as-code; cell hot-load is the one safe runtime mutation (cell-granular, not graph). - §5.7 tiered dedicated/pooled/on-prem on one binary; §7 backward-compatible (today's single-cluster server = a one-cell map). MCP (rfc-003) is one consumer, not the driver. Linked from docs/dev/index.md. |
||
|
Ragnor Comerford
|
86fbb62d12
|
docs(rfc-003): fold external review into correct-by-design fixes
An external review pass raised 8 findings; verified 7 valid (2 confirmed
against the engine coercer). Folded them in as class-closing fixes rather than
point patches:
- §9.1 (③④, the headline): the JSON-Schema generator was a second hand-written
copy of the engine's input contract — Blob (base64 vs URI string) and nullable
(explicit null) were two drifts of one class. Move the projection to a single
param_json_schema in omnigraph-api-types (next to ParamKind/ParamDescriptor),
fix Blob -> {"type":"string","format":"uri"} (query_input.rs:449 / api-types:354
say blob-URI string) and nullable -> anyOf[..,null] (query_input.rs:273,296),
and lock it to json_value_to_literal_typed with a schema/engine equivalence
test so any future drift is a CI failure.
- §7/§4 (①): replace the fail-open "empty allowed_origins => skip" with a total
OriginPolicy and a single McpHostPolicy::from_bind constructor (remote default
DenyBrowsers, enforced by origin_guard independent of rmcp's empty-list quirk).
No absent-=>-skip state can be constructed.
- §6/§12/§16 (②): make the non-paginated list seam a stated contract (Vec<T>,
no nextCursor; meta mode bounds large catalogs) and drop the pagination claims
the signature couldn't express.
- §9.3 (⑦): built-in/stored tool-name collision becomes a cluster validate/boot
error (fold built-in names into the registry uniqueness check), not a silent
skip — per the invariants deny-list.
- §9.2 (⑥): stored_query_mode folded into the one per-graph mcp: block (Phase 6),
not a floating key; not configurable until that surface exists.
- §10/§1 (⑧): scope derives from the per-graph mount; server-scoped `health`
becomes graph-scoped `graph_health` (server liveness stays REST /healthz).
- §13 (⑤, doc-only): OpenAI row corrected to the `authorization` field; Phase-1
reachability via static bearer is unchanged.
§17 records the locked decisions; the validation header notes the review pass.
|
||
|
Ragnor Comerford
|
3771a29dd4
|
docs(rfc-003): align with main after merge (cluster-only, api-types, /load)
Re-validated RFC-003 against the merged tree (RFC-009/010/011/012). Folds in
the deltas that landed on main since the RFC was written:
- §15 routing: RFC-011 made the server cluster-only. GraphRouting::Single/Multi
and the `match state.routing()` branch are gone; build_app always nests
per_graph_protected under /graphs/{graph_id}. So /mcp is always
/graphs/{id}/mcp (even a single-graph boot is a one-graph registry keyed by
`default`). §15.1's per-graph model is now the only model. Line refs repointed
to lib.rs:876/929/953/954/148.
- §9 stored queries: param/catalog DTOs moved to omnigraph-api-types (RFC-009),
re-exported via api.rs — citations repointed (lib.rs:355/373/496). The legacy
omnigraph.yaml `queries:` declaration source is removed; cluster.yaml
graphs.<id>.queries is the sole source. Flag the §D5 bridge: cluster boot
forces expose=true/tool_name=None today, so per-query expose/tool_name is a
planned phase (added to §17 deferred).
- §11 load: /ingest -> canonical POST /load (RFC-009 Phase 5); graph_load reuses
run_ingest via server_load (handlers.rs:1320), /ingest is a deprecated alias.
- Mechanical: policy/handler/identity/ApiError line-number drift repointed
(policy:16/251, handlers:313/334/711/645/913, identity:186, lib.rs:280).
No architectural change — RFC-011 confirms the per-graph model; the rest is
re-citation and the expose/tool_name caveat.
|
||
|
Ragnor Comerford
|
c08e8dbac4
|
Merge remote-tracking branch 'origin/main' into ragnorc/omnigraph-mcp-crate | ||
|
aaltshuler
|
4f8c71fa23 |
Merge remote-tracking branch 'origin/main' into ragnorc/shaping-config-integration
# Conflicts: # crates/omnigraph-cluster/src/lib.rs # crates/omnigraph-cluster/src/serve.rs # crates/omnigraph-server/src/lib.rs # crates/omnigraph-server/src/settings.rs # docs/user/clusters/config.md |
||
|
aaltshuler
|
16e4a833c0 | Wire cluster embedding providers | ||
|
Andrew Altshuler
|
7c092d3206
|
feat(cli): add read-only profile list / profile show (RFC-011 D8) (#255)
Inspect the per-operator `~/.omnigraph/config.yaml` scope profiles without running anything: - `profile list [--json]` — every profile with its binding (server/cluster/store) and default graph; marks the `$OMNIGRAPH_PROFILE`-active one. A malformed (zero/two-scope) profile is reported as `invalid: <reason>`, not a hard failure. - `profile show [<name>] [--json]` — one profile's resolved scope: binding kind + target, the resolved endpoint (a server's URL / a cluster's root / the store URI), default graph, and output format. With no name, shows the active (`$OMNIGRAPH_PROFILE`) profile, else the flat operator defaults. Both are `local` (Session plane) — they read operator config only, take no addressing flags. Display reads `OperatorProfile::binding()` + the same `servers`/`clusters` lookups the scope resolver uses (not `resolve_scope`, which is capability-gated and can't render all three binding kinds at once), so it is honest about what a profile binds. Also: RFC-011 bookkeeping (Status → Accepted; D8 shipped, D11 gated on RFC #219, D5 deferred) and drop the stale "legacy config actor (RFC-008 window)" comment in operator.rs (the legacy actor is gone). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
6a2dfa7325
|
fix: self-heal manifest-unreferenced branch forks (stop wedged branches) (#231)
* chore: correct stale global-lock comments
The global Arc<RwLock<Omnigraph>> that once serialized every server write was
removed — the server holds the engine as a lockless Arc<Omnigraph> and write
methods are &self, so the per-(table_key, branch) write queues are now the
actual write-serialization mechanism (in-process only).
Correct comments that still claimed the global lock is 'still in place' /
'today', or framed the queues as MR-686 scaffolding: write_queue.rs module doc,
exec/merge.rs, db/omnigraph/schema_apply.rs, db/manifest/recovery.rs, and the
bench_concurrent_http.rs example (which also wrongly stated mutate_as is
&mut self). workload.rs is left as-is — its 'previous global RwLock' wording is
accurate history.
* test: regression for self-healing a manifest-unreferenced fork
An interrupted first-write fork (create_branch succeeded, the manifest publish
did not) leaves a fully-formed Lance branch ref the manifest never references.
The branch stays a valid manifest branch, so cleanup's reconciler never
reclaims it, and today the next write to that table wedges with 'incomplete
prior delete; run cleanup'.
Forge that exact residue (a live 'feature' branch + a directly-created
'feature' ref on the Person table the manifest doesn't reference) and assert
the next load AND mutate self-heal. Deterministic and local — no S3 or timing,
since the forge IS the post-crash state. Adds a shared node_table_uri helper.
This commit is RED: it reproduces the bug and fails against the unfixed engine
with the predicted symptom. The fix follows in the next commit.
* fix: self-heal manifest-unreferenced branch forks
The first write to a table on a branch lazily forks it via Lance create_branch,
a durable two-phase op that advances Lance state BEFORE the atomic manifest
publish. If the writer dies or its request future is cancelled between the fork
and the publish, the branch ref is fully formed but the manifest never
references it. The next write re-enters the fork path, create_branch collides,
and the engine wedged with 'orphaned table state ... incomplete prior delete;
run cleanup' — which cleanup could not even fix, because the branch is still a
live manifest branch. This hit load, mutate, ingest, and the merge fork path
(one shared engine chokepoint), so a routine deploy restart or client
disconnect could wedge a branch.
Fix: treat the per-table fork ref as derived state of the manifest. fork_branch_
from_state returns a typed ForkOutcome instead of a human 'incomplete prior
delete' error; on RefAlreadyExists the db layer reclaims the manifest-
unreferenced fork (force_delete_branch + re-fork, exactly once) and proceeds.
A live committed fork is still routed to a retryable conflict before the fork
path, so concurrent first-writes stay correct.
Reclaim is only safe if no in-process writer can be mid-fork, so the write
entry points (load, mutate) acquire the per-(table, branch) write queues for
all touched tables up front — before the fork, held through the publish — when
forking a non-main branch. commit_all accepts these pre-held guards instead of
re-acquiring (the queue is non-re-entrant). The merge fork path already holds
the queue and self-heals through the shared wrapper. Cross-process in-flight
forks remain the documented one-winner-CAS gap.
Mechanical prep folded in: mutation IR lowering is hoisted so the touched-table
set is known before execution; commit_all gains the held_guards parameter.
Flips recreate_over_orphaned_fork_before_cleanup_is_actionable to assert
self-heal; fork_collision_with_live_concurrent_fork_is_retryable still holds.
Docs: writes.md cancelled-future note, invariants.md cross-process known gap.
* fix(cleanup): reconcile per-table manifest-unreferenced forks
reconcile_orphaned_branches keyed orphans on the branch NAME (absent from the
manifest), so it only reclaimed forks from a fully-deleted branch. A fork left
on a still-live branch by an interrupted first-write was never reclaimed — the
backstop the handoff expected cleanup to provide did not cover that case.
Broaden it to a per-table authority test: a Lance branch B on table T is an
orphan iff B is not a live manifest branch (delete-leftover) OR the manifest's
branch-B snapshot does not place T on B (interrupted first-write). Per-branch
snapshots are resolved once and cached across tables. Legitimately-forked
tables, main, and internal/system branches are never reclaimed; children are
dropped before parents to avoid Lance's referenced-parent RefConflict. The
commit-graph half stays whole-branch (per-table doesn't apply there).
This is the guaranteed-convergence backstop to the write-path self-heal: it
reclaims any fork the write path never revisits, and is what Lance's own
create_branch docstring asks embedders to provide for zombie/orphan refs.
* fix: reclaim self-validates against fresh manifest authority
The fork reclaim force-deletes a Lance branch ref, gated on the caller's proof
that the manifest does not place the table on the branch. But the first-write
path obtains that proof via snapshot_for_branch, which returns the coordinator's
CACHED snapshot when the handle is bound to the branch (an embedded handle on
the branch, or branch_merge's target swap). If that snapshot is stale and a
concurrent writer already published a legitimate fork, the reclaim would
force-delete it and re-fork from source, stranding the manifest at a version the
recreated ref no longer has.
Make the destructive primitive own its safety precondition: re-derive it from a
FRESH manifest read (fresh_snapshot_for_branch, which bypasses the cache)
immediately before force-deleting. If fresh authority shows the table is on the
branch, refuse with a retryable conflict instead of destroying a valid fork.
Correct for any caller regardless of snapshot staleness. Also stop branching on
Lance's exact RefConflict prose (loosened match; typed-variant is the durable
follow-up). Addresses PR review (Codex P1, Greptile P2).
* fix: cover delete-cascade edges in up-front fork-queue acquisition
A node delete cascades to every edge table touching that node (execute_delete_
node), forking those edge tables during execution. But touched_table_keys
derived the up-front fork-queue set from the IR ops alone (just node:Type), so a
branch delete that forks node + cascade edges held only the node queue —
commit_all then saw cascade-edge keys it had no guard for.
The touched set is a pure function of (IR ops + catalog), so compute the
COMPLETE set: op types plus, for delete-node ops, the cascade edges derived the
same way the executor derives them (from_type/to_type match). Pre-computed now
equals actual by construction.
Also promote commit_all's held-guard coverage check out of debug_assert into an
all-builds check that fails the write with a typed manifest_internal error: a
load-bearing serialization invariant must fail loudly+safely in release, not
silently proceed unguarded if a future execution path ever touches a table
outside the pre-computed set.
Adds branch_cascade_delete_forks_node_and_edges_under_held_queues, which drives
the cascade path on a branch (the gap the existing insert/load tests missed).
Addresses PR review (Cursor medium, Greptile P2).
* fix(cleanup): serialize fork reclaim against in-process live writers
The broadened per-table reconciler force_delete'd an orphan candidate on a LIVE
branch without holding the per-(table, branch) write queue. An in-process
first-write fork in its fork->publish window holds that queue and has not yet
advanced the manifest, so it looks exactly like an origin-2 orphan — concurrent
cleanup could delete the ref the writer still holds and is about to publish.
(The old branch-name-based reconciler did not have this race: a deleted branch
cannot have a live first-write.)
Bring the reconciler under the same invariant the write-path reclaim already
obeys: never force_delete a fork ref without holding the (table, branch) write
queue AND confirming, under it, from a fresh read, that the ref is still
manifest-unreferenced. Acquire one key at a time (no lock-order inversion vs
multi-table acquire_many writers); if the writer published meanwhile, the fresh
re-check sees the table on the branch and skips. Cross-process writers remain
the documented one-winner-CAS gap. Addresses PR review (Cursor high).
* fix: classify create_branch failure by ref existence, not by failure
fork_branch_from_state mapped ANY create_branch failure to RefAlreadyExists,
routing transient I/O / version / Lance-internal errors into the destructive
reclaim path and masking the real error as a retryable conflict.
Branch on the actual fact instead: on create_branch failure, check whether the
ref exists (list_branches). Only a genuinely pre-existing ref — a fully-formed
manifest-unreferenced fork — is a reclaim candidate; any other failure
propagates with fidelity. We deliberately do NOT force-delete on a not-found-ref
failure: it is indistinguishable from a transient error on a fresh create, and
force-deleting there is the overreach the fresh-authority guard already removed.
A phase-1-only Lance zombie (rarer; create_branch interrupted mid its two
internal phases) surfaces as the propagated error for manual reclaim.
Addresses PR review (Cursor medium).
* fix(cleanup): skip (not delete) on a transient re-check error for a live branch
The reconcile pre-delete re-check treated ANY fresh_snapshot error as 'still an
orphan' and proceeded to force_delete. A transient manifest read failure on a
LIVE branch could therefore destroy a fork the manifest still considers
legitimate — inconsistent with the write-path reclaim (aborts on the same error)
and the candidate scan (skips on snapshot failure).
Distinguish the two origins under the queue: a branch absent from the manifest
authority (origin 1) is a confirmed orphan and is deleted without a fresh read
(no live writer can hold a deleted branch's queue); a LIVE branch (origin 2)
gets the fresh re-check and, on a transient read error, is SKIPPED — never
destroyed on ambiguity — converging on a later cleanup. Same don't-destroy-on-
ambiguous-error principle as the create_branch failure classification.
Addresses PR review (Cursor medium).
* fix(cleanup): unify fork-ref reclaim on fresh authority under the queue
Consolidates the reconcile/reclaim hardening from PR review (the earlier per-site
commits were collapsed when reconciling with the main merge). Both destructive
fork-ref sites — the write-path reclaim and the cleanup reconciler — now share
one classifier, classify_fork_ref -> ForkRefStatus { Legitimate, Orphan,
Indeterminate }, evaluated from FRESH manifest authority under the held
(table, branch) write queue. A fork ref is destroyed ONLY on a confirmed Orphan;
a Legitimate (concurrent writer published a real fork) or Indeterminate
(transient read) status is never destroyed — the write path maps it to a
retryable conflict, cleanup maps it to skip. This closes, by construction:
- reclaim trusting a possibly-cached caller proof (Codex P1);
- reconcile racing an in-process live fork without the queue (Cursor);
- delete-on-transient-error in the re-check (Cursor/Greptile);
- origin-1 trusting a stale live_branches capture for a created-since branch
(Cursor/Greptile P1).
Having one classifier removes the duplication that let the two sites drift.
ForkOutcome is made pub to match the sealed trait method returning it. Verified
green on Lance 7.0.0 (full engine suite + 48/48 failpoints).
* test(cleanup): pin classify_fork_ref decision (Legitimate / Orphan / ghost)
Both fork-ref reclaim sites (write-path reclaim + cleanup reconciler) route
their destroy/skip decision through classify_fork_ref, but it had no direct
test — reverting the fresh-authority logic was not test-detectable. Add a
deterministic in-source unit test that forges each state and asserts the status:
a manifest-placed fork -> Legitimate (never destroyed); a ref the manifest does
not place on the branch -> Orphan; a ref for a branch absent from the manifest
-> Orphan (ghost reclaim preserved). This makes the core fresh-authority
decision behind every reclaim fix revert-detectable in one place.
(The Indeterminate arm — transient read on a live branch -> skip — needs an
injected read failure and is left to the failpoints suite; the cross-process
cleanup-vs-writer and cached-snapshot reclaim races are the documented
one-winner-CAS gap, not reachable same-process bugs, so they are not faked here.)
* test(cleanup): pin the Indeterminate (transient re-check) reclaim arm
Closes the last untested classify_fork_ref arm. Adds a 'classify.fresh_read'
failpoint (no-op without the failpoints feature) that simulates a transient
failure of the fresh-authority read, and a failpoints test driving it through
cleanup: a genuine origin-2 orphan on a LIVE branch whose fresh re-check fails
classifies as Indeterminate, so the reconciler SKIPS it (never destroys on an
inconclusive read) and reclaims it on the next run once the read succeeds.
This makes the don't-destroy-on-ambiguity rule revert-detectable end-to-end.
The only paths now left untested are the cross-process cleanup-vs-writer and
reclaim-vs-publish races — the documented one-winner-CAS gap (cleanup is
&mut self / CLI-only, so no reachable same-process race), not faked here.
* test(server): avoid stale schema apply route handle
* fix(cleanup): report indeterminate fork authority clearly
|
||
|
Ragnor Comerford
|
ffb4a2c9ab
|
fix(embedding): openai-alias api key + deadline scope (PR review)
openai-alias api key (Cursor High): key resolution was dispatched on the Provider enum, which is OpenAiCompatible for both openai and openai-compatible, so OMNIGRAPH_EMBED_PROVIDER=openai (base api.openai.com) could send OPENROUTER_API_KEY and 401. Fix: the alias match now yields the ordered key-env list too, so base, model and key are all alias-derived in one place. openai takes only OPENAI_API_KEY (errors loudly if absent); openai-compatible/unset prefer OPENROUTER_API_KEY then OPENAI_API_KEY. Closes the class, not the instance.
deadline scope (Cursor Medium): the deadline bounds every embed call (query and document), which is correct, but the name said query-only. Renamed OMNIGRAPH_EMBED_QUERY_DEADLINE_MS -> OMNIGRAPH_EMBED_DEADLINE_MS (field query_deadline_ms -> deadline_ms) and updated the doc wording so the name matches the behavior. Two new alias-key tests; 20 embedding unit tests pass.
Resolved earlier in
|
||
|
Andrew Altshuler
|
4601e5f4bf
|
feat!: delete the legacy OmnigraphConfig + config migrate; finish the omnigraph.yaml docs sweep (#252)
* refactor(cli): own ReadOutputFormat/TableCellLayout in the CLI The two output-presentation enums lived in `omnigraph-server::config` and were re-exported for the CLI, even though the server never used them. Move both definitions into `omnigraph-cli/src/read_format.rs` (where the renderer already lives) and drop them from the server's public re-export. This is a step toward deleting the legacy `omnigraph-server::config` module entirely — a CLI presentation concern has no business in the server crate. No behavior change. The server keeps private copies in `config.rs` only for the soon-to-be-deleted legacy `CliDefaults`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli)!: remove the `config migrate` command and migrate.rs `config migrate` was the last CLI consumer of the legacy `omnigraph.yaml` (`OmnigraphConfig` + `load_config`). With the excision complete there is no legacy file to split, so the whole `omnigraph config` command group is removed along with `migrate.rs`. The `OmnigraphConfig` type, `load_config`, and the deprecation machinery are deleted next. - Remove `Command::Config` / `ConfigCommand` from the clap surface and the dispatch arm; drop `mod migrate;` and the now-unused `load_config` import. - Drop the `Command::Config` arms in `planes.rs`. - Delete the `config_migrate_splits_legacy_config` integration test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(server)!: delete the legacy OmnigraphConfig type and load_config With `config migrate` gone, nothing loads `omnigraph.yaml` anymore. Delete the entire `omnigraph-server::config` module: the `OmnigraphConfig` type and its sub-structs (`ProjectConfig`, `TargetConfig`, `CliDefaults`, `ServerDefaults`, `AuthDefaults`, `QueryDefaults`, `AliasConfig`, `AliasCommand`, `PolicySettings`, `QueryEntry`, `McpSettings`), `load_config`, and the RFC-008 deprecation machinery (`OMNIGRAPH_CONFIG`, `OMNIGRAPH_NO_LEGACY_CONFIG`, `OMNIGRAPH_SUPPRESS_YAML_DEPRECATION`, the deprecation map + warner). - `QueryRegistry::load` (the only `OmnigraphConfig`/`QueryEntry` consumer; its only caller was its own test) is removed — server boot and the CLI both build registries via `QueryRegistry::from_specs`. - `graph_resource_id_for_selection` (CLI-only) moves into the CLI (`helpers.rs`), with its unit test; the server no longer exports it. - Drop the already-dead `format_registry_load_errors` helper (config-adjacent). No behavior change — every deleted item was unreachable after the excision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: purge the legacy omnigraph.yaml surface from the docs Finish the RFC-011 excision in the docs: the CLI no longer reads omnigraph.yaml and the server boots cluster-only, so every doc that described the legacy file as a live config is now wrong. - AGENTS.md: rewrite the HTTP-server line to cluster-only boot (drop the single-graph/flat-route and omnigraph.yaml-boot framing); rewrite the CLI two-surface-config passage (drop `config migrate`, the deprecation env vars, and "Never extend omnigraph.yaml"); fix the topic table + capability rows. - cli/reference.md: delete the entire "omnigraph.yaml schema (legacy combined file)" section and the `config migrate` row; re-home the `policy` row, the bearer-token chain, the actor/format/param-precedence references, and the `--config` mentions to the operator config + `--cluster`. - cli/index.md: rewrite the multi-graph-server + add-graph paragraphs to cluster (`--cluster` + `cluster apply`); fix the policy examples to `--cluster`; replace the `## Config` omnigraph.yaml example with the operator/cluster two-surface model. - operations/policy.md: rewrite per-graph-vs-server-level policy to the cluster `policies:`/`applies_to` model; re-home the actor + CLI tooling sections. - clusters/config.md, clusters/index.md, deployment.md: server boots from the cluster only; per-operator facts come from ~/.omnigraph/config.yaml. - architecture.md, testing.md: drop the stale omnigraph.yaml / deleted-test references. RFCs, design specs, and prior release notes are left as historical records. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
70ed848b9d
|
docs(embeddings): @embed model arg + same-space validation (RFC-012 Phase 3-4)
Document the optional @embed model kwarg, the query-time same-space rejection, model-string strictness, and the loud schema-apply refusal on model change. Mark RFC-012 phases 1-4 implemented. |
||
|
Ragnor Comerford
|
b183db078f
|
Index materialization is derived state: defer off the write path, reconcile via optimize (iss-848) (#246)
* test(engine): reproduce empty-table Vector @index aborting schema apply
A Vector (IVF) index trains k-means centroids over the column, so Lance
cannot build it on 0 vectors ("Creating empty vector indices with
train=False is not yet implemented"). schema apply reconciles a table's
whole index set whenever any @index on it changes, so adding an unrelated
scalar @index materializes the dormant empty vector index and aborts the
entire migration (all-or-nothing).
This regression test inits a 0-row Doc with a Vector @index, adds a scalar
@index, and asserts the apply succeeds (then loads one embedded row and
asserts the deferred index materializes). It fails today at the apply step
with the vector-index abort; the fix lands in the next commit.
Refs dev-graph iss-empty-vector-index-schema-apply, iss-848.
* fix(engine): defer Vector @index on an empty table instead of aborting schema apply
build_indices_on_dataset_for_catalog materialized a declared Vector @index
unconditionally. On a 0-row table Lance cannot train the IVF index
("Creating empty vector indices with train=False is not yet implemented"),
so any later migration that touches the table (e.g. adding an unrelated
scalar @index, which reconciles the table's whole index set) aborted the
entire migration on the dormant vector index — all-or-nothing.
Guard the vector arm with a row-count check, matching the guard
ensure_indices_for_branch and the branch-merge rebuild already use: an
untrainable column becomes a pending index that a later ensure_indices /
optimize materializes once the table has rows. Reads stay correct meanwhile
(vector search degrades to a brute-force scan).
Stop-gap: the residual rows-present-but-vectors-null window and the full
decoupling (intent recorded at apply, an idempotent coverage reconciler)
are dev-graph iss-848. Turns the green half of the regression test added in
the previous commit.
Refs dev-graph iss-empty-vector-index-schema-apply, iss-848, iss-687.
* docs(invariants): record the logical-contract-over-physical-state principle
The bug class behind the empty-table vector-index abort (and the schema-apply
vs optimize version drift) is one shape: a physical operation allowed to fail
a logical one. Several hard invariants (2, 5, 7, 13) and deny-list items are
already instances of this, but the unifying rule was never written down.
Add it to docs/dev/invariants.md as a "Governing principle" section above the
hard invariants, naming which invariants and deny-list items instantiate it
and the smell to watch for (a logical operation gated on a physical fact).
Add a one-line always-on rule (7) in AGENTS.md so it stays in working memory,
with the qualifier that genuine logical conflicts still fail loudly — the
licence to lag covers physical convergence, not correctness.
Audience-neutral: no private ticket refs. check-agents-md.sh passes.
* test(engine): index build must tolerate rows with null vectors (load-before-embed)
Loading rows whose vector column is null into a `Vector @index` table fails
today: build_indices (reached via the loader's prepare_updates_for_commit)
calls create_vector_index, and Lance's IVF KMeans errors "cannot train 1
centroids with 0 vectors". The same abort hits ensure_indices/optimize/schema
apply/merge, since they all funnel through build_indices_on_dataset_for_catalog.
This test loads two null-embedding rows and calls ensure_indices; it must not
abort (the untrainable vector column is deferred, sibling indexes still build).
Fails today at the load step; fixed in the next commit.
Refs dev-graph iss-848, iss-empty-vector-index-schema-apply.
* fix(engine): defer unbuildable index columns instead of aborting the write path
build_indices_on_dataset_for_catalog is the chokepoint every write path funnels
through (load/mutate via prepare_updates_for_commit, schema apply, ensure_indices,
optimize, branch merge). Its vector arm called create_vector_index
unconditionally, so a column with no trainable vectors yet — an empty table, or
rows loaded before `embed` populates them — aborted the whole operation with
Lance's IVF KMeans error.
Fault-isolate the vector build: on failure, record the column as a PendingIndex
(table, column, reason), log it, and continue building the sibling indexes; a
later ensure_indices/optimize materializes it once the column is trainable, and
reads use brute-force meanwhile. Manifest/CAS/IO errors at the publish boundary
still propagate. Isolating at the single chokepoint realizes the governing
principle (physical index state never fails a logical operation) for every write
path, and supersedes the earlier symptomatic count_rows==0 stop-gap (removed) —
closing the residual rows-present-but-vectors-null window it left open.
Surfacing pending index status rather than failing is the database norm
(Postgres indisvalid, LanceDB list_indices). ensure_indices and the build_indices
wrappers now return Vec<PendingIndex>; optimize surfaces it in a later commit.
Refs dev-graph iss-848, iss-951 (vector index stays inline-commit until lance#6666).
* test(engine): index-only schema apply must not touch table data
Adding an @index to an existing column should be a pure metadata change once
index materialization moves to the reconciler (iss-848): the apply records the
intent in the catalog/IR but builds nothing inline, so the table's manifest
version is unchanged. Today the indexed_tables block builds the index inline
and bumps the version (4 -> 5). Fixed in the next commit.
Refs dev-graph iss-848.
* fix(engine): schema apply records index intent only; index-only apply is metadata
Schema apply no longer builds indexes inline. The four build_indices calls
(added/renamed/rewritten/index-only tables) are removed; the @index/@key intent
is already persisted in the catalog/IR the apply writes, and the physical index
is materialized off the critical path by ensure_indices/optimize (iss-848).
Concretely:
- AddConstraint (an @index addition — every other added constraint plans as
UnsupportedChange) becomes a pure metadata step alongside the metadata-only
steps: it touches no table data, so the table version is unchanged.
- added/renamed/rewritten tables still write their data; only the trailing
index build is gone. The rewritten table's coverage is restored later by
optimize_indices.
- recovery_pins drops index-only tables (they no longer advance Lance HEAD) and
keeps rewritten tables; their post_commit_pin = expected+1 is now exact (one
rewrite commit), strengthening recovery classification.
- the now-orphaned Omnigraph::build_indices_on_dataset_for_catalog wrapper is
removed.
A migration can no longer abort on an index build, for any index type at any
cardinality. Turns the green half of index_only_constraint_apply_touches_no_table_data.
Refs dev-graph iss-848.
* test(engine): optimize must converge a declared-but-unbuilt index
After iss-848, adding an @index post-data is a metadata-only apply that defers
the physical build, so the column is declared-indexed but unbuilt (reads scan).
`optimize` — the operator's cron reconciler — must materialize it. Today optimize
only maintains coverage of EXISTING indexes (optimize_indices) and never creates
missing ones, so the rank BTREE stays Degraded after optimize. Fixed next commit.
Refs dev-graph iss-848.
* fix(engine): optimize materializes declared-but-unbuilt indexes (the reconciler)
`omnigraph optimize` is the operator's cron reconciler. It already compacts and
folds new fragments into EXISTING indexes (optimize_indices); now it also builds
declared-but-missing indexes, so the indexes schema apply / load defer (iss-848)
converge on the next optimize.
Done inside optimize_one_table (not by composing the all-tables ensure_indices,
which is drift-blind and would re-publish the uncovered HEAD>manifest drift that
optimize deliberately skips): after the per-table drift/blob skips and under the
queue + Optimize sidecar already held, a needs_index_create gate (reusing
needs_index_work_node/edge — "declared index missing AND row_count > 0", so empty
tables stay no-ops) admits index-only work, and Phase B builds the missing index
over the just-compacted layout via the build chokepoint. An untrainable vector
column fault-isolates into the new TableOptimizeStats.pending_indexes (the
list_indices/indisvalid analog operators read), not a failure. committed now
reflects index commits, so the existing post-publish cache invalidation covers
them. LanceDB's optimize only maintains existing indexes; creating
declared-but-missing ones is the L2 behavior omnigraph's declarative @index needs.
Turns the green half of optimize_materializes_index_declared_but_unbuilt.
Refs dev-graph iss-848.
* docs: index materialization is deferred to the reconciler (iss-848)
Update the index-lifecycle docs to reflect the new contract: @index/@key
declares intent and the physical index is derived state that never fails a
logical operation. Schema apply builds nothing (records intent only);
load/mutate build inline through one chokepoint that defers an untrainable
Vector column as pending; optimize/ensure_indices is the reconciler that
creates declared-but-missing indexes and maintains coverage, reporting
still-pending columns.
Touches: dev/invariants.md (truth-matrix Index-lifecycle row), AGENTS.md
(capability matrix), user/search/indexes.md (L2 orchestration), user/operations/
maintenance.md (optimize reconciler bullet), dev/testing.md (new tests).
* test(server): schema_apply_route_can_add_index reflects deferred index build
iss-848 made schema apply record @index intent without building the physical
index inline. The route test asserted the index count increased after apply;
on an empty graph it now stays unchanged (the build is deferred to
ensure_indices/optimize). Assert the new contract: apply succeeds and the
physical index count is unchanged.
* fix(engine): precheck vector trainability — don't pin or swallow (PR review)
Two issues Cursor Bugbot caught in the chokepoint fault-isolation:
1. (HIGH) Pending vector pins roll back siblings. needs_index_work_node counted
a missing vector index as work whenever the table had rows, so a column with
no trainable vectors got pinned in the EnsureIndices recovery sidecar — but
the build deferred it (zero commit). On a crash before manifest publish the
classifier sees NoMovement and the all-or-nothing decision (recovery.rs
decide()) rolls back the WHOLE sidecar, undoing a sibling table's committed
index work.
2. (MED) Vector build swallowed fatal errors. The match arm converted every
create_vector_index error into a deferred PendingIndex, hiding genuine
I/O/manifest/Lance failures as "pending".
Fix both with one trainability precheck (vector_column_trainable: >=1 non-null
vector, the ivf_flat(1) minimum) used identically by needs_index_work_node and
the build arm: an untrainable column is never counted as work (so never pinned —
no zero-commit pin) and never attempted (so it can't fail); only a trainable
column is built, and then any error PROPAGATES (stays fatal). The deferred
column is still recorded as a PendingIndex with a clear reason.
Refs dev-graph iss-848.
* feat(cli): surface pending index column + reason in optimize output (PR review)
Codex (P2): pending_indexes was documented as visible in `optimize --json` but
the CLI projection never emitted it — operators would lose the only signal that
optimize has deferred index work. Greptile (P2): the stat dropped the reason, so
operators saw which column was stuck, not why.
Carry the reason: TableOptimizeStats.pending_indexes is now Vec<PendingIndex>
(column + reason), and `omnigraph optimize --json` emits {column, reason} per
pending index; human output prints a "↳ index pending on '<col>': <reason>" line.
Refs dev-graph iss-848.
* test: align CLI index-add test with deferred build; cover post-rename reconcile
- schema_apply_json_adds_index_for_existing_property (cli_schema_config.rs): the
CLI analog of the server test — asserted the index count grew after apply;
under iss-848 the apply defers the build, so the count is unchanged on an
empty graph. Assert the deferred contract. (The only full-suite failure.)
- optimize_materializes_index_after_type_rename (maintenance.rs, new): covers
the gap Greptile flagged — a RenameType writes the renamed table with rows but
no indexes (inline build removed in Commit B); assert the rank index is
Degraded post-rename and Indexed after optimize reconciles it.
Refs dev-graph iss-848.
* test(engine): in-source apply tests reflect deferred index materialization
The two db::omnigraph in-source unit tests asserted the old "schema apply builds
/ preserves indexes inline" behavior (the only remaining full-suite failures):
- test_apply_schema_defers_index_then_reconciler_builds_it (was
test_apply_schema_adds_index_for_existing_property): apply records the @index
intent but builds nothing; assert the BTREE on `age` is absent after apply and
present after ensure_indices. (Uses `age`, unindexed in TEST_SCHEMA — `name
@key` is already FTS-indexed at seed.)
- test_apply_schema_rewrite_defers_index_then_reconciler_restores (was
test_apply_schema_rewrite_preserves_existing_indices): an AddProperty rewrite
no longer rebuilds indexes inline; assert ensure_indices restores id BTREE +
name FTS after the rewrite.
Verified by grep that these + the server/CLI tests are the complete set of
"apply builds an index" assertions; all other index-presence tests run after
load/ensure_indices/primitives, which still build.
Refs dev-graph iss-848.
* fix(engine): optimize always reports pending indexes, not only on create-work (PR review)
Cursor Bugbot (MED): pending_indexes was filled only when needs_index_create was
true, but the vector trainability precheck makes needs_index_work_node exclude an
untrainable Vector column. So a table whose sole missing index is untrainable, but
which optimize still compacts or reindexes, returned an empty pending_indexes —
contradicting the documented operator contract for deferred columns.
Run the (idempotent) build chokepoint unconditionally once past the no-op gate,
rather than gating it on needs_index_create. It skips existing indexes, builds
any buildable missing one, and reports an untrainable column as pending whether
the table entered for compaction, reindex, or index creation. needs_index_create
still gates the no-op decision (so an index-only table still enters the path).
Refs dev-graph iss-848.
* test(engine): reframe staged-BTREE-failure failpoint onto the reconciler path
ensure_indices_stage_btree_failure_leaves_existing_tables_writable fired
`ensure_indices.post_stage_pre_commit_btree` and expected `apply_schema` (adding
a type) to fail mid-BTREE-build. iss-848 removed apply's inline index build, so
that apply now succeeds and the test's unwrap_err panicked — it exercised a
removed code path.
Reframe onto where BTREE builds happen now: seed Person, add an `@index` on
`age` (apply records intent, defers the build), then `ensure_indices` builds the
deferred BTREE and the failpoint fires between stage and commit. Person's HEAD
is unchanged (no drift) and its EnsureIndices sidecar pins NoMovement; a write to
a different, unpinned table (Company) is unaffected (mutations/loads heal
roll-forward and proceed, unlike optimize/repair which refuse on a pending
sidecar). Preserves the original coverage (staged-index stage failure leaves
other tables writable, no drift) in the new architecture.
Refs dev-graph iss-848.
* feat(server): converge deferred indexes promptly after schema apply (iss-848)
Schema apply records @index intent but defers the physical build. On a
long-lived server, spawn a detached best-effort ensure_indices after a
successful apply so the indexes converge promptly instead of waiting for the
operator's next optimize. Fire-and-forget: it never blocks or fails the apply
response, and a failure is logged (the index still converges on the next
optimize). Guarded on result.applied. The CLI is one-shot, so it has no
equivalent; its convergence path is the optimize cadence.
handle.engine is already an Arc, so the spawn takes an owned clone. Convergence
itself is covered by the engine ensure_indices/optimize tests; the existing
empty-graph schema-apply route tests confirm the response is unaffected (the
spawn is a read-only no-op on an empty table).
Refs dev-graph iss-848.
* docs(maintenance): list pending_indexes in optimize per-table stats (consistency)
|
||
|
Ragnor Comerford
|
30377c453b
|
fix(embedding): address PR review feedback (RFC-012 Phase 2)
openai-alias host (Cursor): OMNIGRAPH_EMBED_PROVIDER=openai now defaults its base URL to https://api.openai.com/v1 (model text-embedding-3-large), while openai-compatible/unset keep the OpenRouter gateway default. The default is derived from the alias rather than the Provider enum, so an operator's stated intent can no longer be silently routed to OpenRouter; an explicit OMNIGRAPH_EMBED_BASE_URL still wins. New test from_env_openai_alias_uses_openai_host_not_openrouter. single model source of truth (Cursor): remove the EmbedSpec.model field. The provider config is authoritative for the model, so a spec can no longer declare a model that is silently ignored while the API uses another (the wrong-space-vectors footgun); the embed summary reports the model actually resolved. Correct by construction rather than a truthful-echo patch. stale @embed docs (Codex): docs/user/schema/index.md and docs/dev/execution.md still claimed @embed embeds at ingest; corrected to the real contract (catalog annotation; vectors supplied or pre-filled by 'omnigraph embed'). Also documented the openai-vs-OpenRouter base default in embeddings.md. Greptile's RFC-status note is declined: the repo lifecycle keeps an RFC Status: Proposed while its PR is open and flips to Accepted on merge. |
||
|
Ragnor Comerford
|
5b83e6b56d
|
docs(rfc): add RFC-012 provider-independent embedding configuration
Records the design for one resolved EmbeddingConfig behind a sealed Provider enum (OpenAiCompatible primary, covering OpenRouter as the default gateway, OpenAI direct, and self-hosted endpoints; native Gemini for task-type asymmetry; Mock). Identity recorded in the schema IR with query-time same-space validation, NFR floor (deadline, observability, single normalize), and a 5-phase rollout. Corrects the prior handoff's misdiagnosis (the OpenAI client it blamed was dead code). Links from docs/dev/index.md. |
||
|
Ragnor Comerford
|
c3d7639377
|
test(engine): pin Lance 7 immutable-PK behavior + sharpen native-namespace alignment notes (#240)
* test(engine): pin Lance 7 immutable-PK behavior + sharpen native-namespace alignment notes Follow-up polish to the Lance 7.0.0 alignment (the immutable-PK migration fix and the realigned native-namespace surface test). Two precision nits, no behavior change: 1. Pin the upstream behavior we now depend on. Lance 7 makes the unenforced PK immutable once set (`lance::dataset::transaction`): re-applying the reserved `lance-schema:unenforced-primary-key` key — even with the same value — errors "cannot be changed once set". That is exactly what broke `migrate_v1_to_v2`'s crash-idempotency and forced its field-guard. Add `lance_surface_guards.rs::unenforced_primary_key_is_immutable_once_set` so a future Lance bump that relaxes immutability turns red, prompting re-evaluation of the migration guard. (Matches the "first smoke check on a Lance bump" discipline in docs/dev/lance.md.) 2. Clarify that the native `DirectoryNamespace` decoupling is contingent on omnigraph's legacy boolean PK key, not an unconditional v7 property: with the position key the native namespace would still read the manifest. omnigraph keeps the boolean key deliberately — Lance honors it permanently (maps to PK position 0) and one uniform on-disk format beats a new-vs-old split, since existing graphs can't be re-keyed under the same immutability rule. Updated the test comment and the lance.md stanza; also corrected the stale `is_empty()` description of the migration guard (it now matches on the specific PK field). * test(engine): make the immutable-PK guard's red-bar diagnostic fire in every change-shape Review follow-up: the guard's re-set assertion chained `.unwrap().await.unwrap_err()`, which only surfaces the actionable "Lance no longer rejects re-setting the unenforced PK" message when immutability is enforced on the async commit path and still returns an error. Two other change-shapes would panic generically instead, defeating the guard's purpose: - if Lance moves the check to the sync validation stage, the first `.unwrap()` panics with a bare "unwrap() on Err"; - if Lance relaxes immutability so the re-set succeeds, `.unwrap_err()` panics with a bare "unwrap_err() on Ok". Normalize the sync `.update()` result and the async `.await` into one `Result` and assert on it, so the diagnostic fires whichever stage enforces (or relaxes) the rule. |
||
|
Andrew Altshuler
|
ceb37dd4cb
|
fix(engine): close the 2 Lance 7.0.0 alignment failures (immutable PK + native namespace) (#236)
* fix(engine): make the v1→v2 manifest migration idempotent under Lance 7's immutable unenforced primary key Lance 7 (dataset/transaction.rs) makes the unenforced primary key immutable once set: any write touching the reserved `lance-schema:unenforced-primary-key` field metadata after the PK is set errors "cannot be changed once set" — even re-applying the same value. `migrate_v1_to_v2` previously relied on the old Lance 6 idempotency (re-applying the annotation was a no-op-ish bump), which it needs for crash-recovery: a v1 graph that crashes after the field-set but before the stamp bump re-enters the migration with the PK already present. Under Lance 7 that re-entry now errors, so a real pre-v0.4.0 graph crashing in that window could never complete its migration. Guard the field-set with `schema().unenforced_primary_key().is_empty()` so a genuine first-set still runs but a re-set is skipped — restoring crash-idempotency by construction. (Fresh graphs bake the PK into manifest_schema() at init and never run this migration.) The existing test_publish_migrates_pre_stamp_manifest_to_current_version is the regression guard: red under Lance 7 before this change, green after. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(engine): realign the native-namespace surface guard to Lance 7 (TableNotFound) `test_directory_namespace_direct_publish_cannot_replace_native_omnigraph_write_path` pokes Lance's NATIVE DirectoryNamespace (not omnigraph's production write path, which is the manifest merge_insert publisher) to document that it cannot replace omnigraph's authority. Lance 7's DirectoryNamespace routes list/describe/create_table_version through `check_table_status`, which now reports an omnigraph-manifest-tracked table as absent — so all three return TableNotFound for `node:Person` (observed). The native namespace is now fully decoupled from omnigraph's manifest: it cannot enumerate, inspect, or publish over omnigraph's tables. This strengthens the guard's thesis. Realigned the assertions to the v7 behavior and kept the authority check (omnigraph's refresh ignores the direct append; row_count stays 0). Test-only; no production impact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(lance): document the 2 runtime behavior changes in the 7.0.0 alignment stanza The #229 stanza verified a clean engine *build* but not the test suite, and claimed "no Lance API surface omnigraph uses changed." Two runtime behaviors did, caught only by the full test suite: - the unenforced primary key is immutable once set in v7 (transaction.rs) — broke the v1→v2 manifest migration's crash-idempotency; fixed by an is-set guard; - the native DirectoryNamespace returns TableNotFound for omnigraph manifest-tracked tables (dir.rs) — test-only; the surface guard was realigned. Corrects the over-broad "no surface changed" claim, adds both findings, and notes the lesson: a clean build is not a clean alignment — run cargo test --workspace before declaring a Lance bump done. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
67baf615d9
|
build(deps): bump Lance 6.0.1 → 7.0.0 (correct-by-design substrate alignment) (#229)
* build(deps): bump Lance 6.0.1 → 7.0.0 (object_store 0.13.2, roaring 0.11.4) Arrow stays 58 and DataFusion stays 53 (no change). The only transitive bump is object_store 0.12.5 → 0.13.2. 141 upstream commits reviewed; no fixes lost (the 6.0.x release-branch backports are all forward-ported into 7.0.0). - object_store 0.13 moved get/put/head/rename/delete behind a new ObjectStoreExt trait (list/list_with_delimiter/put_opts stay on the core trait). Add `use object_store::ObjectStoreExt` in storage.rs and db/manifest/namespace.rs; no call-site changes. Mirrors Lance's own migration in PR #6672. - roaring pinned to 0.11.4 (cargo update -p roaring --precise 0.11.4). Lance 7.0.0's UpdatedFragmentOffsets newtype (lance#6650) derives Eq over HashMap<u64, RoaringBitmap>, which needs RoaringBitmap: Eq, added in roaring 0.11.4; the loose `roaring = "0.11"` constraint otherwise resolves 0.11.3 and lance itself fails to compile. - lance#6774: merge-insert INSERT rows now stamp _row_created_at_version with the commit version (was a fallback of 1). Flip the lance_version_columns assertion to `== v2` and correct the changes/mod.rs rationale comment. Production change-detection keys on _row_last_updated_at_version + ID membership, so its logic is unaffected. Refs lance#6650, lance#6774, lance#6672. * fix(storage): pin WriteParams::auto_cleanup = None (lance#6755 default flip) lance#6755 flipped the WriteParams::auto_cleanup default from on (a full cleanup pass every 20th commit) to None. On 6.0.1 the on-by-default hook could silently GC versions that __manifest pins for snapshots/time-travel. OmniGraph owns cleanup explicitly (optimize.rs::cleanup_all_tables) and never set auto_cleanup, so it was relying on a default that is both wrong for our snapshot model and now changed upstream. Pin auto_cleanup: None explicitly at all 11 production WriteParams sites (table_store ×6, commit_graph ×2, recovery_audit ×1, manifest/graph ×2 — the __manifest + sub-table Create paths). Removes the dependency on a default-flag value and locks in the snapshot-safe behavior regardless of future upstream re-flips. Refs lance#6755. * test(lance): pin BTREE range-boundary correctness (lance#6796) lance#6796 (issue #6792) fixed a BTREE scalar-index range-query bound inclusiveness bug: `x <= hi AND x > lo` returned the wrong boundary row. Add lance_surface_guards.rs::btree_range_query_boundary_is_correct, which reproduces the exact #6792 shape (5 rows + an explicit BTREE drives the index path even on tiny data) and pins the corrected inclusive-<= / exclusive-> semantics. It turns red if a future Lance regression reintroduces the bug. OmniGraph today builds BTREE only on string @key columns and queries them by equality/IN, so its current patterns do not hit this; the guard protects any future BTREE-range path (BTREE-on-properties, range-on-key). Refs lance#6796. * docs(dev): align Lance docs + invariants to 7.0.0 - docs/dev/lance.md: new 2026-06-14 alignment stanza for the 6.0.1 → 7.0.0 bump (object_store ObjectStoreExt move, roaring 0.11.4, #6774/#6796/#6755 behavior, #6658 shipped → MR-A unblocked but separate, #6666 + blob compaction still open); prior 6.0.1 stanza demoted to historical. - AGENTS.md: storage substrate 6.x → 7.x (line + architecture diagram). - docs/dev/invariants.md: deletes/vector known gap updated — the staged two-phase delete API (lance#6658) now exists and MR-A is unblocked, but delete_where stays inline and D2 stays in place until the migration lands; create_vector_index still gated on lance#6666. * fix(storage): skip Lance auto-cleanup on commit paths for legacy datasets Addresses PR #229 review (Codex P1). `WriteParams::auto_cleanup` is create-time config with no effect on existing datasets (Lance write.rs docs), so the previous `auto_cleanup: None` change alone did NOT protect graphs created before the v7 bump: 6.0.1 defaulted auto_cleanup ON, leaving `lance.auto_cleanup.*` config on those datasets, and Lance's per-commit hook (io/commit.rs: `if !commit_config.skip_auto_cleanup`) fires off that stored config — so omnigraph's own writes would GC versions the __manifest pins for snapshots/time-travel. Skip the hook on every commit path, covering new and legacy datasets alike: - commit_staged: CommitBuilder::with_skip_auto_cleanup(true) — the staged data path. - __manifest publisher: MergeInsertBuilder::skip_auto_cleanup(true). - all 11 WriteParams: skip_auto_cleanup: true (direct Dataset::write/append paths; auto_cleanup: None retained so new datasets store no cleanup config at all). Tests: - lance_surface_guards::skip_auto_cleanup_suppresses_version_gc — substrate: negative control (config GCs v1 without skip) + with-skip survival. - staged_writes::commit_staged_skips_auto_cleanup_so_pinned_versions_survive — omnigraph usage: commit_staged on a legacy-config dataset preserves the pinned create version. Refs lance#6755. * test(lance): assert created_at-preserved + updated_at-bumped on merge_insert UPDATE Addresses PR #229 review follow-up. `lance_merge_insert_update_preserves_created_at_version` documented (in a comment) that a merge_insert UPDATE preserves created_at and bumps updated_at, but only asserted the value change — leaving the change-feed invariant unguarded. Add the two missing assertions: - bob created_at == v1 (preserved across UPDATE; what the test name promises; lance#6774 only changed INSERT-row stamping). - bob updated_at == v2 (bumped to the commit version) — the invariant OmniGraph's insert/update classification relies on (changes/mod.rs keys on _row_last_updated_at_version). A regression here would silently drop updates from the diff/change feed. |
||
|
Andrew Altshuler
|
6fa04efc76
|
docs(rfc): RFC-011 — rename "context" → "profile" (#233)
"context" (borrowed from kubectl) collides with agent/LLM "context". Rename the named-operator-defaults concept to "profile" (AWS-CLI precedent: --profile, OMNIGRAPH_PROFILE, profiles:). Doc-only — the concept was unshipped. "scope" (the resolved addressing target a profile populates) is unchanged. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
35e87ab882
|
docs(rfc): RFC-011 — CLI refactoring (one addressing & config model) (#228)
* docs(rfc): RFC-011 — CLI refactoring (one addressing & config model) A maintainer-internal RFC (Status: Proposed) for the post-omnigraph.yaml CLI: one ontology (store/server/cluster; cluster vs operator config; catalog; context; capability); addressing = scope + --graph with the access path *derived*; served is the default front door and direct storage is privileged (admin/break-glass); stateless per command; definitions named, payloads passed. Includes the full end-state command taxonomy (by capability), a current-state appendix, migration, invariants check, and the resolved Decisions (with two deferred). Completes the config/CLI lineage RFC-007 → RFC-008 → RFC-009 → RFC-010. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(rfc): RFC-011 — address Greptile review (4 doc fixes) - P1: end-state taxonomy `schema apply` annotation said "Open Q10" — now points at the resolved Decision 10 (cluster graphs via cluster apply). - P1: add the `alias <name>` verb (Decision 4) to the end-state taxonomy's local section — it was claimed "full command set" but omitted. - P2: Decision 11's bulk-data-plane reference now carries the "PR #219, not yet merged" caveat (matches the Relationship section). - P2: footnote now states the `check`→`lint` argv-shim is removed (its end-state disposition was unspecified). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
1bed998052
|
fix(engine): scalar index coverage + filter literal coercion (query latency) (#216)
* fix(engine): lower date/datetime filter literals as typed Arrow scalars `literal_to_expr` lowered `Date`/`DateTime` query literals as Utf8 strings, relying on DataFusion implicit casts. Against a physical `Date32`/`Date64` column that can coerce the column side (`CAST(col AS Utf8)`), which defeats a scalar BTREE and degrades the scan to a full filtered read. Lower to typed `Date32`/`Date64` scalars instead (reusing the loader's `parse_date32_literal`/`parse_date64_literal`, already used by the in-memory comparison arm), so the predicate stays a direct column comparison and the index is used. Malformed literals fall back to the Utf8 string so pushdown behavior never regresses. Tests: unit goldens asserting the lowered literal is typed (red before, green after) + inline-binding pushdown equality in literal_filters confirming the epoch conversion selects the right rows. * fix(engine): build scalar BTREE for enum and orderable-scalar @index columns `build_indices_on_dataset_for_catalog` only handled `String` (-> FTS) and `Vector` (-> vector). Enums are physically `String`, so an enum `@index` column (e.g. `status`) got an FTS inverted index, which Lance never consults for `=`; and `DateTime`/`Date`/numeric/`Bool` `@index` columns fell through and built nothing. Both meant equality/range filters degraded to full scans with `indices_loaded=0`. Dispatch index kind by property type via a shared `node_prop_index_kind`: enum + orderable scalar -> BTREE, free-text String -> FTS, Vector -> vector, list/Blob -> none. The helper is shared by the builder and `needs_index_work_node` so they cannot drift — the latter decides recovery- sidecar pinning, and under-reporting would leave a HEAD-advancing index build uncovered (invariant 5). Tests: scalar_indexes.rs asserts enum/DateTime/numeric @index columns report `IndexCoverage::Indexed` while free-text String/un-annotated columns stay `Degraded` (negative control). Docs: docs/user/indexes.md. * feat(engine): reindex in optimize to keep index coverage current A scalar/FTS/vector index only covers the fragments it was built over. Rows appended after the build (e.g. `ingest --mode merge`, whose commit does not rebuild an existing index) are scanned unindexed, and `compact_files` rewrites fragments out of coverage. Nothing folded them back in, so coverage decayed as the graph grew — even the id/src/dst BTREEs that power traversal. `optimize_one_table` now runs Lance `optimize_indices` after `compact_files` (incremental merge, not retrain — the same compact->optimize_indices sequence LanceDB's `optimize()` uses) and enters the publish path on compaction work OR stale index coverage (new `TableStore::has_unindexed_fragments`, reusing the fragment_bitmap logic). `optimize_indices` is a committing call with no uncommitted variant in lance-6.0.1, so it is an inline-commit residual covered by the existing `SidecarKind::Optimize` recovery sidecar spanning both ops. Blob-bearing tables are still skipped (the Lance blob-compaction bug is compaction-specific; reindex-for-blob deferred as a noted follow-up). Tests: maintenance.rs asserts an appended fragment is uncovered before and covered after optimize, and idempotency holds (second pass is a no-op). lance_surface_guards pins the `optimize_indices` signature and its incremental- coverage behavior. The existing optimize Phase-B recovery failpoint now also exercises a crash after reindex. Docs: maintenance.md, writes.md, invariants.md, lance.md, AGENTS.md. * fix(engine): coerce pushdown filter literals to the column type Filter literals were pushed to Lance in their natural Arrow type (every integer Int64, every float Float64). Against a narrower indexed column DataFusion widens to the literal's type and casts the COLUMN (`CAST(n32 AS Int64)`), which defeats the scalar BTREE and degrades to a full filtered read. A physical-plan probe confirms it: an Int32 column filtered by an i32 literal uses `ScalarIndexQuery`; by an i64 literal it does not. Thread the scan's `arrow_schema` through `build_lance_filter_expr` -> `ir_filter_to_expr` and coerce each literal operand to the opposite column's exact Arrow type, reusing `projection::literal_to_array` + `arrow_cast` (the same path the in-memory arm uses, so the two arms agree). Coercion never demotes a filter to None: on failure it falls back to the natural literal, because a node scan has no in-memory fallback for inline filters. Supersedes the date-specific change in |
||
|
Andrew Altshuler
|
d46e50dd6d
|
docs(user): restructure user docs into topic sections (Phase 1) (#223)
Move the 23 flat docs/user/*.md files into topic subdirectories so the user guide is organized by area (schema, queries, search, branching, cli, operations, clusters, concepts, reference) instead of a flat list. This is a pure structural move — whole files relocated, every cross-doc link recomputed, no prose rewrites or content splits (those follow in Phase 2). - 19 `git mv`s (install.md, deployment.md stay top-level); history preserved (renames detected at 92–100% similarity). - All intra-doc links, AGENTS.md's topic table (52 pointers), and the docs/dev + docs/releases back-links recomputed via relpath from each file's new location. - docs/user/index.md rewritten as a sectioned nav hub. - Fixed 5 doc-path references in Rust (comments + two user-facing server settings error strings) to point at the new locations. Verified: zero broken .md links across tracked docs; check-agents-md.sh green (with the untracked scratch docs set aside); touched crates build. Note: the public site (omnigraph-web) imports docs/ via a flat-only script; its import-docs.mjs needs a subdir-aware update before the next re-sync. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
8726ca92ec
|
feat: canonical POST /load, deprecate /ingest (RFC-009 Phase 5) (#222)
* feat(server): canonical POST /load, deprecate /ingest (RFC-009 Phase 5) The CLI's non-deprecated `load` verb rode the deprecated `/ingest` route, so `/ingest`'s eventual removal would silently break it. Add a canonical `/load`, mirroring the shipped `/mutate`↔`/change` and `/query`↔`/read` pattern. - Extract `server_ingest`'s body into a shared `run_ingest` (branch-exists / fork-if-`from`, Cedar auth, admission, `load_as`, `IngestOutput` mapping). - `server_load` (canonical) → `run_ingest`, `Json<IngestOutput>`. - `server_ingest` (deprecated) → `run_ingest` + `#[deprecated]` + RFC 9745/8288 `Deprecation: true` / `Link: </load>; rel="successor-version"` headers. - Router mounts `/load` (same 32 MB body limit) beside `/ingest`; OpenAPI `paths(...)` gains `server_load` and flags `server_ingest` deprecated. `/load` reuses `IngestRequest`/`IngestOutput`, exactly as canonical `/mutate` reuses `Change*` — a DTO rename is a separate, larger change (out of scope). openapi.json regenerated. Tests: openapi `/load` present + not deprecated, `/ingest` deprecated, `/load` bearer-secured; data_routes `/load` happy path + `/ingest` deprecation headers. Existing `/ingest` route tests stay green (the shim is unchanged). Docs: server.md endpoint table; RFC-009 Phase 5 marked landed (incl. the hand-mount-vs-utoipa-axum registration finding). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): point remote load at /load (RFC-009 Phase 5) `GraphClient::load`'s remote arm now POSTs to the canonical `/load` route instead of the deprecated `/ingest`; the deprecated `ingest` verb keeps riding `/ingest`. `parity_load` exercises `/load` on the remote arm (its documented flip); the matrix exclusions comment is updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
2ddb88fad9
|
docs(rfc): RFC-010 — apply verification-comment current-state fixups (#215)
Folds in the Codex verification review (kept verbatim with per-point Resolution notes): - `graphs list` is marked remote-only today in the current-state table (the embedded arm bails; it rides GraphClient only to share the resolver). - `init` is noted as positional-URI-only today (no `--target`); adding `--target` to init is part of the proposal, entangled with the init→cluster apply signpost, not current state. - Validated-fact #1 now describes the post-collapse reality (`GraphClient::resolve*`; only the two factories call `apply_server_flag`), dropping the stale "16 call sites" count. - The Authority rule carries a flag-shape caveat: `--graph` is already a global flag requiring `--server`, so the cluster-managed resolver and its flag shape are deferred to a later slice; the illustrative `--cluster <dir> --graph <id>` spelling is marked not-final. Docs-only; no code change. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
be4f29c0d0
|
docs(rfc): RFC-010 — restructure the CLI around explicit planes (#214)
The CLI silently spans three planes (data / storage-maintenance / control) and forces the operator to name a graph differently per plane: the graph you query as `--server prod --graph knowledge` you must maintain as `s3://bucket/knowledge.omni`. Plane restrictions (graphs list is server-only, optimize is storage-only) are accidental — discovered by hitting a cryptic error, not declared. RFC-010 proposes: one graph-addressing model across every verb, a declared per-subcommand capability surface (expanding RFC-009 Phase 4), and plane-grouped --help. Storage maintenance stays off the wire deliberately (no HTTP routes for optimize/cleanup/repair). CLI-internal only — no engine, server, or wire change. Incorporates the Codex review thread (kept verbatim with per-point Resolution notes): sharpened resolver authority rule (operator/legacy target must be direct storage; cluster-managed graphs via explicit --cluster --graph), per-subcommand capability table (schema plan vs show/apply, queries validate vs list, session/tooling classified), graphs list aligned to RFC-009's both-later target, init promoted to an explicit cluster-apply signpost, and a Test plan that extends the existing CLI suites and pins the new wrong-plane error strings. Linked from docs/dev/index.md. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
81b66f9427
|
ci: run Test Workspace only on main, not on pull requests (#212)
The full workspace + failpoints suite was the slowest PR gate (~15min warm, up to the 75min cold ceiling) and dominated PR turnaround. Gate the `test` job with `if: github.event_name != 'pull_request'` so it runs only on push to `main` (post-merge), on `v*` tags, and on manual `workflow_dispatch`. `RustFS S3 Integration` needs `test`, so it becomes push-/dispatch-only by the same cascade. Drop `Test Workspace` from the required-check list in branch-protection.json: a required context that never reports on PRs (the job no longer runs there) would leave every PR permanently pending — the job-never-reports trap the policy already documents. Trade-off accepted deliberately (chosen by the maintainer): a regression the suite would catch now lands on `main` and reddens the post-merge run instead of being blocked pre-merge, so `main` can briefly break. Mitigations documented in ci.md: run `cargo test --workspace --locked` locally before merging non-trivial changes (or trigger the workflow on your branch via workflow_dispatch), and regenerate openapi.json locally for server/API changes (the auto-regen step lived in the now-PR-skipped test job). The fast PR gates remain: Classify Changes, Check AGENTS.md Links, the AWS-feature build/test, and the two CODEOWNERS checks. NOTE: an admin must run ./scripts/apply-branch-protection.sh after this merges, or GitHub keeps requiring the now-unreported Test Workspace context. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
de9e28ecf6
|
docs(rfc-003): document the per-graph multi-graph MCP model
Add §15.1 making the multi-graph model explicit (it was implied across
§9/§14/§15): one isolated MCP server per graph at /graphs/{id}/mcp with the
graph id in the URL path; cross-graph discovery is REST-only via GET /graphs
(server-scoped GraphList, default-denied without a server policy); stored-query
registries, tool-name uniqueness, projection mode, and InvokeQuery are all
per-graph; clients configure one connection per graph and the connection name
namespaces the otherwise-identical tool ids.
|
||
|
aaltshuler
|
3e2502c35e |
docs: omnigraph-api-types in the crate list; RFC-009 Phase 2 outcome
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ragnor Comerford
|
20233a34ae
|
docs(rfc-003): standalone MCP surface spec, validated against upstream
Rewrites the MCP RFC as a single self-contained specification, validated against the live upstream surfaces it builds on: - MCP protocol revision 2025-11-25 (Streamable HTTP transport requirements, authorization model, tool/resource shapes, SEP-1303 input-validation-as- tool-error, JSON Schema 2020-12 default). - rmcp 1.7.0 (StreamableHttpServerConfig real defaults — stateful_mode defaults true and must be flipped; NeverSessionManager for stateless mode; the `local` feature must stay off; RequestContext.extensions carries the http::request::Parts passthrough; RPITIT ServerHandler). - Tool/server best practice: domain-qualified tool names, explicit annotations, structured output + text mirror, and progressive disclosure (per_query vs meta projection modes) for large stored-query catalogs. Adds a provider compatibility matrix (Claude Code/Desktop/web, Messages API connector, OpenAI Responses API/ChatGPT, Cursor, VS Code, OpenCode) with a phased static-bearer -> OAuth 2.1 + RFC 9728 auth plan and the Claude Code PRM header-override caveat, and a code-mode compatibility section describing the server-side properties that make the surface code-execution-friendly without building client-side machinery. |
||
|
Andrew Altshuler
|
82dda296d1
|
Merge pull request #208 from ModernRelay/test/parity-matrix
test(cli): embedded/remote parity matrix (RFC-009 Phase 1) |
||
|
Ragnor Comerford
|
61c18b67ba
|
docs(rfc-003): validate against main; fold the implementation spec in
Rebased onto current main (0.7.0-dev) and reconciled the MCP blueprint
with the code it builds on, replacing the earlier blueprint sketch with a
spec grounded in real interfaces (each EXISTING snippet cited to file:line,
re-read from source). Corrections (delta table in §0):
- ParamKind::Vector is a unit variant; the dimension is
ParamDescriptor.vector_dim: Option<u32>, not an intrinsic struct field.
The JSON-Schema builder now omits minItems/maxItems when the dim is
absent instead of emitting 0.
- Server handlers, auth helpers, and run_query/run_mutate moved from
lib.rs to handlers.rs; stored-query config in config.rs; DTOs in api.rs.
- ingest/load unified: server_ingest calls load_as; a missing branch with
no `from` is a 404, never an implicit fork. The MCP tool is named `load`.
- Client credentials follow the shipped RFC-007 model (OperatorServer { url },
env -> credentials-file chain, no keychain step, no nested auth: block).
- Test references updated for the split server suites (mcp.rs / stored_queries.rs,
not --test server).
- Stored-query declaration source noted as cluster.yaml file-discovery with
omnigraph.yaml as the RFC-008-deprecated legacy surface.
Folds the standalone implementation-spec draft into this file so there is one
MCP RFC; cross-links rfc-005/007/009 now that they are in the corpus.
|
||
|
Ragnor Comerford
|
6fc92cab06
|
docs(rfc-003): correct-by-construction fixes from PR review
Fold the valid #157 review findings into the blueprint as by-design fixes so a from-scratch build cannot reintroduce them: - B.3.1: host/origin policy derived from the server bind + config (loopback -> loopback hosts; non-loopback -> configured public host, else Host-allowlist off with bearer + Origin as the controls). Closes the loopback-only-default footgun that 403s every remote client before bearer auth. - B.6/B.7: stored-query params nested under a `params` object (mirrors POST /queries/{name}), so the branch/snapshot knobs cannot collide with a query param; mutation tools omit `snapshot` + additionalProperties:false, so mutation-against-a-snapshot is unrepresentable. Vector `dim` made intrinsic to the kind, killing the unwrap_or(0) zero-length-array schema. - B.8: one canonical classify(Result<_, ApiError>) for tool errors (5xx -> JSON-RPC, 4xx/409 -> isError), no second mapper to drift; list visibility uses the call-path default-branch authorization, not a branch:None probe that hid branch-scoped-grant tools. - Reconciled the stale Summary count and the section-5-era Rollout/Testing sections (banners pointing to B.12/B.4/B.13). |
||
|
Ragnor Comerford
|
971eefed3a
|
docs(rfc-003): canonical MCP implementation blueprint (omnigraph-mcp crate)
Brings RFC-003 to the canonical from-scratch spec for the in-server MCP surface: a dedicated omnigraph-mcp transport crate + McpBackend trait (server -> mcp dep to avoid a Cargo cycle), the verified rmcp 1.7 design (conformance MUSTs for free), backend reuse (do_* / run_query / run_mutate / authorize / api), the 13-tool catalog + Cedar mapping, ParamKind -> JSON Schema, deny-masking + isError split, two resources, per-graph routing (graphs_list stays REST-only), auth + client-compat matrix + OAuth fast-follow, tests, and locked decisions. Build the implementation on this branch from blueprint sections B.1-B.13. The spec is informed by the #157 reference implementation. |
||
|
Ragnor Comerford
|
446b46d548
|
Recovery liveness, storage fault-injection matrix, and one storage implementation over object_store (#203)
* test(engine): pin the long-lived-handle heal contract for sidecar-covered drift
A Phase B -> Phase C failure (commit_staged advanced Lance HEAD, manifest
publish did not land, recovery sidecar persists) currently wedges every
subsequent staged write on the same engine handle: the commit-time drift
guard rejects with 'run omnigraph repair', but repair itself refuses
while a recovery sidecar is pending, so a long-lived server can only
recover by restart. The documented contract (writes.md 'Long-running
servers', invariants.md invariant 5) says refresh-time roll-forward
closes this residual without restart -- but no write path runs it.
Two red tests pin the intended contract at the write entry points:
a follow-up load (the POST /ingest shape: shared handle, no reopen)
and a follow-up mutation must heal roll-forward-eligible sidecars
in-process and then succeed.
Currently failing with:
table 'node:Company' has Lance HEAD version 2 ahead of manifest
version 1; run `omnigraph repair` before writing
The fix lands in the next commit.
* fix(engine): heal pending recovery sidecars at the staged-write entry points
Close the long-lived-process gap in the recovery protocol: a Phase B ->
Phase C residual (per-table commit_staged landed, manifest publish did
not, sidecar persists) previously recovered only at the next ReadWrite
open or via an explicit refresh() that no production write path called,
so a long-lived server wedged every subsequent write on the commit-time
drift guard until restart.
New recovery::heal_pending_sidecars_roll_forward:
- one list_dir of __recovery/ at write entry (empty -> immediate
return, the steady state), so the per-write cost is one storage list;
- per sidecar, acquires the same per-(table_key, table_branch) write
queues every sidecar writer holds from before write_sidecar until
after delete_sidecar, then re-checks sidecar existence -- this
serializes the heal against live writers instead of rolling an
in-flight sidecar forward from under its writer (which would fail
that writer's publish CAS spuriously). Lock order queues ->
coordinator matches every writer's commit->publish path. This is the
queue-acquisition design recovery.rs and write_queue.rs already
documented for in-process recovery;
- processes in RollForwardOnly mode: the common residual rolls forward
in-process; rollback-eligible sidecars still defer to the next
ReadWrite open (Dataset::restore is unsafe under concurrency).
Wire it into load_as and mutate_as (before the inline delete path can
advance any HEAD), and rebase Omnigraph::refresh onto the same helper
so refresh stops racing live writers' sidecars.
The maintenance entry points (apply_schema_as, branch_merge_as,
ensure_indices) intentionally keep their strict fail-loud preconditions
for now; wiring the same heal there is a follow-up with its own tests.
Turns the previous commit's two red tests green.
* fix(engine): name the right recovery path in the commit-time drift guard
The drift guard's 'run omnigraph repair before writing' advice is a
dead end when the drift is covered by a pending recovery sidecar:
repair refuses while a sidecar is pending. With the write-entry heal in
place, reaching this guard with sidecar-covered drift means the heal
deferred it (rollback-eligible), and the actual recovery path is a
read-write reopen. Distinguish the two classes on the error path only
(one sidecar list, after the conflict is already certain); a listing
failure falls back to the uncovered-drift wording rather than masking
the conflict.
Pinned by extending refresh_defers_rollback_eligible_sidecar_to_next_open
with a write attempt against the deferred sidecar.
* docs: write-entry in-process sidecar heal — contract and coverage
Update the recovery contract docs to match the previous two commits:
invariant 5 now states that the staged-write entry points and refresh
run in-process roll-forward recovery (long-lived processes converge on
the next write, not at restart); writes.md 'Long-running servers'
describes the heal's queue-acquisition concurrency contract, the
improved drift-guard error, and the entry points that intentionally do
not heal yet; testing.md indexes the new failpoint tests; AGENTS.md
capability matrix drops the claim that in-process recovery is entirely
future work (only the rollback path remains with the background
reconciler).
* test(engine): pin the entry heal contract for schema apply and branch merge
Without the write-entry heal, the two maintenance writers do worse than
wedge on sidecar-covered drift -- they proceed and decide its fate
implicitly:
- schema apply re-plans table rewrites from the manifest pin, orphaning
the drifted Phase-B commit (its rows silently vanish from the
rewritten table) while the stale sidecar lingers to misclassify
against the post-apply pins;
- branch merge publishes over the drift, making the failed writer's
commit visible as an unattributed side effect (no recovery audit
row), and leaves the stale sidecar behind.
Two red tests pin the intended contract: both entry points heal the
sidecar first (attributed roll-forward), then run on the converged
state. Currently failing on the stale-sidecar / dropped-rows
assertions; the fix lands in the next commit.
* fix(engine): heal pending recovery sidecars at the schema-apply and branch-merge entries
Extend the write-entry heal to the remaining two write entry points.
Unlike load/mutate (which wedge on the drift guard), these proceeded
over sidecar-covered drift and decided its fate implicitly:
- schema apply re-planned table rewrites from the manifest pin,
orphaning the drifted Phase-B commit -- its rows silently vanished
from the rewritten table -- while the stale sidecar lingered to
misclassify against the post-apply pins;
- branch merge published over the drift, making the failed writer's
commit visible without a recovery audit row, and left the stale
sidecar behind.
Both now run the same queue-serialized roll-forward heal at entry,
before their own sidecar exists, so recovery is attributed (audit row)
and deterministic. ensure_indices stays heal-free: it runs inside the
load / schema-apply flows after their entry heal.
Turns the previous commit's two red tests green. Docs updated in the
same change (invariant 5, writes.md, testing.md, AGENTS.md).
* test(engine): pin Phase A sidecar-write failure semantics
Storage fault-injection matrix, row 1: a sidecar PUT failure (S3
PutObject / fs write) in Phase A. New failpoint recovery.sidecar_write
at the top of write_sidecar -- the single choke point all five sidecar
writers go through -- models the storage error backend-generically.
Also adds the other three storage-fault failpoints used by the
following commits (recovery.sidecar_delete, recovery.sidecar_list,
recovery.record_audit); each is a no-op without the failpoints feature.
Pinned contract: every writer writes its sidecar BEFORE its first
HEAD-advancing commit, so a put failure aborts with zero drift (no
sidecar, Lance HEAD == manifest pin, no rows) and a transient fault
never wedges the graph -- the same handle writes/merges normally once
it clears. Covered for load (the staging writer) and branch_merge (the
multi-table writer, forced onto the RewriteMerged path by diverging
both sides).
* test(engine): pin Phase D delete, list, and audit-append storage-fault semantics
Storage fault-injection matrix, rows 2/3/5, plus the real-backend run:
- recovery.sidecar_delete: a Phase D delete failure (S3 DeleteObject)
must NOT fail the user's write -- the manifest publish already
landed, so the caller's data is durable. The swallowed failure
leaves a stale sidecar; the next write's entry heal consumes it via
the stale-sidecar audit-recovery path (RolledForward, attributed).
- recovery.sidecar_list: a __recovery/ list failure (S3 ListObjectsV2)
is loud at every consumer -- the write-entry heal fails the write
and the open-time sweep fails the open. Silently skipping recovery
over a pending sidecar would be consumer tolerance of drift. Once
the fault clears, open recovers the pending sidecar normally.
- recovery.record_audit: an audit write failure after the
roll-forward's manifest publish aborts that recovery attempt and
keeps the sidecar; re-entry detects the already-published manifest,
records exactly ONE RolledForward audit row, and converges -- the
retry tolerance documented on record_audit, exercised end-to-end.
- s3_load_recovers_after_publisher_failure_without_reopen: the
same-handle heal scenario on a real bucket (gated on
OMNIGRAPH_S3_TEST_BUCKET, skips locally), exercising sidecar
put/list/delete through S3StorageAdapter instead of the local-FS
adapter. CI wiring lands in a follow-up commit.
* test(engine): refuse corrupt recovery sidecars loudly
Storage fault-injection matrix, row 4 (no failpoint needed -- the
corrupt file is written by hand, sibling to the unknown-schema-version
refusal test): a truncated/garbage __recovery/{ulid}.json must be
refused loudly by both the write-entry heal (the write fails naming
the parse error) and the open-time sweep (ReadWrite open fails naming
the file), with the file left on disk for operator inspection.
Read-only opens still work -- the sweep is skipped there.
* test(engine): run the S3 sidecar-lifecycle coverage in CI + document the fault matrix
- ci.yml rustfs_integration: new step running the bucket-gated
failpoints tests (name filter s3_) against the RustFS container, so
sidecar put/list/delete are exercised through S3StorageAdapter on
every storage-affecting PR.
- writes.md: sidecar I/O failure semantics -- Phase A put failure
aborts with zero drift; Phase D delete failure is swallowed (write
already durable) and healed by the next write; list failures are
loud at heal and open; corrupt sidecars are refused with the file
kept for inspection; audit-append failures are retried to exactly
one audit row.
- testing.md: index the storage-fault matrix in the failpoints.rs row
and the new RustFS CI line.
* test(engine): pin read-visibility of acknowledged local if-absent writes
The cluster lib test import_missing_state_creates_state_with_graph_-
observation flakes at ~50% under full-workspace load ('EOF while
parsing a value' reading back the state.json its own import just
acknowledged). Root cause is in the engine's local storage adapter:
write_text_if_absent writes through a buffered tokio::fs::File and
returns when write_all resolves -- which, per tokio's documented File
semantics, means the bytes reached tokio's internal buffer, not the
file. The actual write completes in a background blocking task after
drop, so a caller that acknowledges success and reads the object back
can see an empty or partial file. Under load the window widens; the
red run fails at iteration 0 with 0 of 8192 bytes on disk.
The regression test pins the contract at the adapter boundary: when
write_text_if_absent resolves, the full contents are visible to any
reader; a losing second claim leaves the winner's object untouched.
The fix lands in the next commit.
* fix(engine): publish local storage writes with atomic visibility
Close the class, not the instance. The local adapter admitted three
ways for a reader to observe a write that was acknowledged or visible
before its bytes were complete:
1. write_text_if_absent acknowledged success when the buffered
tokio::fs::File write_all resolved -- i.e. when the bytes reached
tokio's internal buffer, not the file. A caller reading back its own
acknowledged write could see an empty object (the ~50% cluster
import flake under full-workspace load; the regression test failed
at iteration 0 with 0 of 8192 bytes visible).
2. The same call published its CLAIM (create_new) before its CONTENT,
so concurrent readers saw an empty claimed file in the window.
3. write_text (plain tokio::fs::write) exposed truncated content
mid-replace -- silently falsifying write_sidecar's 'readers either
see the complete sidecar or none' contract on local FS (true on S3,
where PutObject is atomic).
A flush in write_text_if_absent would have fixed only (1). Instead,
both local write paths now publish complete temp files atomically:
rename for replace (write_text -- the idiom write_text_if_match
already used) and hard_link for no-replace (write_text_if_absent --
link fails AlreadyExists, so exactly one of N concurrent claimants
wins and the winner's object is fully readable at the instant it
becomes visible). The local adapter now honors the same object-level
atomic-visibility contract as the S3 adapter, which is what every
caller (recovery sidecar protocol, cluster state CAS) was written
against. Crash-orphaned *.tmp.* files are inert: the sidecar sweep
filters to .json, and cluster state reads address state.json by name.
fsync/durability policy is unchanged (no fsync before, none now);
this fix is about visibility ordering, not power-loss durability.
Pre-existing on main (landed with the multi-graph server mode change,
PR #119); surfaced by this branch's heal work only because one extra
list_dir per write shifted test timing. Cluster lib suite: 12/25
failures before, 0/25 after. Turns the previous commit's red test
green.
* refactor(engine): one storage implementation over object_store for every backend
Collapse LocalStorageAdapter (hand-rolled tokio::fs) and
S3StorageAdapter into a single ObjectStorageAdapter backed by
Arc<dyn object_store::ObjectStore> -- LocalFileSystem for local URIs,
the existing AmazonS3 build for s3://, plus a pub in_memory()
constructor (full contract including TRUE conditional updates; the
in-memory test backend testing.md asked for at the adapter level).
Why: the acknowledged-before-visible bug showed the two-impl shape has
no referee -- one prose contract, two independent answers. Upstream
LocalFileSystem::put_opts is byte-for-byte the staged-temp+rename/
hard_link idiom that fix converged on, and Lance's own commit protocol
is built on the same primitives (put-if-not-exists / rename-if-not-
exists), so the substrate-aligned move is to stop hand-rolling it.
The per-backend residue shrinks to a UriCodec (URI <-> object path)
and one capability flag.
Semantics preserved by construction, with three deliberate deltas:
- exists() is now object-store-semantics everywhere (head + non-empty
prefix fallback): an EMPTY local directory no longer 'exists'. The
only dir-shaped caller (_graph_commits.lance probes) self-heals via
ensure_commit_graph_initialized where it previously wedged loudly.
- A directory at an object path reads as NotFound, not as an IO error
('only objects exist'). The cluster unreadable-payload test used a
same-named directory as a portable non-NotFound trigger; it now uses
chmod 000, which still models genuine transient IO.
- write_text_if_match keeps content-token semantics on local
(PutMode::Update is NotImplemented upstream for LocalFileSystem in
0.12.5 and 0.13.2); the capability flag gates the token SOURCE in
read_text_versioned too -- an ETag token with content-compare writes
would lose every CAS.
delete_prefix keeps a local remove_dir_all branch: directories are a
local-FS concept, and list+delete would leave empty skeletons that
cluster graph_root_exists (raw Path::exists) reports as still present.
LocalStorageAdapter remains as a delegating shim so the pinned
contract tests gate this swap textually unchanged; the shim and the
test parameterization over local + in-memory land next. Cargo gains
the explicit 'fs' feature (already transitively enabled by lance).
* test(engine): one executable storage contract, run against every backend
Remove the LocalStorageAdapter delegation shim and migrate its
construction sites to ObjectStorageAdapter::local(). Replace the
per-backend duplicated tests with a single contract_suite asserting
the trait's promises (atomic replace, exists incl. the dataset-root
prefix probe, one-winner if_absent, versioned CAS with loud CAS-lost,
rename, list round-trip with no sibling-prefix bleed, idempotent
delete/delete_prefix), run against the local backend and the new
in-memory backend -- which implements true conditional updates, so the
strong-CAS path is exercised without a bucket. The bucket-gated S3
variant already exists (s3_adapter_conditional_writes_contract).
New local-specific pins for the deliberate semantic edges of the
collapse: empty directories are not objects (exists=false; the Lance
dataset-root probe shape is the non-empty case), file://-anchored and
spaces-in-path list output round-trips byte-identically into
read_text, dot-segment paths are lexically absolutized (the CLI's
./graph.omni shape), and upstream rename creating missing destination
parents. The acknowledged-write visibility regression test stays, now
documenting that the cross-API std::fs read-back is the point.
* refactor(cluster): drop put_json's per-backend atomicity branch
The local temp+rename dance predates the storage adapter guaranteeing
atomic visibility; now that write_text publishes via a staged temp +
rename on the filesystem (and a single atomic PUT on object stores) by
contract, the branch duplicated upstream behavior. One call, both
backends.
* docs: storage adapter collapse — contract, in-memory backend, local CAS gap
- testing.md: the 'no MemStorage backend' note is half-closed —
ObjectStorageAdapter::in_memory() covers the text-object layer with
the full contract (true conditional updates); Lance datasets bypass
the adapter, so the engine substrate ask stays open.
- invariants.md: truth-matrix Tests row updated; new Known Gap for
local write_text_if_match (upstream PutMode::Update is unimplemented
for LocalFileSystem; content-token emulation is safe only under the
cluster lock protocol — close before admitting a lock-free caller).
- writes.md: backend notes for the unified adapter (name#N staging
residue invisible to the sweep, backend-wrapped error text with
exists()-probing for missing-vs-error, loud permission failures).
* docs: finish renaming the storage adapters in user docs and test comments
storage.md's URI-scheme table and the S3 failpoint test's doc comment
still named the deleted LocalStorageAdapter/S3StorageAdapter; both now
describe the unified ObjectStorageAdapter over object_store, including
the relative-path absolutization note for local URIs.
* test(engine): pin branch-awareness of the drift guard's recovery advice
A pending sidecar on ANOTHER branch does not cover this branch's
drift: with a deferred feature-branch sidecar on disk and genuinely
uncovered drift on main, the main write's error must still point at
omnigraph repair -- a read-write reopen recovers the sidecar but
cannot repair main's uncovered drift. Currently red: the guard
matches sidecar pins by table_key only, so the feature sidecar flips
main's advice to the reopen path. Fix in the next commit.
Surfaced by external review of the drift-guard change.
* fix(engine): branch-aware sidecar matching in the drift guard's advice
The commit-time drift guard's sidecar-covered check matched pins by
table_key alone, so a pending sidecar on another branch flipped this
branch's uncovered-drift advice from 'run omnigraph repair' to the
reopen path -- and a reopen recovers that sidecar but cannot repair
this branch's drift. Compare the pin's table_branch too. Turns the
previous commit's red test green.
Surfaced by external review of the drift-guard change.
* test(engine): pin heal non-interference with a live schema apply
The write-entry heal's schema-staging reconcile runs before any queue
acquisition, so a load on the same handle, overlapping a schema apply
parked between its staging write and manifest commit, promotes the
apply's staging files (new catalog live against the old manifest),
classifies the LIVE apply's sidecar, and publishes its registrations
out from under it. The resumed apply then collides with its own stolen
commit. Currently red with:
Lance("Concurrent modification: table version 3 already exists for
node:Tag")
The fix (per-sidecar reconcile under the sidecar's write-queue guards,
plus a serialization key the schema-apply writer and the heal both
acquire) lands in the next commit.
Surfaced by external review of the write-entry heal.
* fix(engine): serialize the heal's schema-staging reconcile with live schema applies
The write-entry heal ran recover_schema_state_files up front, before
acquiring any queue guards. Overlapping a live schema apply parked
between its staging write and manifest commit, the heal promoted the
apply's staging files (new catalog live against the old manifest),
classified the LIVE apply's sidecar, and published its registrations —
the resumed apply then collided with its own stolen commit.
Correct by construction:
- New schema-apply serialization queue key, acquired by the schema-
apply writer (alongside its per-table keys) from before write_sidecar
until after delete_sidecar. Per-table keys alone don't cover a
registration-only migration, which pins no existing tables but has a
sidecar and staging files on disk.
- The heal reconciles schema staging lazily, PER SchemaApply sidecar,
after acquiring that sidecar's guards (including the serialization
key) and re-confirming the sidecar exists — a sidecar that survives
the queue wait belongs to a dead writer, so the reconcile can no
longer race a live apply. Recomputing per sidecar also removes the
staleness of one up-front result across a multi-sidecar pass.
- Omnigraph::refresh drops its up-front reconcile-and-pass-through
(same race, and a pre-promoted result would make the heal's guarded
reconcile see clean staging and wrongly defer the sidecar): it now
reconciles standalone only when NO sidecar exists — which cannot
race a live apply, whose sidecar always precedes its staging files —
and otherwise defers entirely to the heal.
The open-time sweep keeps its precomputed reconcile: open has no
concurrent writers. Turns the previous commit's red test green.
Surfaced by external review of the write-entry heal.
Self-audit addendum folded in: refresh's no-sidecar gate had a TOCTOU
(a live apply could write its sidecar + staging between the empty
check and the reconcile) — the standalone reconcile now holds the
serialization key across the list-then-reconcile pair. The remaining
residual is cross-process only (in-process queues cannot serialize
against a writer in another process; the open-time sweep has the same
pre-existing exposure) and is now an explicit Known Gap in
invariants.md rather than an implicit one.
* test(engine): pin catalog reload after the heal recovers a schema apply
When the write-entry heal rolls a crashed apply's SchemaApply sidecar
forward on the same handle, disk and manifest move to the new schema
(staging promoted, registrations published) but the handle's in-memory
schema_source/catalog do not. Subsequent writes then validate against
the stale catalog and reject rows of types the graph already has.
Currently red with:
record 1: unknown node type 'Tag'
refresh() reloads after its heal; the write entry points must too.
Fix in the next commit.
Surfaced by external review of the write-entry heal.
* fix(engine): reload the in-memory catalog after the heal recovers a schema apply
heal_pending_recovery_sidecars refreshed the coordinator and
invalidated the runtime cache after processing sidecars, but never
reloaded schema_source/catalog — so a write whose entry heal rolled a
crashed SchemaApply sidecar forward proceeded to validate against the
OLD schema while disk and manifest were already on the new one.
reload_schema_if_source_changed is the same post-heal step refresh()
already runs; it no-ops on the (overwhelmingly common) non-schema heal
because the on-disk source is unchanged. Turns the previous commit's
red test green.
Surfaced by external review of the write-entry heal.
* test(engine): pin that a deleted-branch sidecar cannot wedge the graph
A rollback-eligible sidecar pinned to a branch is deferred by every
roll-forward-only pass; if the branch is then deleted, the sidecar
survives, referencing a branch with no manifest tree. The heal (every
write entry) and the open-time sweep (every ReadWrite open) both fail
opening the dead branch, and repair refuses while a sidecar is pending
-- a terminal read-only state with manual sidecar surgery as the only
exit. Currently red with:
Lance("Not found: .../__manifest/tree/feature/_versions")
The branch's tree and forks are already reclaimed, so the pinned drift
is unreachable and the sidecar is provably moot; the fix classifies it
as an orphaned-branch terminal state (audit + discard) in both passes.
Surfaced by review (P1, verified by repro).
* fix(engine): classify deleted-branch sidecars as orphaned instead of wedging
A deferred (rollback-eligible) sidecar pinned to a branch survives
branch_delete; both the write-entry heal and the open-time sweep then
failed unconditionally opening the dead branch -- every write and
every ReadWrite open errored, and repair refuses while a sidecar
pends. Terminal state, manual sidecar surgery the only exit.
The branch's tree and per-table forks are already reclaimed at delete,
so the drift the sidecar pins is unreachable and the sidecar is
provably moot. Both passes now check the sidecar's branch against the
manifest's branch list (the authority -- deliberately NOT inferred
from a Not-found on open, which could be a transient storage error
masking real recovery intent) and discard orphans with an
OrphanedBranchDiscarded audit row, commit appended on main since the
sidecar's own branch no longer has a commit graph.
The open-time half is pre-existing; the write-entry heal made it hot.
Turns the previous commit's red test green.
Surfaced by review (P1, verified by repro).
* chore: harden review nits — vacuous CI filter, root-runner skip, liveness note
- ci.yml: the RustFS sidecar-lifecycle step now fails loudly if the
's3_' name filter matches zero tests (cargo passes vacuously on an
empty filter; the step exists specifically to prove S3 sidecar I/O
coverage). The pre-existing CLI smoke step has the same shape and is
left for a follow-up.
- cluster unreadable-payload test: cfg(unix) + a skip-with-log when
running as root (mode 000 is still readable to root, common in
container dev runners), so the test degrades instead of failing.
- refresh: document the one-pass-late convergence for legacy staging
residue while non-SchemaApply sidecars pend, so nobody 'fixes' it by
re-running the reconcile unserialized — the exact race the
serialization key closes.
* test(engine): pin orphan-discard idempotency across a delete fault
discard_orphaned_branch_sidecar writes its audit row and main commit
before deleting the sidecar; a Phase D delete fault leaves the sidecar
on disk with the audit already durable, and the retry repeated the
whole path -- a second OrphanedBranchDiscarded audit row (and commit)
for the same operation. Currently red: 2 rows after one fault + retry.
The retry must only finish the delete. Fix next.
Also promotes the recovery-audit kinds reader into the shared test
helpers (it was recovery.rs-local).
Surfaced by external review of the orphan-discard fix.
* fix(engine): orphan-discard idempotency + heal reports acted-vs-deferred
Two review findings on the recovery surface:
- discard_orphaned_branch_sidecar now checks the audit table for an
existing (operation_id, OrphanedBranchDiscarded) row before appending
the commit + audit pair, so a Phase D delete fault retries ONLY the
delete instead of duplicating audit rows and commit-graph entries.
Cold path: the list scan runs only when an orphaned sidecar exists.
Turns the previous commit's red test green (exactly one audit row
across fault + retry).
- process_sidecar returns whether durable state changed; the heal sets
processed_any only for sidecars that were actually rolled forward /
rolled back / audit-recovered (orphan discards count). Deferred
sidecars (rollback-eligible, invariant-violating, unpromoted
SchemaApply) no longer trigger a per-write schema reload + full
runtime-cache invalidation while they pend -- the cache is
snapshot-keyed so this was waste, not corruption, but it was paid on
every write until reopen. Acted-paths' processed=true remains pinned
by load_after_schema_apply_phase_b_failure_uses_recovered_catalog
(the reload depends on it).
Surfaced by external review.
* test(engine): pin the orphan-discard audit-append fault leg as documented tolerance
The orphan discard's commit append and audit append are two writes; a
failure between them leaves a recovery commit with no audit row, and
the retry (keyed on the audit row, the operator-facing record) appends
a second commit before the audit lands. This is the same
not-atomic-pair-write tolerance record_audit documents and the
manifest->commit-graph Known Gap covers for every publish: bounded
commit-graph noise, audit row exactly-once under clean failures.
Keying idempotency on commit rows instead would need an operation_id
column on _graph_commits, and audit-before-commit would dangle the
graph_commit_id join -- both worse than the documented residual.
Make the tolerance explicit instead of implicit: docstring names the
window, a failpoint sits inside it, and the new test pins convergence
across the fault (sidecar consumed, exactly one audit row), completing
the orphan-discard fault matrix alongside the delete-fault leg.
Surfaced by external review of the orphan-discard idempotency.
* test(engine): pin honest drift-guard advice when sidecar listing fails
The guard's unwrap_or(false) conflated 'classified as uncovered' with
'could not classify': a transient list fault on the guard's second
list (the entry heal's first list having succeeded) confidently routed
the operator to omnigraph repair even when the heal had just deferred
a rollback-eligible sidecar -- and repair refuses while a sidecar is
pending. Currently red: the error says 'run omnigraph repair' with no
mention of the reopen path. The fix names both paths plus the failure
cause when classification is impossible.
Surfaced by external review of the drift-guard fallback.
* fix(engine): admit ambiguity in the drift guard when sidecar listing fails
Replace the unwrap_or(false) fallback with a tri-state: covered ->
reopen advice; uncovered -> repair advice; listing FAILED -> say the
drift could not be classified, name the cause, and give both paths in
order ('run repair, or reopen read-write if repair reports a pending
sidecar'). The old fallback confidently routed a transient list fault
to repair, which refuses while a sidecar is pending -- a self-
correcting but pointless detour. The conflict itself is still always
raised; only the advice degrades honestly. Turns the previous commit's
red test green.
Surfaced by external review of the drift-guard fallback.
|
||
|
aaltshuler
|
08c9b03d40 |
test(cli): the embedded/remote parity matrix (RFC-009 Phase 1)
The referee before any unification moves: every forked verb runs once against the local graph and once against a spawned server on a twin copy of the same fixture, with the SAME actor (--as locally; bearer-resolved remotely) and the SAME Cedar bundle on both arms — like-for-like enforcement is part of the harness (a tokens-only server is default-deny by design; comparing that against a bare local arm measures configuration, not the fork). Declared-volatile fields (ids, wall-clock, transport locations) scrub to placeholders; everything else must match exactly, and exit codes must match for shared failures. Headline result: 11 rows green with an EMPTY divergence ledger — the arms agree on every verb today. The ledger (KNOWN_DIVERGENCES) exists so any future divergence is pinned or filed, never silently repaired; repairs are Phase 3's job, gated by this referee staying green. One engine observation surfaced and filed (#207): inline execution with a declared-but-unbound param matches ALL rows on both arms, while the stored-query invoke path hard-errors — a cross-path asymmetry the matrix pins as agreeing behavior pending a deliberate fix. Documented exclusions (graphs list, ingest/load-over-/ingest, storage-plane verbs) map to RFC-009 Phases 4-5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrew Altshuler
|
e0d80c0062
|
Merge pull request #206 from ModernRelay/rfc/unify-access-paths
docs(rfc): RFC-009 — unify CLI access paths; align the RFC corpus |
||
|
aaltshuler
|
9002cfd5b9 |
docs(rfc): RFC-009 — unify CLI access paths; align the RFC corpus
Adopts the unify-embedded/remote draft as RFC-009 with three alignment amendments: (1) the promised 'companion config-authority RFC' is RFC-008, already landed through stage 4 — referenced, not re-proposed; (2) open question 3 is answered by the two-surface architecture (embedded graphs list enumerates the cluster catalog via read_serving_snapshot, never omnigraph.yaml); (3) Phase 2 salvages PR #139's reviewed-clean omnigraph-api-types extraction instead of rebuilding. Adds the cycle's two no-referee bugs (alias positional, write-if-absent flush) as concrete parity-matrix motivation, and RFC-007's addressing/credential chains as RemoteClient constructor inputs. Corpus alignment: RFC-002's header now maps each of its pieces to the successor that landed or superseded it (007/008/009) with a do-not- implement-from-here-unchecked warning; RFC-007 gains the RFC-009 relationship; RFC-008 stage 5 notes the Phases-4/5 easing; dev index row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
98c6147c38 |
docs(testing): bring the test map up to release truth
Lands an orphaned-but-accurate working-tree edit (the engine table rows for forbidden_apis.rs, lance_surface_guards.rs, traversal_indexed, proptest_equivalence, ordering, literal_filters, policy_engine_chassis — all real files; 21 -> 28 count) and replaces the stale pre-modularization crate rows: the CLI and server entries now describe the per-area suites (#192/#193 splits) plus this cycle's additions (RFC-008 deprecation coverage, keyed-credential auth, hermetic OMNIGRAPH_HOME harness, the bucket-gated s3 suites). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
b24bb16d0c |
ci(codeowners): restore ragnorc to engineering and docs roles
Re-adds ragnorc to both roles in the source of truth and regenerates CODEOWNERS + the ownership tables. This also resolves the standing inconsistency from #169: branch-protection.json's bypass_pull_request_allowances still listed ragnorc after his codeowners removal — the two lists are in sync again (no protection change needed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
4c50170c77 |
feat(config): OMNIGRAPH_NO_LEGACY_CONFIG strict mode (RFC-008 stage 4)
Opt-in: with the env set, loading a legacy omnigraph.yaml is a hard error pointing at config migrate — the regression guard for migrated teams (a stray legacy file would otherwise silently outrank operator config during the window) and the rehearsal for stage 5's removal. Strict refuses the FILE, never its absence: flag-less invocations on migrated setups are untouched. Inert unless set. The RFC's stages-1-3-then-4 release gap collapsed honestly: no version boundary was crossed between them, so all four ship in the same release (noted in the RFC). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
5328c91341 |
refactor(cli): drop cluster init — no replacement scaffold
Andrew's call, and the right one by the repo's own lens: a minimal
cluster.yaml is five lines; a generator is a second copy of the schema to
keep in sync forever, emitting a file that is unusable until hand-edited
anyway (graphs: {} cannot apply or serve). Terraform has no config
scaffolder either. New users copy from the cluster quick-start; migrants
get a ready-to-review cluster.yaml from config migrate. RFC-008 stage 3
becomes purely subtractive.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
aaltshuler
|
3adbc65af2 |
docs(cli): config migrate, cluster init, the legacy-file deprecation notice
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
dc91c55970 |
feat(cli): operator aliases — pure bindings invoking stored queries (RFC-007 PR 3, part 2)
aliases: in the operator config bind a personal name to (server, graph,
stored-query NAME, positional arg mapping, fixed param defaults, format)
— zero content, per the ratified bindings-not-content model. Invocation
goes through the server's stored-query endpoint (POST
{base}/graphs/{g}/queries/{name}) with the keyed credential resolving via
the ordinary URL match; param precedence --params > positionals > fixed
defaults; the result renders through the existing format cascade with the
alias's format as its hop. A legacy omnigraph.yaml alias with the same
name wins during the RFC-008 window, with a warning naming both.
E2e (spawned policy-gated server, invoke_query granted via a per-graph
bundle): the alias invokes with name + one positional and nothing else —
server, graph, query, and token all from the operator layer; --server/
--graph explicit targeting; unknown --server lists defined names;
--server exclusive with a positional URI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
aaltshuler
|
65160cc060 |
docs(rfc): aliases are bindings, not content — the ratified alias model
RFC-007 §D2 gains the model the alias design reasoned through: stored queries are content + its canonical team-owned name; legacy omnigraph.yaml aliases conflate a personal name with a local-file content pointer (the muddle RFC-008 retires); operator aliases are pure bindings (server, graph, stored-query NAME, arg mapping, defaults) — an alias that carries content competes with the catalog, one that references a name composes with it. The three senses of 'global' are resolved explicitly: cross-graph globality is strengthened (one $HOME file vs per-directory), team-shared shorthand is deliberately NOT an alias mechanism (the shared name IS the catalog name), cross-machine follows the dotfile. Collision rule: legacy wins during the RFC-008 window, with a warning. RFC-008's migration row for aliases sharpens accordingly: a legacy alias splits — content to the catalog (via cluster apply), binding to the operator layer; config migrate proposes both halves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
a819ab500e |
feat(cli): keyed credentials — servers:, the token chain, login/logout (RFC-007 PR 2)
The operator config gains servers: (name -> url; never a token). A remote command whose URL prefix-matches an operator server resolves its bearer token through the keyed chain first — OMNIGRAPH_TOKEN_<NAME> env, then the [<name>] section of ~/.omnigraph/credentials (created 0600 via temp+rename, #139 finding 7; group/world-readable files refused loudly) — falling through to the legacy chain unchanged. URL keying makes §D5 rule 3 structural: a token is only ever sent to the server it is keyed to. Longest-prefix matching with a path-boundary check (http://h:8080 never matches http://h:8080-evil). Inserting the keyed hop above the legacy chain is safe by construction — no existing setup can have servers: defined. omnigraph login <name> stores/rotates one section (token from --token or one stdin line — the pipe flow keeps secrets out of shell history); omnigraph logout removes it, idempotently; logging in before declaring the server warns instead of failing (the gh model). Coverage: URL-match/no-substring-trap, credentials round-trip preserving sibling sections, 0600 write + over-permissive refusal, env-name mapping; the legacy resolve test is now hermetic against a real ~/.omnigraph and asserts byte-identical legacy behavior with no servers defined; one spawned-binary e2e walks the whole lifecycle against an authed server: refusal -> wrong-token login (stdin) -> rotate (--token) -> authorized read -> env-beats-file -> non-matching-URL negative -> logout revokes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
08ce8dc34d |
docs(rfc): align RFC-007 with RFC-008's two-surface architecture
RFC-007 now speaks the end-state language throughout: the operator surface is one half of the two-surface split (cluster config / operator config), not a layer over a living omnigraph.yaml. The precedence cascade drops the project layer (cluster config carries no operator-resolvable keys — a checkout can never supply identity); legacy omnigraph.yaml appears only as the RFC-008 deprecation-window slot. The trust boundary is restated as closed-by-construction in the end state, with the rules governing the window. PR 3 becomes operator targeting (--server + operator aliases — the replacement RFC-008 needs before legacy aliases migrate), and the schema example gains the aliases block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
320311e759 |
docs(rfc): RFC-008 — deprecate omnigraph.yaml, one concern per config surface
The file is three unrelated concerns wearing one filename — server deployment config, project/CLI conveniences, operator identity — and the mixture is the root cause of a recurring problem class (per-operator copies of project files, checkout-supplied credential redirection, init scaffold pollution). End state: two single-owner surfaces — cluster config (team, repo) and operator config (person, $HOME) — plus the zero-config flags/env tier. Complete key-by-key migration map over the verified OmnigraphConfig surface; staged retirement per the repo's Hyrum rules (warn with per-key guidance -> `config migrate` tool -> stop scaffolding -> opt-in strict -> removal at the next major). RFC-007's project-layer framing is amended to transitional accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
d531f60999 |
docs(rfc): RFC-007 — per-operator config, the operator slice of RFC-002
Terraform-style operator/project split: ~/.omnigraph/config.yaml for identity (operator.actor in the --as cascade), credentials keyed by server name (env -> 0600 credentials file; no inline secrets), and operator-owned named servers that project configs reference but cannot redefine. Explicitly a staged subset of RFC-002: adopts its settled decisions (one dir, keyed credentials, env precedence), defers GraphLocator/use/state-layer, and encodes the ten confirmed PR #139 findings as design rules (compat shims, key-level merges, atomic writes, the project-layer trust boundary). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
aaltshuler
|
8d7aed065f |
test(cluster,server): gated object-storage cluster e2e + CI wiring + docs
s3_cluster.rs runs the full control-plane lifecycle against a real bucket (CI: containerized RustFS; locally the RustFS binary): import → lock released (pins the drop-time release regression caught on the first live smoke) → apply (graph roots + catalog on the bucket, nothing local) → serving snapshots from both the config dir and the bare URI → schema evolution → approved delete (prefix removal) → empty-cluster refusal. The server suite gains the config-free boot test: --cluster s3://… with zero local files serves a stored query over HTTP. CI: the rustfs job runs both suites; the classify filter covers the cluster store/serve modules and the new test files. The server smoke drops its name filter — every test in the s3 target is bucket-gated, and a filter matching nothing passes vacuously (which silently ran zero tests for a while). Docs: deployment.md gains the Bucket-no-volume shape as the preferred cloud deployment; cluster.md/server.md document --cluster <uri>; testing.md maps the new suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |