Harden collaboration stream errors and repair stale route assertions - #1288
Merged
Paul Lizer (paullizer) merged 2 commits intoAug 18, 2026
Merged
Conversation
Follow-up to the shared conversation fix in 0.250.224. Three defects were found while tracing that bug; none caused the reported symptoms, so they were kept out of the original change. chat-streaming.js picks its post-error recovery endpoint from conversation_kind, falling back to the personal /conversation/<id>/messages endpoint when it is absent. None of the seven _serialize_stream_error() call sites in the shared stream bridge set it, so a shared conversation would have hit the same 404 that fix removed. It could not fire because the surrounding guard also requires message_id and those payloads never carried one -- a latent trap rather than a live defect, but one that returns the moment anyone adds message_id to an error payload. Rather than repeating the field at seven call sites, funnel every shared stream failure through one nested helper that cannot omit it, and add an AST-walking test asserting exactly one raw _serialize_stream_error call remains, that it sets conversation_kind=COLLABORATION_KIND, and that all failure paths route through the helper. Separately, the Blueprint migration left production with a single @app.route decorator -- an example inside a swagger_wrapper.py docstring -- while 82 test assertions across 40 files still expected the old form. This is how the streaming defect shipped: test_collaboration_shared_ai_workflow.py existed to guard that exact bridge but broke on an @app.route assertion and died before reaching the endpoint-lookup assertion two lines later. Rewrite 59 assertions across 32 files to @bp.route, each verified against a real @bp.route path in application/single_app first. Leave 14 alone because no matching production route exists -- those point at routes that appear to have been removed or renamed, which is a separate problem that must not be hidden behind a passing assertion. Measured on the affected files: 47 failures to 34, zero newly broken. The third defect, a dead window.chatMessages.loadMessages guard that has never worked since commit 54e37c8, is filed as #1286. Sizing it needs a repro, and switching on a path that has never executed in production is not a safe blind change. Refs #1281 Refs #1286 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…i-user-conversation-bug # Conflicts: # application/single_app/config.py # docs/explanation/release_notes.md
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.
Follow-up to #1283, which merged before this commit landed on the branch.
Refs #1281
Refs #1286
Three defects surfaced while fixing the shared conversation bug in #1283. None of them caused the reported symptoms, so they were deliberately kept out of that PR. Two are addressed here; the third is filed for separate investigation.
1. Shared stream errors now always carry
conversation_kindchat-streaming.jspicks its post-error recovery endpoint fromconversation_kind, falling back to the personal endpoint when it's absent:None of the seven
_serialize_stream_error()call sites in the shared stream bridge set it — so a shared conversation would have landed in theelseand hit the exact 404 that #1283 removed.It could not fire in practice. The surrounding guard also requires
message_id, and those error payloads never carried one. So this was a latent trap rather than a live defect — but one that returns the moment anyone addsmessage_idto an error payload, which is a very natural change.Rather than pasting the field into seven call sites — which is how it got missed in the first place — every shared stream failure now funnels through a single nested helper that cannot omit it:
test_collaboration_stream_errors_always_carry_conversation_kindwalks the AST ofstream_collaboration_message_apiand asserts exactly one raw_serialize_stream_errorcall remains, that it setsconversation_kind=COLLABORATION_KIND, and that every failure path routes through the helper. Adding a new error path without the tag now fails CI.2. Repaired 59 stale
@app.routeassertions across 32 test filesProduction has exactly one
@app.routedecorator left — an example inside aswagger_wrapper.pydocstring. The test suite still asserted the old pre-Blueprint form in 82 places across 40 files.This is how the streaming defect in #1283 shipped.
test_collaboration_shared_ai_workflow.pyexisted specifically to guard that bridge, but broke on line 35 (an@app.routeassertion) and died before reaching line 37, which checked the endpoint lookup. The test that should have caught the bug was standing right there, red for an unrelated reason.Each rewrite was verified against a real
@bp.routepath inapplication/single_appbefore being changed.14 occurrences were deliberately left alone because no matching production route exists —
/workflow,/workflow/file-selection,/workflow/summary-selection,/workflow/summary-view,/api/workflow/generate-summary,/api/workflow/generate-pii-analysis,/api/workflow/pdf. Those point at routes that appear to have been removed or renamed. Rewriting them would convert "this test is broken" into "this test passes while checking nothing" — worth investigating separately.Measured on the affected files: 47 failures → 34, zero newly broken.
3. Dead post-stream reload guard — filed as #1286, not fixed here
chat-streaming.js:1449and:1515guard ontypeof window.chatMessages?.loadMessages === 'function', butloadMessagesis not among the six functionschat-messages.jsassigns towindow.chatMessages, andgit log -Sconfirms it never was. The guard has been dead since commit54e37c87.The backend sets
reload_messages: truewhen an agent plugin persists extra message documents into Cosmos, so those messages stay invisible until a manual reload. Impact is probably narrow — the final payload rendersimage_urlseparately — but sizing it needs a repro, and exporting the symbol would switch on a code path that has never executed in production. That's not a safe blind change, so it's tracked rather than rushed.Validation
test_collaboration_multi_user_reload_and_stream_fix.pytest_collaboration_shared_ai_workflow.pytest_message_metadata_loading_fix.pytest_chat_layered_message_masking.pyui_testscollaboration + mask controlsBefore/after was baselined against stashed source rather than assumed. All checks re-run after merging
origin/Development(a2eaa3f3).Version
0.250.227— renumbered from0.250.225during the merge, since Development had already claimed0.250.225and0.250.226.