feat(cli): cluster-managed maintenance addressing + init signpost (RFC-010 Slice 3) - #221
Merged
Merged
Conversation
…lice 3) Public helper the CLI uses to refuse `init` into a cluster-managed location: given a graph storage URI of the cluster layout (`<root>/graphs/<id>.omni`), return the cluster root if `<root>` holds `__cluster/state.json`, else None. Cheap by construction — a URI that doesn't match the `<root>/graphs/<id>.omni` shape returns None with zero I/O, so ordinary `init` targets never probe storage. Works for file:// and s3:// via the storage adapter. Adds two ClusterStore accessors (`display_root`, `has_state`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…C-010 Slice 3) Two cluster-graph-aware CLI behaviors, sharing the cluster-resolution path. Maintenance addressing. `optimize`/`repair`/`cleanup` gain `--cluster <dir|s3://…> --cluster-graph <id>`, which resolves the graph's storage URI from the served cluster snapshot (the same truth a `--cluster` server boots from — `read_serving_snapshot*`) and opens it embedded. The operator no longer hand-types `<storage>/graphs/<id>.omni`. A distinct flag is required because the global `--graph` is `requires = server` and means a remote multi-graph id. clap enforces both-or-neither and exclusion with the positional URI / `--target`; an unserved graph errors loudly, pointing at `cluster apply`. init signpost. `init` refuses a cluster-managed positional path (the `<root>/graphs/<id>.omni` layout where `<root>` holds `__cluster/state.json`, detected by `cluster_root_for_graph_uri`) and points at `cluster apply` — graphs in an established cluster are created with ledger/recovery/approvals, not by hand. The check is gated on the path shape, so ordinary `init` does no extra I/O and existing pre-apply cluster-graph inits are unaffected. planes guard remediation now also mentions `--cluster … --cluster-graph …` (the two Slice-1 guard-string tests track it). Docs updated (cli-reference Command planes, maintenance.md, cluster.md §7); the stale "no S3-hosted cluster directories" limitation is dropped (RFC-006 landed it). Tests (cli_cluster.rs, reusing the apply-a-cluster fixture): resolve by id, unknown-id error, `--cluster` requires `--cluster-graph`, init refusal + signpost, and ordinary init still works. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng snapshot Addresses the Greptile review on #221. `read_serving_snapshot*` does all-or-nothing serving validation — recovery-sidecar checks plus a digest verify of every catalog payload (query .gq, policy blobs). Using it to resolve a maintenance target coupled `optimize`/`repair`/`cleanup` to the readiness of unrelated resources: a single corrupt policy blob, or a pending recovery sweep, would block the command before it could touch the graph — worst for `repair`, the tool you reach for *when the cluster is degraded*. Add `omnigraph_cluster::resolve_graph_storage_uri(cluster, graph_id)`: read the state ledger, confirm the graph is in the applied revision, return `graph_root(id)` — the URI is deterministically derivable, no catalog validation. The CLI's cluster resolver now calls it. Test: `optimize --cluster … --cluster-graph …` still resolves after the catalog payloads (`__cluster/resources/`) are removed — the ledger-only path is not blocked by degraded/unrelated catalog state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
The last two RFC-010 items, both about cluster-managed graphs, sharing the cluster-resolution path. Completes RFC-010.
feat(cluster): cluster_root_for_graph_uri helperGiven a graph storage URI of the cluster layout (
<root>/graphs/<id>.omni), returns the cluster root if<root>holds__cluster/state.json, elseNone. Cheap by construction — non-cluster-shaped URIs returnNonewith zero I/O. Worksfile://+s3://via the storage adapter. Unit-tested.feat(cli): maintenance addressing + init signpostMaintenance addressing —
optimize/repair/cleanupgain--cluster <dir|s3://…> --cluster-graph <id>, which resolves the graph's storage URI from the served cluster snapshot (read_serving_snapshot*— the same truth a--clusterserver boots from) and opens it embedded, so the operator needn't hand-type<storage>/graphs/<id>.omni. A distinct flag is required: the global--graphisrequires=serverand means a remote multi-graph id. clap enforces both-or-neither + exclusion with the positional URI /--target; an unserved graph errors loudly, pointing atcluster apply.initsignpost —initrefuses a cluster-managed positional path (the<root>/graphs/<id>.omnilayout where<root>holds__cluster/state.json) and points atcluster apply— graphs in an established cluster are created with ledger/recovery/approvals, not by hand. Gated on the path shape, so ordinaryinitdoes no extra I/O and pre-apply cluster-graph inits are unaffected.Guard remediation now also mentions
--cluster … --cluster-graph …. Docs updated (cli-reference Command planes,maintenance.md,cluster.md§7); the now-stale "no S3-hosted cluster directories" limitation is dropped (RFC-006 landed it).Open question resolutions (deferred from earlier slices)
<storage>/graphs/<id>.omni). Resolve via the serving snapshot (applied state); a not-yet-applied graph is correctly unresolvable.--graphflag collision: a distinct--cluster-graphflag (global--graphcan't be reused).Verification
5 new
cli_clustertests (resolve-by-id, unknown-id, flag-requires, init-refusal, init-still-works) reusing the apply-a-cluster fixture; the two Slice-1 guard-string tests updated;parity_matrixunaffected; fullcargo test --workspace --locked→ exit 0.🤖 Generated with Claude Code
Greptile Summary
This PR completes RFC-010 by adding cluster-managed maintenance addressing (
--cluster/--cluster-graphonoptimize/repair/cleanup) and aninitsignpost that refuses to create graphs inside an established cluster's storage layout. The cluster URI resolution deliberately uses a lightweight ledger-only read (not the full serving-snapshot validation) so maintenance works even on a degraded cluster.resolve_graph_storage_urireads only the state ledger to derive<storage>/graphs/<id>.omni, deliberately skipping catalog-payload and recovery-sidecar checks that would blockrepairon a degraded cluster — the previous reviewer concern is addressed and a dedicated robustness test is added.cluster_root_for_graph_urigates on the<root>/graphs/<id>.omnishape before doing any I/O, keeping ordinaryinittargets zero-cost; theinitsignpost fires only when the ledger probe confirms an established cluster.guard_addressinginplanes.rsnow advertises--cluster/--cluster-graphin its catch-all storage-plane remediation, butschema plan,queries validate, andlintare also covered by that arm and did not receive those flags.Confidence Score: 4/5
Safe to merge for maintenance commands and init; the guard message in planes.rs misdirects users of schema plan, queries validate, and lint toward flags those commands don't have.
The core feature is correctly implemented and well-tested. One concrete defect in planes.rs: the catch-all wrong-plane remediation was updated to mention --cluster/--cluster-graph, but that arm also covers schema plan, queries validate, and lint which lack those flags. A user who accidentally attaches --server to one of those commands will receive guidance pointing at options that clap will reject.
crates/omnigraph-cli/src/planes.rs — the _ arm of guard_addressing and the matching assertion in crates/omnigraph-cli/tests/cli_schema_config.rs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["CLI: optimize / repair / cleanup"] --> B{cluster + cluster-graph?} B -- yes --> C["resolve_storage_uri (cluster path)"] B -- no --> D["resolve_storage_uri (normal path)"] C --> E["resolve_cluster_graph_uri"] E --> F["resolve_graph_storage_uri"] F --> G["ClusterStore"] G --> H["read_state (ledger only, no catalog validation)"] H --> I{graph id applied?} I -- yes --> J["graph_root(id) -> storage URI"] I -- no --> K["Diagnostic: graph_not_applied"] J --> L["Omnigraph::open(uri)"] D --> L M["CLI: init URI"] --> N["cluster_root_for_graph_uri"] N --> O{URI shape: /graphs/id.omni?} O -- no --> Q["proceed (zero I/O)"] O -- yes --> P["has_state probe"] P -- no --> Q P -- yes --> R["bail: use cluster apply"]Reviews (2): Last reviewed commit: "fix(cli): resolve cluster graphs from th..." | Re-trigger Greptile