fix(platform): return 409 (not 500) when re-pushing a published ruleset#165
Merged
Conversation
Re-pushing a draft whose version matches what's already published hits a deliberate guard in save_draft_ruleset (silently overwriting a published snapshot in place is never right — the client needs to bump the version first). But the guard raised a bare anyhow error, and the handler only special-cased the sibling optimistic-lock "conflict" string — every other store error, including this one, fell through to PlatformError::Internal, which is hardcoded to a generic 500 'Internal server error' with the actual message discarded. A dedicated 409 mapping (ruleset.version_bump_required) already existed in error.rs's code table; it was unreachable, since that table is only consulted for PlatformError::Conflict, never Internal. Symptom: `ordo push` against an already-published ruleset (the common case — most pushes are re-pushes) returns HTTP 500 common.internal_server_error with no indication of what to fix, instead of a 409 telling the client to bump the version. Fix: share the guard's message as a crate-level constant (RULESET_VERSION_BUMP_REQUIRED) instead of a string literal only the raise site knew about, and special-case it in the handler exactly like the existing "conflict" arm — routing it through PlatformError::conflict() so it reaches the mapping that was already there. Added a test pinning the anyhow round-trip so the two sides can't silently drift apart again, which is exactly how this bug was introduced.
|
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.
The bug
`ordo push` against an already-published ruleset — i.e. re-pushing without bumping the version, the common case since most pushes are re-pushes of an existing ruleset — returns HTTP 500 `common.internal_server_error` with no indication of what went wrong, instead of a clear 409 telling the client to bump the version.
Confirmed live: pushing 3 rulesets where 2 already existed on the platform (`circle-post-risk`, `listing-risk`) both 500'd; the 1 new one (`report-triage`) pushed fine — isolating the trigger to "ruleset already exists with the same version."
Root cause
`save_draft_ruleset` (`store/rulesets.rs`) deliberately guards against silently overwriting a published snapshot in place — if the incoming draft's version equals `published_version`, it raises a bare `anyhow!("Published ruleset changes require a new version number")`.
The handler (`ruleset_draft.rs::save_draft`) only special-cased its sibling error — the optimistic-lock `"conflict"` string — into a proper 409. Every other store error, including this one, fell through to `PlatformError::Internal`, which is hardcoded to a generic 500 with the actual message discarded (see `error.rs`'s `Internal` arm).
The frustrating part: a dedicated 409 mapping already existed — `error.rs`'s code table has `(StatusCode::CONFLICT, "Published ruleset changes require a new version number") => "ruleset.version_bump_required"`. It was dead code, because that table is only consulted for `PlatformError::Conflict`, never `Internal`.
Fix
Share the guard's message as a crate-level constant (`RULESET_VERSION_BUMP_REQUIRED` in `store.rs`) instead of a string literal only the raise site knew about, and add a second match arm in the handler — same pattern as the existing `"conflict"` arm — routing it through `PlatformError::conflict()` so it reaches the mapping that was already sitting there unused.
Added a unit test pinning the `anyhow!()` round-trip (message in == message out), because that exact assumption — two layers agreeing on an error string with nothing enforcing it — is how this bug was introduced in the first place (the handler's match was written once and never updated when this second guard was added).
Verification
`cargo test -p ordo-platform` (34 tests, incl. the new one), `cargo clippy --all-targets`, `cargo fmt --check`, `cargo check --workspace` all clean.