Skip to content

Harden collaboration stream errors and repair stale route assertions - #1288

Merged
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-multi-user-conversation-bug
Aug 18, 2026
Merged

Harden collaboration stream errors and repair stale route assertions#1288
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-multi-user-conversation-bug

Conversation

@paullizer

Copy link
Copy Markdown
Contributor

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_kind

chat-streaming.js picks its post-error recovery endpoint from conversation_kind, falling back to the personal endpoint when it's absent:

if (data.conversation_kind === 'collaborative' && ...loadConversationMessages) {
    // collaboration endpoint
} else {
    loadMessages(data.conversation_id);   // personal endpoint, 404s for shared conversations
}

None of the seven _serialize_stream_error() call sites in the shared stream bridge set it — so a shared conversation would have landed in the else and 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 adds message_id to 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:

def collaboration_stream_error(error_message, **extra_fields):
    """Serialize a stream error that stays attributed to this shared conversation."""
    return _serialize_stream_error(
        error_message,
        user_message_id=serialized_user_message.get('id'),
        message_persisted=True,
        conversation_id=conversation_id,
        conversation_kind=COLLABORATION_KIND,
        **extra_fields,
    )

test_collaboration_stream_errors_always_carry_conversation_kind walks the AST of stream_collaboration_message_api and asserts exactly one raw _serialize_stream_error call remains, that it sets conversation_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.route assertions across 32 test files

Production has exactly one @app.route decorator left — an example inside a swagger_wrapper.py docstring. 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.py existed specifically to guard that bridge, but broke on line 35 (an @app.route assertion) 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.route path in application/single_app before 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:1449 and :1515 guard on typeof window.chatMessages?.loadMessages === 'function', but loadMessages is not among the six functions chat-messages.js assigns to window.chatMessages, and git log -S confirms it never was. The guard has been dead since commit 54e37c87.

The backend sets reload_messages: true when 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 renders image_url separately — 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

Check Result
test_collaboration_multi_user_reload_and_stream_fix.py 7/7 passed
test_collaboration_shared_ai_workflow.py passed
test_message_metadata_loading_fix.py passed
test_chat_layered_message_masking.py passed (was failing)
Route policy tests (3) 12/12 passed
ui_tests collaboration + mask controls 5 passed
Functional tests with repaired assertions 47 → 34 failures, 0 newly broken

Before/after was baselined against stashed source rather than assumed. All checks re-run after merging origin/Development (a2eaa3f3).

Version 0.250.227 — renumbered from 0.250.225 during the merge, since Development had already claimed 0.250.225 and 0.250.226.

Paul Lizer (paullizer) and others added 2 commits August 18, 2026 17:30
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
@paullizer
Paul Lizer (paullizer) merged commit e66c3e0 into Development Aug 18, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant