Skip to content

feat(policy)!: gate maintenance behind the maintain and reclaim actions - #1175

Draft
ragnorc wants to merge 6 commits into
ModernRelay:mainfrom
ragnorc:feat/maintenance-policy-actions
Draft

ragnorc wants to merge 6 commits into
ModernRelay:mainfrom
ragnorc:feat/maintenance-policy-actions

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Update, 2026-10-29. #1168 is closed. RFC #1190 (remote optimize) now depends on this PR for the maintain action and optimize_as. The merge of main brings in omnigraph index build from #1150, which checked change. It now checks maintain like the other index writers: that change gate never shipped in a release, so it needs no deprecation, unlike the full-text rebuild's. ensure_indices_on_as returns main's IndexBuildResult, and #1150's assertions on the built indexes and the commit's actor move into maintenance_entry_points_enforce_maintain_on_their_branch_before_effects_issue_840. The release notes and the maintenance page are brought within their caps. After the merge I built and tested the engine locally: workspace Clippy with failpoints passes, as do policy_engine_chassis (27), forbidden_apis (37), schema_apply (41) and the maintenance:: and branching:: integration modules (110).

Phase A, step 1 ("Policy") of docs/rfcs/2026-10-26-served-graph-maintenance.md (#1168). No maintenance entry point checks policy today: optimize, cleanup, ensure_indices, ensure_indices_on and repair take no actor, and repair with confirm publishes under hard-coded actor strings, so an embedded program with a policy checker installed can compact, rebuild indexes and delete history around the gate that invariant 10 requires of every mutating entry point (fact F7 in the RFC). Served maintenance, the later Phase A steps, needs a grant to check before it can have a route.

Two Cedar actions join the vocabulary, following decisions O1 and O6. maintain covers optimize, index builds, the full-text rebuild and the repair report, and is judged on the branch the operation maintains (main for optimize and repair), so it takes branch_scope like change. reclaim covers cleanup, which spans every branch's history, so it is graph scoped and refuses branch scopes. Admin stays reserved. The engine's maintenance surface is now only _as forms, each enforcing its action before any effect: optimize_as, ensure_indices_as, ensure_indices_on_as, rebuild_full_text_indices_on_as and the read-only repair_report_as check maintain, and cleanup_as checks reclaim. The actor-less forms are removed rather than made crate-private, since nothing inside the crate needs them; optimize and the index writer now record the actor on the commit they publish. For O5 the full-text rebuild admits maintain or change through a new provided method, PolicyChecker::check_any, behind Omnigraph::enforce_any, of which enforce is now the one-action case: the first allowed action admits, an evaluation failure refuses at once rather than falling through to the next action, and a refusal names every action. Repair is report-only: RepairOptions, the healed and forced actions, RepairStats::graph_manifest_version and the publish with omnigraph:repair and omnigraph:repair:force are gone. No internal path used the publishing mode, so nothing is kept crate-private for it; the coordinator's PublishPrecondition::Any publisher it called now compiles only for tests and failpoint fixtures. The guard is forbidden_apis.rs::maintenance_entry_points_take_an_actor: every surface registered under the index, optimize and cleanup protocols, plus the repair report, must be an _as method with an actor: Option<&str> parameter, and the set is pinned, so a new actor-less maintenance method fails the registry. The CLI, DST, GQT and bench callers move to the _as forms; the direct CLI passes resolve_actor, as the full-text rebuild and direct writes already do.

policy_engine_chassis.rs gains maintenance_entry_points_enforce_maintain_on_their_branch_before_effects, which refuses every maintenance entry point to an actor holding only change, reclaim or maintain on unprotected branches, and to no actor, with the storage tree byte-for-byte unchanged, then admits each under maintain and checks that optimize's commit carries the actor; cleanup_as_enforces_reclaim_before_effects does the same for cleanup; and the full-text test now admits the rebuild under either grant, requires the refusal to name both, and checks each commit's actor. The policy crate's tests round-trip every action through its name and serde, compile a rule for every action against the Cedar schema, pin the scope rules of the two new actions, and cover check_any, including that an evaluation failure on the first action is not passed over. The ignored changes_cost.rs test that built its multi-version interval with a forced repair is deleted, since no builder for that fixture remains, and the repair confirm and force tests in integration/maintenance.rs fold into the two report tests they duplicated.

For review: the decision to challenge is the scope of maintain. Judging it per branch lets a policy keep automation off protected main, and lets the O5 overlap compare change and maintain on the same scope; a graph-wide maintain would be simpler for the future routes but could not express that. The access report (GET /graphs/{graph_id}/access, the MCP access tool and omnigraph access) does not name the new actions, because no served operation performs maintenance and a new value in that enum is a wire change under the 0.14 contract; the routes step should add them. cleanup_plan, the read-only preview, keeps no actor here; the RFC's reclaim on the preview lands with its route. The CLI still refuses --as on optimize, repair and cleanup, so they act as operator.actor; widening --as belongs to the CLI step. repair --json drops its always-null graph_manifest_version. This is a source-breaking change for Rust embedders, with a breaking note. The engine was not built locally: the machine's free disk fell below the 20 GB floor during the first engine build, so the engine, CLI and workspace results are CI's. On the first CI run csv_source::tests::cancelled_csv_producer_settles_and_releases_its_admission failed its pause.is_paused() assertion; it touches nothing here and passed on the re-run, so it looks like a race in that test.

Add the `maintain` and `reclaim` Cedar actions. `maintain` covers
optimize, index builds, the full-text rebuild and the repair report and
is judged on the branch maintained; `reclaim` covers cleanup and takes
no branch scope.

Maintenance has no actor-less public engine entry point any more:
`optimize_as`, `cleanup_as`, `ensure_indices_as`, `ensure_indices_on_as`,
`rebuild_full_text_indices_on_as` and the read-only `repair_report_as`
each enforce their action before any effect. The full-text rebuild
admits `maintain` or `change` through `PolicyChecker::check_any`.
Repair loses its publishing mode, its options and its hard-coded actor
strings. The CLI, DST, GQT and bench callers move to the `_as` forms;
the direct CLI passes the actor it resolves for direct writes.
@ragnorc

ragnorc commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

This draft was built on Phase A of RFC #1168, which was closed on 2026-10-27 in favor of focused RFCs that start with a bounded, manually requested remote optimize through the serving owner. It stays a draft. The replacement RFC will state whether this change is reused as is, trimmed or not needed, and the PR will be re-justified against that RFC before review.

ragnorc added a commit to ragnorc/omnigraph that referenced this pull request Oct 28, 2026
Record the intent nonce and resolve by commit_id_answers; keep an
in-doubt publication non-terminal; release the writer gates before the
artifact scan; add a resume position, resource ceilings and per-table
failure isolation; require maintain on every route; depend on ModernRelay#1175
and ModernRelay#1176.
…cy-actions

The index build from main checked `change`; it now checks `maintain` like
the other index writers, and `ensure_indices_on_as` returns main's
`IndexBuildResult`. The actor-less `ensure_indices` and `ensure_indices_on`
stay removed. Main's unreleased index-build note is updated to match, and
the maintenance page and notes are brought within their caps.
ragnorc added a commit to ragnorc/omnigraph that referenced this pull request Oct 29, 2026
* rfc: remote optimize through the serving owner

A bounded, manually requested optimize of a served graph's main branch,
executed by the process that serves and publishes it: today's gate shape
and one publication, a gate-hold bound with an exact outcome per table, a
durable operation record under a caller-chosen id before any effect,
lookup by that id after a lost response, and restart resolution from the
graph's history. Replaces the closed served-maintenance proposal in the
form its review asked for.

* rfc: link the remote optimize RFC to its discussion

* rfc: revise remote optimize after review

Record the intent nonce and resolve by commit_id_answers; keep an
in-doubt publication non-terminal; release the writer gates before the
artifact scan; add a resume position, resource ceilings and per-table
failure isolation; require maintain on every route; depend on ModernRelay#1175
and ModernRelay#1176.

* rfc: settle remote optimize resume, ceilings and resolution after second review

* rfc: settle remote optimize resolution reads and the artifact bound after third review

* rfc: resolve through the buffered commit lookup and count both offset sides after fourth review

* rfc: state the history cost of a released commit and reuse the collector's path resolution after fifth review

@ragnorc ragnorc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: not ready to merge. Resolve the merge conflict first. The design is sound and I found no code defect.

Why

No maintenance entry point checks policy today. optimize, cleanup, ensure_indices and repair take no actor. An embedded program with a policy checker can compact, rebuild indexes and delete history around the gate that invariant 10 requires. Served maintenance (RFC #1214) needs a grant to check before it can have a route.

What

  • Two Cedar actions join the vocabulary. maintain is branch scoped. reclaim is graph scoped and refuses branch scopes.
  • Every maintenance method is now an _as form that enforces its action before any effect. The actor-less forms are removed.
  • Optimize and the index writer record the actor on the commit they publish.
  • The full-text rebuild admits maintain or change through PolicyChecker::check_any. An evaluation failure refuses at once and does not fall through to the next action.
  • repair becomes the read-only repair_report_as. The publishing mode (confirm, force) is gone.
  • forbidden_apis.rs::maintenance_entry_points_take_an_actor pins the set of maintenance surfaces.

Blocking

  1. The branch conflicts with main in crates/omnigraph/src/db/omnigraph/repair.rs (checked with git merge-tree against dd7a9f75). Main changed that file in #1228. The merge has to keep the report-only shape and the maintain gate. Please rebase or merge and run CI again.
  2. The PR is a draft and its own comment says it needs re-justifying against the replacement RFC. #1214 now depends on this PR for maintain and optimize_as, and says reclaim, cleanup_as and the repair report belong with cleanup and repair. That settles the question for maintain and optimize_as. State in the PR whether the reclaim and repair parts stay here or move out, and take it out of draft when you decide.

Optional

  1. cleanup_plan and its helpers stay actor-less and public. They are read-only, but they list what reclaim would delete. Gate them with reclaim (or maintain) when the cleanup route lands, as the PR text says. Add a test then, so the surface is not forgotten.
  2. "Accepts change for one more release" has no mechanism. The note says the full-text rebuild stops accepting change after one release. Nothing fails at that point. Add a tracking issue, or a test that goes red at the next version bump. Otherwise check_any and its two-action call stay for good. They are the only multi-action gate.
  3. The PR joins two changes. One is the policy gate. The other removes repair's publishing mode. The removal is correct: invariant 5 says foreign drift is reported and never adopted, and no internal caller used the mode. It also removes a mutating door instead of gating it, and it deletes state. It does enlarge the breaking API change for embedders. If review slows down, this part splits out cleanly.
  4. The access report does not list the new actions. The PR defers this to the routes step, because a new enum value is a wire change under the 0.14 contract. That is right. Make the routes PR carry it, so the report cannot disagree with the policy language.
  5. Liability check. Five more maintenance operations would each add one _as method and one registry row, and the pinned-set test would fail on a method without an actor. That is a good place for the invariant to live. The cost is two actions plus check_any. Both are small and have one owner each.

Validation

  • Exact-head CI at de602a32: all required checks pass, including Test Workspace, Clippy, rustfmt, DST pinned suite and the four GQT contexts. The PR text reports one flaky csv_source failure on a rerun. It is unrelated.
  • Local run at de602a32 (cargo, reused build outputs):
    • cargo test -p omnigraph-policy --locked --lib: 24 passed.
    • cargo test -p omnigraph-engine --features failpoints --test policy_engine_chassis maintenance: 1 passed (maintenance_entry_points_enforce_maintain_on_their_branch_before_effects_issue_840).
    • cargo test -p omnigraph-engine --test forbidden_apis maintenance_entry_points: 1 passed.
  • Code inspection: every enforce call comes before the schema-state check and the export reservation, so a refused call writes nothing. The chassis test asserts the physical tree is unchanged after each refusal.
  • No before/after regression proof. This is a feature, not a bug fix, and the embedded policy API is not visible in GQT rows. The chassis and policy-crate tests are the cheapest owners. They are new tests, so they have no earlier head to fail against.
  • Not run locally: the full workspace, the CLI tests and the S3 and Azure suites. I rely on CI for those.
  • Not verified: the server and MCP behavior after the conflict is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant