fix(server): stop imposing one instance's status workflow - #77
Merged
Conversation
update_bug_status hard-coded a workflow no Bugzilla instance is required to have: a local CLOSED/resolution pre-check that missed RESOLVED and any custom closed status, and a synthesised "resolution": "" on every non-CLOSED/VERIFIED target that Bugzilla's _check_resolution actually rejects (missing_resolution) on a stock RESOLVED transition. mark_as_duplicate had the same assumption, hard-coding status=CLOSED alongside dupe_of. Bug.pm's set_bug_status already clears the resolution itself when the target status is open, and set_dup_id already applies the instance's duplicate_or_move_bug_status when dupe_of is set alone — so both local guesses were dead weight on stock behavior and wrong on customized workflows. Drop them: update_bug_status sends resolution only when the caller gives a non-empty one, and mark_as_duplicate sends dupe_of (and the comment) only, letting Bugzilla decide the resulting status. Accepted behavior change: mark_as_duplicate now lands a bug in whatever status the instance's duplicate_or_move_bug_status names (RESOLVED/DUPLICATE on stock Bugzilla) rather than always CLOSED.
bug_fields' detail projection reduced legal values to bare names,
dropping the is_open and can_change_to data Bugzilla attaches to
bug_status specifically — exactly what a client would need to learn
an instance's status workflow instead of guessing it, which is what
update_bug_status and mark_as_duplicate now defer to Bugzilla for.
Project each legal value to {name}, plus is_open and can_change_to
(itself projected to [{name, comment_required}]) when the upstream
value carries them. Absent keys are omitted, never null, so every
non-workflow field's values stay exactly as cheap as before. The
catalog shape (no field_names) is untouched, and the data is reported
exactly as Bugzilla gave it — I16's guard-policy exemption already
covers it.
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
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.
Two self-contained commits, each verified independently (builds/tests/lints
clean on its own).
What
update_bug_statusandmark_as_duplicatehard-coded one Bugzillainstance's status workflow, and
bug_fieldsdiscarded exactly the datathat would let a client discover it instead of guessing.
fix(server): stop imposing one instance's status workflow—update_bug_statusdropped a localCLOSED-requires-resolutionpre-check (it missed
RESOLVEDand every custom closed status, andfired on instances where
CLOSEDisn't even the closing status) alongwith a synthesised
"resolution": ""on every non-CLOSED/VERIFIEDtarget — verified against
Bugzilla::Bug::set_bug_status, that emptystring makes a stock
RESOLVEDtransition fail withmissing_resolution, an upstream error the tool was causing, notavoiding. The payload now sends
statusalways andresolutiononlywhen the caller gives a non-empty one; Bugzilla enforces the
resolution requirement itself and clears it automatically on reopen.
mark_as_duplicatesimilarly stopped forcingstatus = CLOSED—set_dup_idalready applies the instance's ownduplicate_or_move_bug_status(plus resolutionDUPLICATE) fromdupe_ofalone.feat(server): report the status workflow in bug_fields—bug_fields's detail projection reduced legal values to bare names,dropping
is_openandcan_change_to, the exact fields Bugzillaattaches to
bug_statusfor this purpose. Each legal value nowprojects to
{name}plusis_open/can_change_to(itself[{name, comment_required}]) when the upstream value carries them;absent keys are omitted, never
null, so every other field's valuesstay exactly as cheap as before.
Behavior change
On a stock Bugzilla instance,
mark_as_duplicatenow leaves a bug inRESOLVED/DUPLICATErather than alwaysCLOSED/DUPLICATE, because theoperator's
duplicate_or_move_bug_statusdecides, not this server.Anyone with a runbook expecting
CLOSED/DUPLICATEneeds to update it.Invariants touched
bug_id/duplicate_ofare untouched, only the resulting write payload changed.
audit_refusal_textentry and writes exactly one audit record percall (existing tests unmodified).
bug_fields's workflow data is still reportedexactly as Bugzilla gave it, never filtered against the guard policy.
Verification
cargo fmt --checkcargo clippy --workspace --all-targets --locked -- -D warningscargo clippy -p bugwarden --features gen --all-targets --locked -- -D warningscargo test --workspace --all-targets --locked— all green, including:crates/bugwarden/tests/tools_wiremock.rs:update_bug_statuswithout a resolution PUTs exactly
{"status": ...}(no synthesisedresolution); with a resolution PUTs both; targetingCLOSEDwithout one now reaches upstream instead of being refused locally;
mark_as_duplicatePUTs exactlydupe_of+ comment; thebug_fieldsdetail test extended to assert
is_open/can_change_tosurvive theprojection for
bug_statuswhile a plain field (priority) stillprojects to
{name}-only.cargo deny check— clean (no new dependencies).test, deny) — both build and pass on their own, not just as a pair.