From 52f28cebe889a985fa443a265461ce9a62f7e1a2 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Wed, 27 May 2026 11:57:04 +0200 Subject: [PATCH] mr-668: comment cleanup and policy format style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strip "PR Na/Nb" sub-PR references throughout MR-668 surfaces — they were useful during the 10-PR delivery sequence but rot now that the work is in the tree. Keep the MR-668 umbrella references. Also: - Add explicit `when = when` and `resource_literal = resource_literal` named args in `compile_policy_source`'s outer `format!` to match the surrounding crate style (already explicit for `group` and `action`). - Rename the best-effort cleanup tracing target from "omnigraph::init" to "omnigraph::init::cleanup" so operators can filter init-failure cleanup events separately from init's other log lines. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/omnigraph-policy/src/lib.rs | 8 +++++--- crates/omnigraph-server/src/api.rs | 2 +- crates/omnigraph-server/src/lib.rs | 12 ++++++------ crates/omnigraph-server/src/registry.rs | 2 +- crates/omnigraph-server/tests/openapi.rs | 2 +- crates/omnigraph-server/tests/server.rs | 2 +- crates/omnigraph/src/db/omnigraph.rs | 2 +- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/crates/omnigraph-policy/src/lib.rs b/crates/omnigraph-policy/src/lib.rs index eaec378..79563c7 100644 --- a/crates/omnigraph-policy/src/lib.rs +++ b/crates/omnigraph-policy/src/lib.rs @@ -591,7 +591,7 @@ fn compile_entities(config: &PolicyConfig, graph_id: &str, schema: &Schema) -> R entities.extend(actor_entities); entities.push(graph_entity); - // MR-668 PR 6a: include the `Omnigraph::Server::"root"` entity + // MR-668: include the `Omnigraph::Server::"root"` entity // whenever any rule references a server-scoped action. Cedar's // schema validator will otherwise reject the policy. Keeping this // conditional (rather than always-on) avoids polluting test @@ -648,7 +648,7 @@ fn compile_policy_source(rule: &PolicyRule, action: &PolicyAction, graph_id: &st format!("\nwhen {{ {} }}", conditions.join(" && ")) }; - // MR-668 PR 6a: emit the resource literal that matches the action's + // MR-668: emit the resource literal that matches the action's // `resource_kind`. Per-graph actions reference the engine's // `Omnigraph::Graph::""` instance; server-scoped // actions reference the singleton `Omnigraph::Server::"root"`. @@ -669,6 +669,8 @@ fn compile_policy_source(rule: &PolicyRule, action: &PolicyAction, graph_id: &st ){when};"#, group = cedar_literal(&rule.allow.actors.group), action = cedar_literal(action.as_str()), + when = when, + resource_literal = resource_literal, ) } @@ -697,7 +699,7 @@ fn target_branch_scope_condition(scope: PolicyBranchScope) -> String { } fn policy_schema_source() -> &'static str { - // MR-668 PR 6a: `entity Server;` plus the `graph_list` action that + // MR-668: `entity Server;` plus the `graph_list` action that // binds to it. Per-graph actions stay bound to `Graph`. // The Cedar schema string lives here (not on a fixture file) so any // omnigraph-policy build picks up the new vocabulary in lock-step diff --git a/crates/omnigraph-server/src/api.rs b/crates/omnigraph-server/src/api.rs index c59d036..24374fa 100644 --- a/crates/omnigraph-server/src/api.rs +++ b/crates/omnigraph-server/src/api.rs @@ -468,7 +468,7 @@ pub fn read_target_output(target: &ReadTarget) -> ReadTargetOutput { } } -// ─── MR-668 PR 6b — management endpoint shapes ───────────────────────────── +// ─── MR-668 — management endpoint shapes ────────────────────────────────── /// One entry in the response from `GET /graphs`. Cluster operators /// consume this list to discover which graphs the server is currently diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 6d0842f..e49555b 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -157,7 +157,7 @@ pub enum ServerConfigMode { /// non-empty `graphs:` map and no single-mode selector. Multi { /// Per-graph startup configs, sorted by graph id (BTreeMap - /// iteration order). PR 5's parallel-open loop iterates this. + /// iteration order). The parallel-open loop iterates this. graphs: Vec, /// Path to the config file the server was started from. Kept on /// the mode so future runtime mutation (deferred — see release @@ -214,8 +214,8 @@ pub struct AppState { /// Topology + (single mode only) the single graph's URI for /// startup wiring. The registry below is the runtime source of truth. mode: ServerMode, - /// PR 2 (MR-686) + PR 4a (MR-668): the engine and per-graph policy - /// now live inside `GraphHandle`s in the registry. Reads via + /// MR-686 + MR-668: the engine and per-graph policy live inside + /// `GraphHandle`s in the registry. Reads via /// `ArcSwap` are lock-free; mutations (currently only `insert`) /// serialize through the registry's internal mutex. registry: Arc, @@ -433,7 +433,7 @@ impl AppState { } } - /// Multi-mode constructor — used by PR 5's startup loop. Operators + /// Multi-mode constructor — used by the startup loop. Operators /// reach this by invoking `omnigraph-server --config omnigraph.yaml` /// with a non-empty `graphs:` map. /// @@ -1204,7 +1204,7 @@ async fn server_openapi(State(state): State) -> Json` as an extension so handlers can /// extract it via `Extension>`. /// diff --git a/crates/omnigraph-server/src/registry.rs b/crates/omnigraph-server/src/registry.rs index 2140a93..b1c879e 100644 --- a/crates/omnigraph-server/src/registry.rs +++ b/crates/omnigraph-server/src/registry.rs @@ -1,4 +1,4 @@ -//! `GraphRegistry` — the multi-graph routing substrate (MR-668 PR 3). +//! `GraphRegistry` — the multi-graph routing substrate (MR-668). //! //! Holds the open `Arc` for every graph the server is currently //! serving. Lock-free reads via `ArcSwap`; mutations diff --git a/crates/omnigraph-server/tests/openapi.rs b/crates/omnigraph-server/tests/openapi.rs index e93e11b..4945998 100644 --- a/crates/omnigraph-server/tests/openapi.rs +++ b/crates/omnigraph-server/tests/openapi.rs @@ -960,7 +960,7 @@ fn openapi_spec_is_up_to_date() { } // --------------------------------------------------------------------------- -// MR-668 PR 4b — multi-mode OpenAPI cluster filter +// MR-668 — multi-mode OpenAPI cluster filter // --------------------------------------------------------------------------- // // In multi-graph mode, `/openapi.json` reports cluster routes diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index ed5088d..4599250 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -4334,7 +4334,7 @@ async fn schema_apply_route_additive_property_preserves_existing_rows() { ); } -// ─── MR-668 PR 5: multi-graph startup ───────────────────────────────────── +// ─── MR-668: multi-graph startup ────────────────────────────────────────── mod multi_graph_startup { use super::*; diff --git a/crates/omnigraph/src/db/omnigraph.rs b/crates/omnigraph/src/db/omnigraph.rs index 7bc4e12..f3df8fb 100644 --- a/crates/omnigraph/src/db/omnigraph.rs +++ b/crates/omnigraph/src/db/omnigraph.rs @@ -1551,7 +1551,7 @@ async fn best_effort_cleanup_init_artifacts(root: &str, storage: &dyn StorageAda ] { if let Err(err) = storage.delete(&uri).await { tracing::warn!( - target: "omnigraph::init", + target: "omnigraph::init::cleanup", uri = %uri, error = %err, "init failed; best-effort cleanup could not delete artifact",