ddl,http,control: classify routed failures and gate non-default DBs

[EnRaiha]

Problem

Two surfaces still flattened a classified failure while the routed pgwire paths already carried its class:

• CRDT MERGE wraps its admission/apply failures under XX000, so an RLS write-policy denial — ExternalCrdtPostImagePolicy::deny returns RejectedAuthz — reached the client as an internal fault (ddl/neutral/dsl/crdt_merge.rs:90,101,166).
• The HTTP stream's in-band error lines carried no code at all, only a message, so a client consuming the stream could not classify a failure (http/routes/query_stream.rs:173,209).

Change

• CRDT MERGE maps through the existing error_to_sqlstate, which renders 42501 for a policy denial. No new class and no second mapping path. The "authorization returned no capability" site stays XX000: CRDT MERGE renders a policy denial as XX000 #344 records it as an internal invariant by decision.
• HTTP stream lines carry the numeric NodeDB code (shape and malformed-batch failures) and the status the gateway map already computed.

Consumer trace for the class change — the two wire assertions that pinned the placeholder move with it, in the same commit: nodedb/tests/wire/cases/crdt_write_rls_database_scope.rs:141,202 expected XX000 and now expect 42501, and the comment above the first one no longer describes the old wrapper. grep XX000 over tests/wire/cases finds no other crdt/merge site.

Follow-up: the same denial was also routed on the wrong key

Writing the wire test for that class change surfaced a second, larger defect underneath it.

In a non-default database, the admission gate routed on the caller's bare collection name while the planner derives the vShard from the database-qualified name. Admission and the planner therefore disagreed about which vShard a CRDT write belonged to: a policy-denied write was refused for the wrong reason, or admitted when it should not have been. Behind that, three planner catalog reads passed the same qualified name to a catalog keyed by the bare one, so the collection read as "not CRDT" and the CRDT engine was never seeded. That is why the client saw XX000 before the policy ever ran — and why fixing the class alone was not enough.

• crdt_admission keys on the canonical collection, so admission and the planner agree on the vShard.
• crdt_gate, update_delete::shared and implicit_edges::catalog reduce the routed name through the existing target_identity::bare_collection_name, as the sibling gate reads already did. It strips the prefix only when it is really there and is identity for DatabaseId::DEFAULT, so the default-database path is byte-for-byte unchanged.
• The three DdlError::new("XX000", e.to_string()) sites in neutral/crdt_ops.rs map through error_to_sqlstate too, so crdt_state and crdt_apply report the same 42501 the MERGE path now does. The ok_or_else(… "authorization returned no capability") at crdt_ops.rs:206 is deliberately left as XX000: it is a synthetic invariant breach with no underlying Error to classify.

Effect in every non-default database: a predicate UPDATE/DELETE on a CRDT collection and an INSERT … ON CONFLICT (id) DO UPDATE are refused again instead of silently taking the non-convergent path, and an edge-bearing collection gets its mark_collection_edge_bearing flag, so the mirrored-edge cleanup runs.

Changed files

4 commits on 1ff35512b — 13 files, +305 −32.

file	+/−
nodedb/src/control/crdt_admission.rs	+48 −5
nodedb/src/control/planner/implicit_edges/catalog.rs	+8 −2
nodedb/src/control/planner/sql_plan_convert/dml/balanced_gate.rs	+7 −1
nodedb/src/control/planner/sql_plan_convert/dml/crdt_gate.rs	+7 −1
nodedb/src/control/planner/sql_plan_convert/dml/insert/identity.rs	+6 −1
nodedb/src/control/planner/sql_plan_convert/dml/update_delete/shared.rs	+7 −1
nodedb/src/control/server/http/routes/query_stream.rs	+18 −4
nodedb/src/control/server/response_shape/schema.rs	+4 −4
nodedb/src/control/server/shared/ddl/neutral/crdt_ops.rs	+13 −3
nodedb/src/control/server/shared/ddl/neutral/dsl/crdt_merge.rs	+13 −3
nodedb/tests/wire/cases/crdt_write_rls_database_scope.rs	+76 −7
nodedb/tests/wire/cases/engine_surface_crdt_document.rs	+77 −0
nodedb/tests/wire/cases/engine_surface_graph.rs	+21 −0

Evidence

All five new tests are red on the unpatched tree and green with the fix.

step	run	exit	log	commit
red — the 5 new tests against 1ff35512b with the fix absent	nextest run --no-fail-fast -p nodedb --test wire -E '<the 5 tests>'	100	20260925T195039-unit2-new-tests-red-base-nofailfast-base.log	9c0cb47f
green — same filter with the fix	same, in the fix worktree	0	20260925T195131-unit2-new-tests-green-fix-exitcode-fix.log	9c0cb47f
preflight	cargo fmt --all -- --check + repo preflight vs origin/main	0	20260925T195215-unit2-fmt-and-preflight-exitcode.log	9c0cb47f
full wire suite	nextest run -p nodedb --test wire	0	232 run, 232 passed	final tree
-p nodedb --lib	nextest run -p nodedb --lib	0	394 run, 394 passed	final tree
in-proc suite	nextest run -p nodedb --test inproc	0	24 run, 24 passed	final tree
clippy	cargo clippy -p nodedb	0	clean	final tree

Red-arm integrity: the red worktree is detached at 1ff35512b with only the three test files applied — grep -c bare_collection_name over implicit_edges/catalog.rs and update_delete/shared.rs, and grep -c error_to_sqlstate over neutral/crdt_ops.rs, all return 0.

Review

Independent Review 2 (fresh context, read-only, separate agent): PASS, 0 blockers, on 9c0cb47f3516.

