Repository navigation
feat(server)!: say on every error body whether the request ran - #1076
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.
Every error body carries an outcome object (execution, effects, action) that the server computes for the request it answered: not_started with no effects when it refused before submitting a write, or refused the write's one publication with a typed reason, which an engine gives only before it publishes; unknown otherwise, including a typed refusal after a load forked its branch. The next step a caller takes is retry, refresh, recover or reconcile. A write whose completion is uncertain answers outcome_unknown instead of internal. Errors that carried a reason only in their details gain registry codes: precondition_failed, recovery_required, external_blob_source and change_feed_gap. CLI data-write failures print the same object as outcome, in place of command_outcome, and take the server's when its request was the command's only effectful one.
|
This commit adds one Reviewed commit: Request changes. One path reports BlockingMCP
That under-claims a publication. A temporary test at this head failed in that shape. Outer Carry Optional
RFC #1017Compared with The implementation matches these parts: one These parts differ:
Managed Siblings and orderingThe review surface is the last commit: 22 files, +623/−143. #1074 is the parent and composes cleanly.
Liability
The HTTP single-publication path is the right shape. One decision function. One shared ValidationHead
Temporary regression, then the tree was restored clean at this SHA. A spawned compound mark rendered on the outer request task returned GQT cannot express an HTTP or MCP error body, a spawned task, or a deployment route. There is no row, count, or column change to pin with a I did not run the full CLI integration binaries, the GQT corpus, or a live MCP load against a graph. Lance is not on this path. Inline comments
A typed refusal after
Put
This sentence, and the changelog note, say an unsettled write is the
Draft RFC #1017 at
This runs for every Mark compound from the receipt's |
…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.
Write facts survive task boundaries. Whether a request had submitted an owned write, and whether its load forked a branch, lived in a task-local, so an MCP tool call, which runs its handler in an owned read task and builds the error body in another, lost both: a load whose `from` forked a new branch and then met a key conflict reported not_started with no effects. ApiError now carries `compound` beside `owned_write`, an owned read keeps the request's record in its own task, and an error leaving it takes that record along; the outcome reads the error's own fields. The outcome unit test gains cases run through an owned read, the MCP write test a load that forks and then conflicts, and the HTTP load test its twin. A load is compound only when it forks. Both load routes marked it whenever `from` was set, so a refused load onto an existing branch reported unknown although its one publication never ran. The routes now mark it from the branch listing their authorization already reads, and only on the owned write's error. An earlier step's publication also outranks a refusal the write met before it started. The HTTP load test covers a key conflict onto an existing branch with `from`, which reports not_started. Every error the server answers has a code. ApiError's code is no longer optional, and the deployment status read that races an owner change answers 503 `service_unavailable` with a retry outcome instead of no code. Malformed input, unknown routes and unserved methods answer the error body. Handlers took axum's Json, Query and Path, whose rejections are plain text (422 for a JSON body of the wrong shape), and unrouted requests had empty bodies. ApiJson, ApiQuery and ApiPath refuse with ApiError (400, or the transport status for 413 and 415), the router's fallbacks answer not_found and method_not_allowed, and a source guard refuses a handler parameter of axum's own extractors. The three query structs whose parameter location came from the extractor's name declare it, so their optional parameters render as the other query parameters do. Embedded callers can render an engine failure as a served request with the same facts would: engine_error_output_after takes whether a write was submitted and whether an earlier step published, and engine_read_error_output renders a read, which never answers outcome_unknown. The RustFS S3 test expected the 424 external Blob source refusal to carry no code; it now asserts its code and outcome. The troubleshooting guide and the changelog note state the outcome decision table the server implements.
|
Addressed at e5d4747 (fix 5e46f92, plus the merge of #1074's fixes). Blocking: MCP compound loads.
Optional
Also:
|
The stack's other changes to this page need its last line.
What this change doesA caller that gets an error must know whether the request ran. This commit puts one The fix commit Earlier findingsReviewed commit
BlockingA load can say
|
|
The new commit only shortens the errors paragraph. The Rust sources are the same as A caller uses Prior findings at
|
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`.
Brings in HTTP contract 0.14 and the removal of the assertions response header, so this branch speaks the contract its outcome field rides.
…failed A load names whether it forked its branch only from inside the engine: the routes took that fact from the branch listing their authorization reads, before the load lists the branches again and decides. A branch deleted between the two, as one can be while an NDJSON body is still arriving, was forked by the load, and a refusal of its rows then answered `not_started` with no effects although the fork had published. `CompletionEvidence::PartlyPublished` records that an earlier step of the owning operation published and a later one failed. The load marks every failure after its fork with it, the window between the fork and the rows included. It outranks the definite `BeforeEffect` and `Unpublished`, which no longer cover the operation, survives composition, and never hides `Uncertain`. The server reads it as the compound fact and leaves containment where no evidence leaves it, so a typed refusal keeps its code with `unknown` and `refresh`. The routes' list-derived flag and `in_compound_write` are gone, and `engine_error_output_after` drops its `compound` argument: embedded callers get the fact from the error. A route test deletes the branch while the NDJSON body waits and expects the fork reported; the core test pins the composition rules. The key conflict and `graph_unavailable` descriptions say what they now mean, and the breaking note names the plain query-parameter types in openapi.json.
|
Addressed at 1abd5bb (fix), after 722a104 (the merge of #1074's contract bump). Blocking: the fork is now the engine's fact.
Regression:
It expects Optional
RFC differences: rule 6 in #1017 now carries the server's outcome table, with the typed code kept for a refusal after an earlier publication and for a settled untyped error after a write. The contract bump is #1074's, so this branch now speaks |
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.
Brings in main through ModernRelay#1074: the access route from ModernRelay#1073 under HTTP contract 0.14.
Brings in main through ModernRelay#1076: the access route from ModernRelay#1073 under HTTP contract 0.14.
The key-conflict description said the refused rows wrote nothing; the OpenAPI vocabulary guard keeps graph vocabulary in the published description, so it now says the refused write changed nothing.
The actor assertion header is `Omnigraph-If-Actor` (`IF_ACTOR_HEADER`). The error-code registry keeps its shape, with `actor_mismatch` naming the new header; the published spec is regenerated, and no spelling of the old header remains outside released history.
A served 401, an unsupported contract and an `actor_mismatch` ask the caller to fix the credential or client; a policy denial still asks for refresh.
Brings in ModernRelay#1074's Omnigraph-If-Actor header through ModernRelay#1076, and ModernRelay#1076's final error-code registry.
|
This review covers WhyA caller that receives an error has to know whether the request ran, and what to do next. A wrong answer makes that caller miss a published branch, or replay a write that already landed. WhatEvery server error body carries How
ImpactCallers can trust Earlier findingThe optional finding at A settled unpublished step, after an earlier publication, was answered as Blocking
CI on this head builds that merge. Run 39378123022 fails in Test Workspace, Lint, the aws build, RustFS, Azurite, the format fence, and storage-upgrade compatibility. Run 39378123070 fails GQT served the same way. The error is
Optional
One status list, and two statuses for one codeThese agree. The rename
The #1016 noteResolved. Deployment and MCP
Siblings and RFCs#1074 at #1078 at #1081 at #1096 edits #1075, #1077, #1090, #1059, and #1012 do not define this error body. #1059 edits CLI Draft #1017 and #1089 already say Draft #1093 still describes today's MCP body as LiabilityOne disposition function and one The status list on each code is the registry. TradeoffCorrectness comes first. The extra evidence bit is the simple record of a fact the engine already knows. A coarser ValidationReviewed head The head itself compiles. The merge with Settled step. GQT does not show an HTTP body. The owner is the server unit test. Before, parent
Result: failed. After, on this head, the same shape is in One code under its own status. Before,
Result: failed. left After, the same test on this head passed. Deployment and MCP bodies. Before,
Result: failed. Status 500. Body:
Result: failed. Body was After, on this head:
Result: 5 passed. An earlier attempt failed to compile because the shared target still had an older
Result: 1 passed.
Result: 1 passed. At
Result: exit 0 on this head. CI clippy builds the merge with
Result: Documentation OK, 349 Markdown files.
Result: exit 0, tree LimitationsI did not compile the merge tree locally. CI did. I did not run the workspace test suite or the GQT corpus locally. Served GQT in CI failed at the same RecommendationDo not merge yet. Rebase onto |
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.
It brings main's explicit targets (ModernRelay#1090) and Lance 13. A target refusal that ModernRelay#1090 reports with the command's evidence carries `disposition`, as every other data-write failure does.
…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.
Brings the actor branch's cluster remote test fix and its page trims.
Main's branch action route took axum's `Path` and answered 405 with no body. It takes the typed extractor, and its 405 carries `method_not_allowed` and the disposition beside the `Allow` header.
Brings in main through ModernRelay#1076, including ModernRelay#1090's explicit targets. The exit table keeps its classification: the removed use and profile commands leave it, and the target and credential commands, which keep their own failure reports, join the unclassified ones. ModernRelay#1078's tests move to --target, ModernRelay#1090's usage-error tests expect exit 64, and the diagnostics checks pass the destination line ModernRelay#1090 prints first.
…s guards The router-wide method fallback replaced each protected route's default fallback, layers included, so a method the route does not serve answered 405 before bearer authentication, the contract check, the actor assertion and admission. Each protected router now installs the fallback before its guards, through one helper that owns their order.
|
This review covers WhyA caller that receives an error has to know whether the request ran. A 405 that skips the guards also tells an anonymous caller which methods a protected path serves. WhatTwo fixes land after How
ImpactA caller the guards admit can trust Earlier findings
BlockingNone. OptionalThe three notes above. No new one. Liability
The branch path keeps its own 405 so TradeoffCorrectness comes first. The caller learns the methods after the guards admit the request. On the catalogue and deployment routers a wrong method also passes admission, as a method they serve does. That is the same rule, and the cost is one admission slot for a request that does no graph work.
ValidationHead
Body regression at
Result: 1 failed. Status was 405 and Guard regression at
Result: 1 failed. After, on this head, after
Result: 3 passed. CI run 39489959127 on this SHA: Test Workspace, Lint, and GQT ordinary, served, and dst passed. CI Gate failed because Dependency Guard failed. RecommendationApprove. The two blocking findings are resolved. The guard order is the right layer for the leak. The three optional notes are unchanged. |
The failure logs main added read ApiError's status, which this branch derives from its code.
…lication unpublished() returned an error unchanged once it carried any evidence, so a step marked after its operation recorded an earlier publication lost its own proof. It now sets step_unpublished on that evidence.
The 412 row now says what to do about actor_mismatch, the changelog names branch_start_refusal's two statuses, and the CLI reference says a served blob put, blob clear, commit publish and commit drop send --as as the actor assertion.
Brings in main through ModernRelay#1076: the ModernRelay#1074 squash, ModernRelay#1092 and ModernRelay#1103.
…eady contained
Stacked on #1074 (
Omnigraph-If-Actor).What & why
Every error body says what the request did and what to do next, computed by the server for the request it answered, in a
dispositionobject:effects:nonewhen nothing took effect (a refusal before a write, a typed refusal of its one publication, or a failure the engine proved published nothing);partialwhen an earlier step of the same operation published and the failing step wrote nothing, such as a load that forked its branch;unknownwhen the server cannot say.action:fix(correct the credential, input, path or client),refresh(read the state that refused it),retry(busy or unavailable, withRetry-After),recoverorreconcile.A write whose completion the server could not settle answers
500 outcome_unknown. Every code is in one registry with the statuses it is sent under, whichopenapi.jsonpublishes; codes that were told apart only by their details gain their own (precondition_failed,recovery_required,external_blob_source,change_feed_gap,branch_start_refusal), and 408, 413, 415 and 416 stop answeringbad_request. Deployment routes and MCP tool errors answer the same body. CLI data-write failures print the samedispositionin place ofcommand_outcome.Backing issue / RFC
Checklist
api_error_tests(each refusal class, settled step after a fork, status andRetry-Afterper code, branch start refusal),every_error_code_documents_its_status, core completion-evidence tests,boot_settings::deployment_routes_refuse_with_the_error_body, MCP error payloads, OpenAPI schema, CLIdispositionacrosscli_data,cli_queries,parity_matrix,system_remotechangelog.d/error-outcome.breaking.md,error-code-statuses.breaking.md,mcp-error-body.breaking.mdLocal verification
RUST_MIN_STACK=16777216 cargo test -p omnigraph-server -p omnigraph-cli -- --test-threads=6(server binary built first,openapi.jsonregenerated) — all pass-D warnings,cargo fmt --all --check,python3 scripts/check-docs.py,typos— cleanNotes for reviewers
Breaking for clients that read
command_outcomeoroutcome, told errors apart by a missingcode,conflictorbad_request, or read MCP's{code, message}andoutcome_unknown: true.