fix(conversations): fail closed on append-back item loss (#837) - #860
Open
leseb wants to merge 1 commit into
Open
fix(conversations): fail closed on append-back item loss (#837)#860leseb wants to merge 1 commit into
leseb wants to merge 1 commit into
Conversation
…y#837) Non-streaming Responses calls tied to a conversation_id persist the input and output items after the turn completes. That append-back logged and swallowed any persistence error and still returned 200, so items could be silently dropped while the client saw success -- the next turn then rehydrated an incomplete history (praxis-proxy#837). Align fail-closed handling with the durability boundary: - Item insertion is the source of truth: on failure, propagate the error. It reaches this point before the buffered response body is released, so under the default failure_mode: closed the body is withheld and the turn aborts instead of returning a success that hides lost items. Operators can still opt out with failure_mode: open. - The denormalized message cache is a self-healing projection rebuilt from the items table, so a post-commit refresh failure is logged and tolerated: failing the turn would drive a client retry that re-appends the same (id-less) input items as duplicates. Add a fault-injecting ConversationItemStore proving item-insert failure returns Err while message-cache refresh failure returns Continue. This does not close the transient stale-cache window; an atomic item-insert + projection is tracked separately. Signed-off-by: Sébastien Han <seb@redhat.com>
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.
Summary
Non-streaming Responses calls tied to a
conversation_idpersist their input/output items after the turn, but that append-back logged and swallowed any persistence error and still returned200, silently dropping items so the next turn rehydrated an incomplete history (#837). This fails closed at the durability boundary: an item-insertion failure now propagates (the error reacheson_response_bodybefore the buffered body is released, so under the defaultfailure_mode: closedthe body is withheld and the turn aborts instead of hiding the loss), while a post-commit message-cache refresh failure is logged and tolerated because that cache is a self-healing projection and failing would drive a retry that duplicates the id-less input items. A fault-injectingConversationItemStoreproves item-insert failure returnsErrand message-cache refresh failure returnsContinue.Related issue
Closes #837
Validation
cargo test -p praxis-ai-apis(2590 passing), incl.on_response_body_surfaces_item_insert_failure(→Err) andon_response_body_tolerates_message_cache_failure(→Continue)openai_conversationsfunctional suite is unaffected (store-failure injection is not reachable through the proxy)make lint(andmake build)Checklist
Signed-off-bytrailer.Breaking changes
Under the default
failure_mode: closed, a conversation whose items fail to persist now aborts the turn (withheld body) instead of returning a success that hides the loss; operators wanting the prior best-effort behavior can setfailure_mode: openon the filter. The transient message-cache stale window is unchanged — fully closing it (atomic item-insert + projection) is tracked separately.