Its first pass failed on one real blocker and the fix is worth recording: the initial red run used nextest's default fail-fast, so it stopped at 4/5 tests run and the fifth test — the reserved-edge-field expression UPDATE, the only new coverage for two of the three de-qualify hunks — never executed on the unpatched tree. Re-running the same filter with --no-fail-fast produced 5 tests run: 0 passed, 5 failed, exit 100. No source or test file changed as a result; the reviewed commit is the same one the green proof names.

Points the reviewer settled rather than assumed: the denial arm maps to INSUFFICIENT_PRIVILEGE at error_map.rs:155 and the unmapped catch-all at error_map.rs:221 preserves XX000 plus the original message, so no previously-working code became less specific; get_committed_collection keys the table on (database_id.as_u64(), "{tenant_id}:{name}") at security/catalog/collections.rs:308,316, which is what makes de-qualifying the correct direction; and each of the three changed functions has exactly one catalog collection read, so no sibling read was left qualified.

Scope notes

• Fixes CRDT MERGE renders a policy denial as XX000 #344.
• Refs HTTP drops the shaping failure's class: allowlist 400, else 500; in-band lines carry no code #343 — partially: the in-band code half. The status-by-class list that issue asks for (42501 → 403, 22xxx → 400, 57xxx → 503) is deliberately not here: it depends on the SQLSTATE table the maintainers are settling in-tree for Routed response-shaper errors flatten to XX000; the numeric SQLSTATE mapper covers 20 of 81 codes #338, and it lives in result_shape.rs rather than this change's files.
• The stream and DDL-dispatch sites are not in this PR beyond the in-band code above. The earlier attempt (fix(pgwire): classify the streamed and DDL shaping failures #341) was closed as stacked on fix(pgwire): complete the shaper error classification and keep internal detail in the log #359; per that closure, that work waits for the mapper settlement and then rebases onto the then-current main.
• The four new refusal tests assert the refusal by message through the shared expect_error helper, not by SQLSTATE; only the crdt_apply test pins 42501 directly. Tightening the other four to assert the class too is left as a follow-up.

---

Review 2 follow-up (3 commits added)

An independent, read-only Review 2 audit of 9c0cb47f3 returned FAIL with 3 blockers. All three are addressed in the three commits now on top of it; the audit's own verdict on this head is below.

Blocker 1 — the same defect survived at a sixth site, and it was the consumer of this PR's own change. plan_needs_implicit_edge_recon (planner/calvin/dependent_recon.rs) fed the plan's database-qualified collection straight to a catalog keyed by the bare name. In every non-default database the read missed, has_implicit_edges read false, the gate returned None, and the OLLP/Calvin dependent-edge reconnaissance never routed — so the mirrored edges of a PK-equality UPDATE/DELETE were never cleaned up. This is the exact class the five de-qualified reads fix, and it is the gate that had to act on the lowering this PR introduces (update_delete/update.rs, delete.rs). Fixed in fae15ae0: the catalog read reduces to the bare name, while the returned tuple keeps the plan's qualified routing key. Both call sites (pgwire/.../pre_dispatch.rs, native/dispatch/edge_recon_gate.rs) were checked and observe no change in the tuple.

Blocker 2 — the restore path's new comment asserted the opposite of the truth. It claimed the collection registry qualifies the bare string before the engine is keyed. It does not: the string reaches from_stored and the tenant engine's collection map verbatim, which is precisely why restore and an ordinary apply address different documents outside DatabaseId::DEFAULT. The mismatch is pre-existing and left alone; the comment now states it instead of denying it.

Blocker 3 — the lint gate is red on main, not on this branch. cargo clippy --profile ci -p nodedb --all-targets --all-features -- -D warnings exits 101 on clippy::nonminimal_bool at planner/sql_plan_convert/dml/vector_primary.rs:161. That line is byte-identical to origin/main and is not in this diff, so it was never this PR's to fix, and a one-line is_none_or rewrite was briefly carried here to unblock the lint.

It has since been removed from this branch so the PR stays exactly its own scope. The consequence is explicit: while origin/main still carries that lint, this branch's lint job is red for a reason unrelated to its own code, and merging the rewrite PR first turns it green again. Do not read a red lint here as a fault in this change — the diff contains no clippy diagnostics.

The test blocker, and the red arm

The audit also required a test that the fix could fail: plan_needs_implicit_edge_recon had zero test call sites anywhere, and the existing non-default-database graph test sends an expression update that the planner rejects at plan time — it never reaches this gate, so reverting the fix left the whole suite green.

f468c66e adds two tests that drive the gate directly: seed a catalog with an edge-bearing collection under a non-default DatabaseId, hand the gate a BulkUpdate carrying the database-qualified collection the planner builds, and assert it fires and returns that qualified key; a second covers DatabaseId::DEFAULT as the identity case.

Red arm, run by reverting only the bare-name lookup:

arm	result
fix reverted	gate_fires_for_an_edge_bearing_collection_in_a_non_default_database FAILED — "the gate must fire … a qualified catalog lookup misses and the mirrored edges leak"; the default-database test still passed
fix restored	2 passed; 0 failed

Evidence at this head: cargo clippy --profile ci -p nodedb --all-targets --all-features -- -D warnings → exit 0; bash ~/scripts/nodedb-preflight.sh → exit 0, no VIOLATION lines; the targeted wire suites (engine_surface_crdt_document, crdt_write_rls_database_scope, engine_surface_graph, sql_transactions_graph*) → 29/29 passed.

Left as follow-ups, not folded in here: the six byte-identical db-prefix strippers across the tree (five strip_db_prefix copies plus bare_collection_name) are worth consolidating into target_identity, and a typed QualifiedCollection::from_request(db, form) would have made blocker 1 impossible to write — the audit's strongest refactor signal. The restore-path routing mismatch still stands. Both are separate changes; this PR stays a correctness fix.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.