fix(rules): report proposal resolution failures instead of pretending success - #87
Open
Lafnaps wants to merge 1 commit into
Open
fix(rules): report proposal resolution failures instead of pretending success#87Lafnaps wants to merge 1 commit into
Lafnaps wants to merge 1 commit into
Conversation
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.
Approve a rule proposal while 10 rules are already active (
MAX_ACTIVE_RULES) and watch it "activate":resolve_rule_proposalcallsrules.activate(rule_id), ignores that it returnedNone, stamps the message metadataactivated, and returns 200. The proposal card renders "Activated" forever while the rule is still pending in the Rules panel — the two permanently disagree, and nothing in the UI hints that the click did nothing. The same pattern sits on every branch of the endpoint (activate,draft,dismiss— the store also returnsNonefor an unknown or deleted rule id) and ondemote_rule_proposal.The frontend has the matching half of the problem:
resolveRuleProposal/dismissRuleProposalnever checkresponse.ok, so even an honest 4xx dies as aconsole.errorwith no user feedback.The backend now checks each store call's return value; on failure it answers 409 with an error body and leaves the message metadata alone. The frontend checks
response.okand surfaces the error through the existingshowToast, so the card keeps its real pending state. Happy path unchanged;?v=bumped for rules-panel.js.Tests: new
tests/test_rule_proposals.py— activate at the active-rule limit → 409 and metadata stays pending; activate a deleted rule id → 409, metadata unchanged; happy-path activate → 200, metadataactivated, rule active; demote of a deleted rule → 409. 4 passed. Full suite: 85 passed (81 on current main).Also verified against a live server with a scripted browser, seeding rules through the store and running the same scenario on this branch and on main. With 10 active rules, clicking Activate on a pending proposal now shows the error toast and the card keeps its buttons, and the server keeps the rule
pending. On main the card flips to "Activated" and the message metadata recordsactivatedwhile the rule's real status stayspending— the permanent disagreement described above. With 9 active rules the card resolves and the rule activates exactly as before.