fix(cli): validate facts.json/concepts.json enums locally, before push#166
Merged
Conversation
Neither ordo validate nor ordo push checked facts.json/concepts.json against anything — push.rs's sync_facts/sync_concepts forwarded them to the platform as opaque JSON. The platform's NullPolicy (error/default/ skip) and FactDataType (string/number/boolean/date/object) enums are strict: an unrecognized variant fails the server's JSON deserialization outright, surfacing as a 4xx only at push time, after every prior catalog entry already round-tripped over the network. Found live: a facts.json using null_policy values (reject, default_false, default_zero) that were never valid, undetected because nothing local checked this field and the pushes had been masked by the save_draft_ruleset 500 (now fixed separately) until that stopped hiding it. Add crates/ordo-cli/src/catalog.rs — a small, independently-maintained mirror of the platform's two enums (ordo-cli talks to the platform over HTTP, not as a Rust dependency on ordo-platform, so there's no path to import the real types) — and wire it into both: - ordo push: validated before sync_facts/sync_concepts is ever called, so an invalid catalog file fails fast locally with a specific per-field message, no network round-trip. - ordo validate: reported as a facts.json/concepts.json entry alongside ruleset reports, reusing the exact same report/print/JSON shape (extends what its own doc comment already promises: catching what the platform would reject, offline). Moved push.rs's private read_array to project.rs as read_json_array so both push and validate can share it. Verified end-to-end through the compiled binary (not just unit-level): push rejects a bad null_policy/data_type without ever attempting a network call (ORDO_API_URL points at a guaranteed-refused port, so a regression that bypasses validation fails the test fast instead of hanging), and ordo validate reports the same. cargo test/clippy/fmt clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Andero-Wang
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while testing #165 live: pushing a real project's `facts.json` returned a 500 that turned out to be masking a second, independent bug once #165 fixed the 500 into a proper 409.
The bug
Neither `ordo validate` nor `ordo push` checked `facts.json`/`concepts.json` against anything. `push.rs`'s `sync_facts`/`sync_concepts` forward them to the platform as opaque JSON `Value`s with zero local structure checks.
The platform's `NullPolicy` (`error`/`default`/`skip`) and `FactDataType` (`string`/`number`/`boolean`/`date`/`object`) enums are strict — an unrecognized variant fails the server's JSON deserialization outright (a 4xx from Axum's default `Json` rejection), and only at push time, after every catalog entry ahead of it already round-tripped over the network.
Confirmed live: a `facts.json` using `null_policy` values (`reject`, `default_false`, `default_zero`) that were never valid — undetected because nothing local ever checked this field, and the resulting error had been masked by the `save_draft_ruleset` 500 (#165) until that stopped hiding it.
The fix
New `crates/ordo-cli/src/catalog.rs` — a small, independently-maintained mirror of the platform's two enums. (`ordo-cli` talks to the platform over HTTP via `ordo-api-client`, not as a Rust dependency on `ordo-platform`, so there's no existing path to import the real types — and adding that dependency edge just to share one enum would be the wrong direction.)
Wired into both:
Moved `push.rs`'s private `read_array` to `project.rs` as `read_json_array` so both commands share it instead of duplicating.
Verification
End-to-end through the compiled binary, not just unit-level (`tests/push_catalog_validation.rs`): `ordo push` rejects a bad `null_policy`/`data_type` without ever attempting a network call — `ORDO_API_URL` points at a port nothing listens on, so if a regression ever bypasses the local check, the test fails fast on connection-refused instead of hanging on a slow timeout. `ordo validate` reports the same failure through its own output.
`cargo test -p ordo-cli` (56 tests across 5 binaries), `cargo clippy --all-targets`, `cargo fmt --check`, `cargo check --workspace` all clean.
Independent of #165 — this branch is off current `main`, doesn't touch `ordo-platform` at all.