Repository navigation
Implement RFC-022 unified graph write protocol - #343
Merged
Merged
Conversation
aaltshuler
marked this pull request as ready for review
July 11, 2026 10:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
read_set_conflict(409) andrecovery_required(503) HTTP responsesWhy
The previous path did not capture branch identity, graph lineage, schema authority, and table versions as one coherent validation token. A publisher retry could therefore reparent validation-sensitive work, recovery sidecars could not always prove ownership of the exact Lance effects they observed, and native branch/control operations had same-process ABA and stale-catalog windows.
RFC-022 defines one correctness protocol while preserving writer-specific Lance effect adapters. This slice implements the mutation/load adapter and closes the adjacent control-plane race windows needed to make that adapter safe.
User and developer impact
Validation
cargo test --workspace --lockedcargo test --locked -p omnigraph-engine --features failpoints --test failpoints -- --test-threads=1— 80/80scripts/check-agents-md.sh— 67 links across 63 docsgit diff --checkCurrent boundary
RFC-022 and its follow-up RFCs remain drafts. The ordered gates in this slice are process-local: unrestricted multi-process writers against the same graph still require a substrate-native conditional-reference primitive or distributed fence. Branch merge uses a hardened authority bridge in this PR; it is not yet the final unified exact-effect adapter.
Greptile Summary
This PR implements the RFC-022 unified graph write path. The main changes are:
Confidence Score: 4/5
This is close, but the schema-apply API contract should be fixed before merging.
crates/omnigraph-server/src/handlers.rs and crates/omnigraph-server/tests/openapi.rs
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant Server as schema/apply handler participant Engine as schema write path participant API as error mapping Client->>Server: "POST /graphs/{graph_id}/schema/apply" Server->>Engine: apply schema Engine-->>Server: recovery required Server->>API: convert engine error API-->>Client: 503 ErrorOutput with recovery_required%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client participant Server as schema/apply handler participant Engine as schema write path participant API as error mapping Client->>Server: "POST /graphs/{graph_id}/schema/apply" Server->>Engine: apply schema Engine-->>Server: recovery required Server->>API: convert engine error API-->>Client: 503 ErrorOutput with recovery_requiredComments Outside Diff (2)
crates/omnigraph/src/db/manifest/recovery.rs, line 2319-2328 (link)When this convergence path finds the sidecar intent already satisfied, it records the audit against the latest
omnigraph:recoverycommit rather than the commit that satisfied this operation. If another recovery commit lands first, the audit row for this sidecar can point at an unrelated graph commit, so operators following the audit trail see the wrong recovery commit for the operation.crates/omnigraph-server/src/handlers.rs, line 1136 (link)server_schema_applystill callsapply_schema_as_with_catalog_check()throughApiError::from_omni, so a recovery-blocked schema migration can return the new 503ErrorOutputwithrecovery_required. This route does not list 503 in its OpenAPI responses, while the other recovery-capable write routes do. Clients generated from the spec can treat a valid schema-apply recovery response as an unexpected status or body shape instead of exposing the operation id needed to recover.Context Used: AGENTS.md (source)
Reviews (2): Last reviewed commit: "Preserve recovery error wire compatibili..." | Re-trigger Greptile
Context used: