Repository navigation
feat(server)!: assert the expected actor with Omnigraph-If-Actor - #1074
Conversation
A request carrying omnigraph-expect-actor is refused with 412 actor_mismatch, after authentication and the contract check and before graph lookup, body work or effects, when its credential resolves to another actor. A match changes nothing, so the header only narrows a request and never supplies an actor. Every protected router and /mcp check it. The CLI sends it for --as on served writes and cluster --server commands, which no longer refuse --as. A mismatch is a not-started refusal and exits 4, like a failed graph-commit precondition.
WhyA served request must act as the actor its credential resolves to. WhatThis pull request adds the request header How
ImpactThe bearer credential stays the only actor. The header can only narrow the request. That matches invariant 10 and RFC 1017 rule 5, on a server that implements the check. A server that still speaks contract Blocking
Optional
Tradeoff and liabilityOne middleware and one header is the right shape. The header does not add a second identity, a queue, or a shadow actor store. Five more checks of this kind belong in the same middleware, on one contract version. The pull request splits RFC 1017 phase 1 so each review stays small. That choice weakens the upgrade fence. The RFC puts this header, the access route, RFC 1017 at
|
…es it A server built before `omnigraph-expect-actor` speaks the same HTTP contract and ignores the header, so `--as` on a served write would run as the credential's actor. Every response now lists the assertions its server enforces in `omnigraph-http-api-assertions`, and the CLI refuses before sending when discovery does not list `expect-actor`. A served `cluster plan` or `cluster apply` refused for another actor now exits 4 as a data write does: its requests are observed, and a refused assertion with nothing effectful sent before it is a condition, with or without `--json`.
The cluster skill page still said `cluster apply --server` refuses `--as`; it now names the assertion `cluster plan` and `cluster apply` send and its exit 4. The release note moves to `breaking`, since a matching `--as` now runs a write the CLI used to refuse. The server guide says `/mcp` has no contract check, an empty assertion joins the malformed-header test, and stale comments name the current behaviour.
RecommendationRequest changes. Two blocking findings remain. The cluster skill still documents the refusal this pull request removed. Reviewed head: WhyA served write must run as the actor its credential resolves to. What this head changes
Earlier findings
Optional items from that review:
Blocking1. Health advertisement is not the contract fence
The comment on I ran Draft RFC #1017 at An honest server that omits the header is closed. The head CLI does not send the write. See Validation. The supported single-process topology is closed for that server. The open case is a backend that lists the assertion on discovery and ignores the header on the write.
Please bump If the header remains as advertisement, do not freeze a one-value enum on every OpenAPI response. 2. The embedded cluster skill still says the server refuses
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…efusals as the server does The merge keeps this branch's command kinds and ModernRelay#1074's served cluster `--as` exit: a cluster command is observed without being classified. An embedded `load --from` marked its write compound whenever `from` was set, so a refusal before its fork exited 2. It now marks it only when the branch is missing, as the server's load does, and an embedded write's own error goes through `engine_error_output_after`: a policy refusal precedes every publication, the fork's included, and any other refusal of a compound load follows the fork. A read's engine error keeps its own code instead of `outcome_unknown`, and the failure reporter no longer panics on a wrapped engine write error. The crossversion journeys read a `--json` read's refusal from stdout, where the failure now goes. The exit documentation says once that a failed condition exits 4 embedded or served, names the data commands the outcome table covers and the commands that keep their own exits, and says that only the argument parser's refusals exit 64.
|
Addressed at 8a4288a (fixes 2fff68e and 8a4288a). Blocking
Optional: all four done. The note is now |
RecommendationRequest changes. One blocking finding is still open. Reviewed head: WhyA served request must run as the actor its credential resolves to. What this head changes
How
ImpactA matching token now runs a write that the CLI on Earlier findingsFrom the review at
Optional items from that review:
The first review's three blocking items are the same three. Item 1 is still open. Item 2 is resolved. Item 3 was resolved at Blocking1. The health advertisement is not the contract fence
The comment on
Draft RFC #1017 at Bump Optional
Where the code and draft RFC #1017 disagreeRFC head:
Siblings
Tradeoff and liabilityOne middleware and one request header is the right enforcement shape. The header only narrows. A match continues with the token's actor. A mismatch returns 412 before graph lookup. The client does not supply a trusted actor. That matches invariant 10. The change adds no second identity, no queue, and no shadow actor store. The response header is a second compatibility record beside the contract version. The RFC already chose one version bump so an old server refuses the data request. The header lets this slice land before the access route and The docs commit removes the stale refusal text. An operator who follows the cluster skill no longer expects a local refusal. That text change does not close a backend that advertises the assertion and then ignores it. ValidationGQT cannot express an HTTP header or a CLI exit code. This delta does not change query rows, counts, column types, or query errors. I did not add a Local tests at this head, each with
GitHub Actions run 39179059242 on this head: Lint, Format, and the GQT jobs passed. Test Workspace failed. The only failed test is I did not repeat the lying-server harness. LimitationsI did not boot a real server at Inline comments that could not be attached
|
A server that predates `omnigraph-expect-actor` ignores it, so `--as` on a served write would run as the credential's actor. The earlier fence, a response header listing the assertions a server enforces, was a second compatibility record beside the contract version: two servers speaking 0.13 could disagree about the assertion, and every further assertion would extend an enum on every response. The contract moves to 0.14 instead, the one bump the access route, `outcome` and the code registry also ride. A 0.13 server answers discovery with its own contract, so the CLI refuses it before sending, and it refuses a 0.14 request before reading the header. The assertions header, its constant and the CLI's discovery check are gone, and so is the enum on every OpenAPI response. The guides, the developer pages, the skill, the version samples and the fixtures name 0.14, and a breaking note says to upgrade the CLI, server, Studio and HTTP integrations together. The server guide says a repeated, padded or empty header is refused, and that `/mcp` compares it after authentication; the cluster skill names the subcommands that take `--as`.
|
Addressed at 76cc941. #1073 now merges this branch (be18e46), so the access route ships under the same bump, and its policy example sends 0.14. Blocking
Optional
The bump also updates every Validation: |
The access route from ModernRelay#1073 now rides HTTP contract 0.14 with the actor header: its policy guide example sends 0.14, and its OpenAPI operation lists the contract value and the optional omnigraph-expect-actor parameter like every other protected route.
RecommendationApprove. There is no blocking finding. Reviewed head: WhyA served request must run as the actor its credential resolves to.
What this delta changesTwo commits landed after
How the merge resolvedThe merge parents are Earlier optional findings
The whole pull request, after the merge
BlockingNone. Optional
Where the code and the drafts disagreeRFC #1017 head: The code and that RFC disagree on the header name. The RFC says These parts of RFC #1017 match the code:
Draft RFC #1089 at OpenAPI writes every header name in lowercase, including this one. RFC #1089's Title-Case check is a later lint over the whole spec. This head keeps one spelling with the other headers. Siblings
Tradeoff and liabilityOne constant and one header are the right shape. The header only narrows the request. A match continues with the token's actor. A mismatch returns 412 before graph lookup. The client does not supply a trusted actor. The change adds no second identity, no queue, and no shadow actor store. This delta does not add a primitive. The deployment sentence now matches The remaining cost is the draft name. An accepted RFC #1017 that still says ValidationLocal tests ran at
The actor test covers a match (200), a mismatch (412, manifest version unchanged), a repeated, padded, or empty value (400), and a server with no authentication (412). GitHub Actions run 39331978861 on this head is green. That includes Test Workspace, Lint, Format, CI Gate, and the GQT jobs. That run is separate from the local tests. Regression proofThis pull request is a feature. The delta does not fix a bug. The regression-proof rule does not apply. The contract is an HTTP header and a CLI exit code. A LimitationsI did not run the full |
RecommendationRequest changes. One blocking finding is open. Reviewed head: WhyA served request must run as the actor its credential resolves to.
What this delta changesOne commit landed after
That merge brings a branch start at a published graph commit, and a cleanup fix for an interrupted local sweep. How the merge resolved
The auto-merge keeps both sides in the Rust files. The auto-merge of Actor check on the routes this merge adds
CLI The cleanup change is inside the engine collector. It adds no HTTP route. It does not read an actor. A policy denial of a commit start is still Earlier optional findings
Blocking1. Two
|
The main merge kept two createBranch responses, 404 and 409, from a tree that still spoke contract 0.13. The generated spec names 0.14 on every response, as the server sends it.
|
Addressed at 0432c9d. Blocking
Optional
|
Brings in ModernRelay#1074's Omnigraph-If-Actor header through ModernRelay#1076, and ModernRelay#1076's final error-code registry.
RecommendationApprove. There is no blocking finding. Reviewed head: WhyA served request must run as the actor its credential resolves to.
What this commit changes
How
ImpactA client that trusts the checked-in spec now reads contract Earlier findingsFrom the review at
Sibling branches and draft RFCs#1076 at Draft RFC #1089 at Merge with main, and the #1091 routes
The merged per-graph router registers Lance 13 Tradeoff and liabilityOne middleware and one header remain the right shape. The header does not add a second identity. This commit does not add a queue, a shadow actor store, or a Lance actor. The fix puts the checked-in spec back on the one constant. The generator stays the source of truth. The freshness test stays the gate. The liability this removes is a client that reads ValidationIsolated checkout of
Command: GQT cannot express an OpenAPI response enum or this header. The existing owner is the freshness test. It fails on the parent for the predicted reason and passes on this head. No new case is required. Local merge with
LimitationsI did not run the full workspace suite, workspace Clippy, the CLI process tests, GQT, or S3/Azure. The merged tree compiled the server integration tests and the three Clippy packages above. The temporary branch-route requests ran only in the isolated merge checkout and are not in the pull request. |
Main replaced the legacy routing flags with explicit targets (ModernRelay#1090). The actor assertion sits on that structure: `GraphClient::from_target` and the served cluster client send `--as` as `Omnigraph-If-Actor` on a server target, where ModernRelay#1090 refused it, and only a managed target still refuses `--as`. `accepts_actor` keeps deciding which commands take it. The HTTP contract stays 0.14.
…ges within the cap The served cluster entry now takes the actor `--as` asserts. The server, CLI reference and cluster guides fold the merged text back under 350 lines.
RecommendationApprove. There is no blocking finding. Reviewed head: WhyA served request must run as the actor its credential resolves to.
What this delta changesThree commits landed after
How
Write commands pass that actor. Read commands pass
The server routers still run Lance 13 ImpactA served Earlier findingsFrom the review at
BlockingNone. Optional
Tradeoff and liabilityOne middleware and one header remain the right shape. The header does not add a second identity. This delta does not add a queue, a shadow actor store, or a Lance actor. The compile fix supplies the The page edit keeps the same sentences and brings three guides under the 350-line cap. It does not add a second documentation mechanism. This delta removes a cluster test that does not compile, and three user pages that fail the documentation check, after the target merge. It adds one obligation: a later command that accepts
ValidationIsolated checkout of
GQT cannot express this header, a CLI exit code, an OpenAPI enum, or a documentation line cap. The owners above already fail for the predicted reason before the fix and pass on this head. No new case is required. GitHub Actions on this head are green ( LimitationsThe full workspace suite, workspace Clippy, GQT, and S3/Azure were not run locally. The temporary test edit and the temporary guide swap were restored. Nothing was pushed. |
Brings in main through ModernRelay#1076: the ModernRelay#1074 squash, ModernRelay#1092 and ModernRelay#1103.
What & why
A request carrying
omnigraph-if-actoris refused with412 actor_mismatchwhen its credential resolves to another actor (or to none), after authentication and the contract check and before graph lookup, body work or effects. A match changes nothing, so the header only narrows a request and never supplies an actor. A duplicate, empty or padded value is a400. Every protected router and/mcpcheck it.The CLI sends it for
--ason a server target, for data commands and forcluster planandcluster apply, which no longer refuse--as; a mismatch exits 4, like a failed graph-commit precondition. A managed target still refuses--as.Backing issue / RFC
Checklist
auth_policy::expected_actor_header_narrows_a_request_and_never_supplies_an_actor,cli_data::as_on_a_served_write_asserts_the_actor, OpenAPI variantschangelog.d/if-actor-header.breaking.mdLocal verification
RUST_MIN_STACK=16777216 cargo test -p omnigraph-serverandcargo test -p omnigraph-cli(with the server binary built) — all passcargo clippyfor both crates with-D warnings,cargo fmt --all --check,python3 scripts/check-docs.py,typos— cleanNotes for reviewers
Breaking only for scripts that relied on served commands refusing
--as.openapi.jsondescribes the optional header and its 412 on every route.