Repository navigation
fix(cli): report a served write refused at auth, policy or contract as not started - #981
Conversation
…s not started The server answers 401 `unauthorized`, 403 `forbidden` and 400 `api_contract_mismatch` before it admits a write: bearer authentication, the contract header check and every policy gate (HTTP and engine) run ahead of admission and staging. The CLI still reported these refusals as `execution: unknown, effects: unknown, action: reconcile`, sending callers to reconcile writes that never began. Classify a dispatched write answered with one of these codes under its own status, in a response that carries the contract header, as `not_started` / `none` / `refresh`. Like the 429 retry rule it requires the refusal to be the command's only effectful request with no writable open, and a body with no contradicting detail. Exit code stays 1. Fixes ModernRelay#976.
Review of #981 at a0aedd5Recommendation: approve. I found no blocking issues. I post this review as a comment because this account cannot submit a formal approval. The three inline comments are optional or recommended follow-ups. Why, what, how, impact
Blocking findingsNone. Optional and recommended findings (inline)
Tradeoffs and software liability
Code references checked
ValidationClean isolated checkout at head
I restored the worktree after each experiment. Limitations.
|
Fold the 429 retry rule and the auth, contract and policy refusal rule into `admission_refusal`, one map from a code under its own status to the action, with the shared guards applied once. The refusal rule now also requires a non-empty error, as the retry rule did. State the server and engine promise the classification relies on in docs/dev/writes.md: 401, contract 400 and 403 refuse a served write only before its first durable effect. Add blob put and blob clear to the outcome matrix.
Fixes #976.
Problem
Failure::classifyreported every failure of a dispatched served write asexecution: unknown, effects: unknown, action: reconcile, except a typed 429 and a verified 412. A server that refused the write at authentication (401 unauthorized), by policy (403 forbidden) or at the HTTP contract check (400 api_contract_mismatch) therefore told callers to reconcile a write that never started.Change
A dispatched write answered with one of those three codes, under its own status, in a response that carries the server's API contract header, now reports
execution: not_started, effects: none, action: refresh. The exit code stays 1.admission_refusalmaps each pre-admission refusal (the 429 retry and these three) from its status and code to its action, and the guards apply once:single), so a compound command keeps its earlier effects;is_plain_refusal); a 403 that also reportsrecovery_requiredstaysrecover;unknown/reconcile.Server audit: these codes are produced only before effects
401 unauthorized:require_bearer_authis the outermost per-graph route layer and runs before the contract check, graph resolution and the handler. The other producers (authorizewith a policy but no actor, graph discovery) are also pre-admission.400 api_contract_mismatch: produced only byhttp_contract::require_contract, which runs after authentication and before graph resolution and body work.403 forbidden, HTTP layer: every write handler callsauthorize_requestbeforeworkload.try_admitandowned_write(run_mutate,authorize_load_scopein both load routes,branch_create_body,branch_delete_body,admit_branch_merge,authorize_blob_write). The one policy check that follows an effect, the source deletion afterbranch merge --delete-branch, converts its denial intobranch_delete_error_detailson a 200 response, never a 403. Commit publish/drop and unattached-child denials answer 404.403 forbidden, engine layer:OmniError::Policy(the only source of an engine 403 inApiError::from_omni) is produced only byOmnigraph::enforceand the offline storage upgrade. Everyenforcecall is the first step of its_asentry point (resolve_write_routefor mutate,load_input_as,branch_create_as,branch_create_from_as,branch_delete_as,branch_merge_as_with_options,write_blob_cell_as, schema apply, table ops). The one nested call, a load's implicitbranch_create_from_asfor--from, checksbranch_createbefore the fork; nothing after the fork callsenforce.operations.rsalready pins a policy error as not completion-uncertain.schema applynever runs against a server, andcluster applyis outside the data-write outcome contract.Tests
cli_data::data_write_outcomes_and_retry_permission_issue_466_issue_976(renamed owner of the outcome matrix): each row now states its full expected outcome. New rows run the eight served write forms (mutate stored, inline andbranch createstatement,branch create,branch delete,branch merge,loadappend,load --branch --from), plusblob putandblob clear, against 401unauthorized, 403forbiddenand 400api_contract_mismatch(not_started/none/refresh, exit 1), plus three controls: a 403 without a code,forbiddenunder 400, and a 403 that also reports recovery work.cli_data::remote_json_errors_preserve_server_codes_and_details: the 403forbiddenmutate row now expectsnot_started.system_remote::remote_policy_enforces_branch_first_cli_workflow(real server and policy): the denied mutate and merge, plus a new denied load, denied branch delete and unknown-token mutate, reportnot_started/none/refreshwith their status, and main's snapshot is unchanged across the denials.client::tests::command_outcome_keeps_earlier_effects_and_scopes_independent_invocations(in-source): a 403 alone isnot_started; a 403 after an earlier successful write keepseffects: unknown.After merging main (#967, #990 and the rest up to b6ad205): the CLI
integrationbinary passes (309 passed, 0 failed, 10 ignored), and the CLI's in-source tests pass (160 passed). Clippy onomnigraph-cliandomnigraph-server(--all-targets -D warnings) was clean on a0b3ee8, before the merge; CI'sLint (clippy)covers the merged tree. The new authorization sites from main (diff reads,load csv) also check before admission.Earlier local runs (
RUST_MIN_STACK=16777216):cli_datatests failed on every 401, 403 and contract-400 cell reached (left: unknown/unknown/reconcile, right: not_started/none/refresh); no control row failed.integrationbinary passes (294 passed, 0 failed, 8 ignored); the CLI's in-source tests pass (143 passed, 6 ignored).cargo fmt --all --check,scripts/check-docs.py,scripts/check-agents-md.shand the release-note preview pass.Docs
docs/user/operations/troubleshooting.md, Failed data-write commands: the three refusals and the conditions under which they count.docs/dev/writes.md, Policy and attribution: the server and engine promise this relies on (these refusals come only before a write's first durable effect), with a checklist item for new writers and a pointer at the server's Policy→403 mapping.changelog.d/served-write-refusals-are-not-started.changed.md.Related: #466.