diff --git a/application/single_app/config.py b/application/single_app/config.py index 0c3bc7094..1e8d4e158 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.226" +VERSION = "0.250.227" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index 2f24e5dee..3c706ff45 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -8,7 +8,7 @@ from flask import Response, current_app, jsonify, redirect, request, session, stream_with_context from config import * -from collaboration_models import MEMBERSHIP_STATUS_PENDING, MESSAGE_KIND_AI_REQUEST, add_seconds_to_iso, normalize_collaboration_user, utc_now_iso +from collaboration_models import COLLABORATION_KIND, MEMBERSHIP_STATUS_PENDING, MESSAGE_KIND_AI_REQUEST, add_seconds_to_iso, normalize_collaboration_user, utc_now_iso from functions_appinsights import log_event from functions_authentication import * from functions_collaboration import ( @@ -1471,6 +1471,23 @@ def stream_collaboration_message_api(conversation_id): message_content, ) + def collaboration_stream_error(error_message, **extra_fields): + """Serialize a stream error that stays attributed to this shared conversation. + + Every failure in this bridge belongs to the collaboration conversation, not the + hidden source conversation, so ``conversation_kind`` is always included. The + browser recovery path keys off it to reload through the collaboration endpoint + instead of the personal one, which does not know this conversation id. + """ + 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, + ) + def generate_stream(): try: yield build_user_message_persisted_stream_event( @@ -1487,12 +1504,7 @@ def generate_stream(): }, level=logging.ERROR, ) - yield _serialize_stream_error( - 'Chat streaming endpoint is unavailable', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, - ) + yield collaboration_stream_error('Chat streaming endpoint is unavailable') return buffer = '' @@ -1506,11 +1518,8 @@ def generate_stream(): error_payload = internal_response.get_json(silent=True) or {} except Exception: error_payload = {} - yield _serialize_stream_error( + yield collaboration_stream_error( error_payload.get('error') or error_payload.get('message') or 'Failed to start collaboration AI workflow', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, ) return @@ -1542,12 +1551,9 @@ def transform_event_block(event_block): and stream_payload.get('message_id') ) ): - return _serialize_stream_error( + return collaboration_stream_error( stream_payload.get('error'), partial_content=stream_payload.get('partial_content'), - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, ) if stream_payload.get('error'): stream_payload['done'] = True @@ -1572,11 +1578,8 @@ def transform_event_block(event_block): return f'data: {json.dumps(make_json_serializable(transformed_payload))}\n\n' if not source_message_id: - return _serialize_stream_error( + return collaboration_stream_error( 'AI workflow completed without a source assistant message', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, ) source_user_message_id = str(stream_payload.get('user_message_id') or '').strip() @@ -1599,11 +1602,8 @@ def transform_event_block(event_block): try: source_message_doc = _read_source_message_doc(source_conversation_id, source_message_id) except CosmosResourceNotFoundError: - return _serialize_stream_error( + return collaboration_stream_error( 'Failed to load the generated assistant response', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, ) collaboration_conversation_doc = updated_conversation_doc @@ -1633,11 +1633,8 @@ def transform_event_block(event_block): }, ) if not mirrored_message_doc: - return _serialize_stream_error( + return collaboration_stream_error( 'Failed to mirror the assistant response into the collaboration conversation', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, ) create_collaboration_message_notifications(final_conversation_doc, mirrored_message_doc) @@ -1710,12 +1707,7 @@ def transform_event_block(event_block): level=logging.ERROR, exceptionTraceback=True, ) - yield _serialize_stream_error( - 'Failed to stream collaborative AI response', - user_message_id=serialized_user_message.get('id'), - message_persisted=True, - conversation_id=conversation_id, - ) + yield collaboration_stream_error('Failed to stream collaborative AI response') return Response(stream_with_context(generate_stream()), mimetype='text/event-stream') except CosmosResourceNotFoundError: diff --git a/docs/explanation/fixes/COLLABORATION_MULTI_USER_RELOAD_AND_STREAM_FIX.md b/docs/explanation/fixes/COLLABORATION_MULTI_USER_RELOAD_AND_STREAM_FIX.md index e9e80b2bc..d674d6cd3 100644 --- a/docs/explanation/fixes/COLLABORATION_MULTI_USER_RELOAD_AND_STREAM_FIX.md +++ b/docs/explanation/fixes/COLLABORATION_MULTI_USER_RELOAD_AND_STREAM_FIX.md @@ -1,6 +1,7 @@ # Shared (Multi-User) Conversation Reload and Streaming Fix **Fixed in version: 0.250.224** +**Hardening added in version: 0.250.227** **Tracking issue: [#1281](https://github.com/microsoft/simplechat/issues/1281)** ## Issue Description @@ -183,3 +184,56 @@ The resolver is loaded by compiling just its AST node out of `route_backend_coll * Fix note: this also restores group collaborative conversations, which use the same stream bridge. * Feature documentation: `docs/explanation/features/COLLABORATIVE_CONVERSATIONS_FOUNDATION.md` + +## Follow-up Hardening (0.250.227) + +Three further defects were found while tracing this bug. None of them caused the reported symptoms, so they were kept out of the original fix. + +### Stream errors are now always attributed to the shared conversation + +`chat-streaming.js` chooses its recovery endpoint from `conversation_kind`: + +```js +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 `conversation_kind`, so a shared conversation would fall into the `else` branch and hit the same 404 this fix removed. + +It could not fire in practice, because the surrounding guard also requires `message_id` and those error payloads never carried one. It was a latent trap rather than a live defect: adding `message_id` to an error payload — a natural change — would have reintroduced the bug. + +Rather than adding the field to seven call sites, all shared stream failures now funnel through a single nested helper that cannot omit it: + +```python +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 the test. + +### Stale `@app.route` assertions repaired across the test suite + +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 this exact bridge, but broke on line 35 (`@app.route`) and died before reaching line 37, which checked the endpoint lookup. The test that should have caught the bug was already red for an unrelated reason. + +59 assertions across 32 files were rewritten to `@bp.route`, each 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 — those point at routes that appear to have been removed or renamed, which is a different problem and must not be papered over with a passing assertion. + +Measured effect on the affected files: **47 failures to 34, with zero newly broken.** + +### Dead post-stream reload guard (tracked separately) + +`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 switching on a path that has never executed in production is not a safe blind change. Filed as [#1286](https://github.com/microsoft/simplechat/issues/1286). diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 00dacd6aa..47e9201c3 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,20 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.227)** + +#### Bug Fixes + +* **Shared Conversation Stream Errors Stay Attached to the Shared Conversation** + * Follow-up hardening to the v0.250.224 shared conversation fix. When an AI request in a shared conversation failed, the error the browser received did not say which kind of conversation it belonged to, so the recovery path could have reloaded from the personal endpoint and produced the same "Conversation not found" error that was just fixed. + * It could not actually happen yet because of an unrelated guard, but it would have come back the moment anyone added a message id to those errors. All shared stream failures now go through a single serializer that always tags the conversation, and a test walks the code to prove no failure path can skip it. + * (Ref: #1281, `route_backend_collaboration.py`, `chat-streaming.js`, collaborative AI streaming) + +* **Repaired Route Assertions Across the Test Suite** + * The recent Blueprint security hardening renamed how routes are declared, but 82 assertions across 40 test files still checked for the old form. Those tests were failing on the rename before they ever reached the behavior they were written to protect. + * This is how the shared conversation streaming bug reached users: the test guarding that exact code path was already red for an unrelated reason. 59 assertions across 32 files were corrected, each verified against a real route first. 14 were deliberately left alone because they point at routes that no longer exist, which is a separate issue worth investigating rather than hiding. + * (Ref: #1281, `functional_tests/`, Blueprint route registration) + ### **(v0.250.226)** #### Bug Fixes diff --git a/functional_tests/test_admin_release_notifications_registration.py b/functional_tests/test_admin_release_notifications_registration.py index ac490b6f4..d25177a0b 100644 --- a/functional_tests/test_admin_release_notifications_registration.py +++ b/functional_tests/test_admin_release_notifications_registration.py @@ -99,7 +99,7 @@ def test_release_notifications_backend_and_settings_markers(): raise AssertionError(f'Missing release notifications frontend markers: {missing_frontend}') backend_markers = [ - "@app.route('/api/admin/settings/release_notifications_registration', methods=['POST'])", + "@bp.route('/api/admin/settings/release_notifications_registration', methods=['POST'])", 'def release_notifications_registration():', "'release_notifications_registered': True", 'log_admin_release_notifications_registration(', diff --git a/functional_tests/test_admin_send_feedback_tab.py b/functional_tests/test_admin_send_feedback_tab.py index c01a7a090..fafbd6290 100644 --- a/functional_tests/test_admin_send_feedback_tab.py +++ b/functional_tests/test_admin_send_feedback_tab.py @@ -143,7 +143,7 @@ def test_send_feedback_javascript_and_backend(): raise AssertionError(f'Missing sidebar tab activation markers: {missing_sidebar}') backend_markers = [ - "@app.route('/api/admin/settings/send_feedback_email', methods=['POST'])", + "@bp.route('/api/admin/settings/send_feedback_email', methods=['POST'])", 'def send_feedback_email():', 'log_admin_feedback_email_submission(' ] diff --git a/functional_tests/test_agent_citation_full_results_modal.py b/functional_tests/test_agent_citation_full_results_modal.py index 9514cc899..c162ff475 100644 --- a/functional_tests/test_agent_citation_full_results_modal.py +++ b/functional_tests/test_agent_citation_full_results_modal.py @@ -38,7 +38,7 @@ def test_route_exposes_agent_citation_artifact_endpoint(): route_source = read_file_text(ROUTE_FILE) required_snippets = [ - "@app.route('/api/conversation//agent-citation/', methods=['GET'])", + "@bp.route('/api/conversation//agent-citation/', methods=['GET'])", 'build_message_artifact_payload_map', "artifact_payload_map.get(str(artifact_id or ''))", "return jsonify({'citation': citation})", diff --git a/functional_tests/test_agents_catalog_feature.py b/functional_tests/test_agents_catalog_feature.py index 24ce41525..dc7282e9c 100644 --- a/functional_tests/test_agents_catalog_feature.py +++ b/functional_tests/test_agents_catalog_feature.py @@ -69,7 +69,7 @@ def test_agents_catalog_routes_and_navigation(): sidebar = read_repo_file("application/single_app/templates/_sidebar_nav.html") short_sidebar = read_repo_file("application/single_app/templates/_sidebar_short_nav.html") - assert_contains(app_route, "@app.route('/agents'", "Agents page route") + assert_contains(app_route, "@bp.route('/agents'", "Agents page route") assert_contains(app_route, "@swagger_route(security=get_auth_security())", "Agents route swagger security") assert_contains(app_route, "@login_required", "Agents route login guard") assert_contains(app_route, "@user_required", "Agents route user guard") diff --git a/functional_tests/test_ai_search_index_management_fix.py b/functional_tests/test_ai_search_index_management_fix.py index ae6521e69..428720033 100644 --- a/functional_tests/test_ai_search_index_management_fix.py +++ b/functional_tests/test_ai_search_index_management_fix.py @@ -110,7 +110,7 @@ def test_index_creation_endpoint(): # Check for index creation endpoint required_patterns = [ - "@app.route('/api/admin/settings/create_index', methods=['POST'])", + "@bp.route('/api/admin/settings/create_index', methods=['POST'])", "def create_index():", "from azure.search.documents.indexes.models import SearchIndex", "index = SearchIndex.deserialize(index_definition)", diff --git a/functional_tests/test_analyze_compare_claude_workflow_stream.py b/functional_tests/test_analyze_compare_claude_workflow_stream.py index 7ff95239e..eb3713c1b 100644 --- a/functional_tests/test_analyze_compare_claude_workflow_stream.py +++ b/functional_tests/test_analyze_compare_claude_workflow_stream.py @@ -225,8 +225,8 @@ def test_chat_document_action_stream_uses_workflow_executor(): """Validate the chat Analyze/Compare stream path still uses workflow execution.""" route_text = CHAT_ROUTE.read_text(encoding="utf-8") required_markers = [ - "@app.route('/api/chat/document-action/stream', methods=['POST'])", - "@app.route('/api/chat/analyze/stream', methods=['POST'])", + "@bp.route('/api/chat/document-action/stream', methods=['POST'])", + "@bp.route('/api/chat/analyze/stream', methods=['POST'])", "from functions_workflow_runner import _execute_document_action_workflow", "execution_result = _execute_document_action_workflow(", ] diff --git a/functional_tests/test_chat_layered_message_masking.py b/functional_tests/test_chat_layered_message_masking.py index ea1eca8d4..8d8225aec 100644 --- a/functional_tests/test_chat_layered_message_masking.py +++ b/functional_tests/test_chat_layered_message_masking.py @@ -298,7 +298,7 @@ def test_frontend_and_routes_use_layered_masking_contract() -> None: assert_contains( collaboration_route_source, [ - "@app.route('/api/collaboration/conversations//messages//mask', methods=['POST'])", + "@bp.route('/api/collaboration/conversations//messages//mask', methods=['POST'])", "_assert_user_can_mask_collaboration_message(current_user['user_id'], message_doc)", "_sync_collaboration_mask_metadata_to_source(message_doc)", "'collaboration.message.masked'", diff --git a/functional_tests/test_chat_retry_thought_tracker_init_fix.py b/functional_tests/test_chat_retry_thought_tracker_init_fix.py index a21607f32..24539bf41 100644 --- a/functional_tests/test_chat_retry_thought_tracker_init_fix.py +++ b/functional_tests/test_chat_retry_thought_tracker_init_fix.py @@ -44,8 +44,8 @@ def test_retry_and_edit_paths_initialize_thought_tracker_before_content_safety() print('🔍 Testing retry/edit thought-tracker initialization...') route_source = read_file_text(ROUTE_FILE) - chat_route_marker = "@app.route('/api/chat', methods=['POST'])" - chat_stream_marker = "@app.route('/api/chat/stream', methods=['POST'])" + chat_route_marker = "@bp.route('/api/chat', methods=['POST'])" + chat_stream_marker = "@bp.route('/api/chat/stream', methods=['POST'])" chat_route_index = route_source.find(chat_route_marker) chat_stream_index = route_source.find(chat_stream_marker) diff --git a/functional_tests/test_chat_stream_heartbeat_reattach.py b/functional_tests/test_chat_stream_heartbeat_reattach.py index 417f0239f..20b08528c 100644 --- a/functional_tests/test_chat_stream_heartbeat_reattach.py +++ b/functional_tests/test_chat_stream_heartbeat_reattach.py @@ -42,8 +42,8 @@ def test_chat_stream_heartbeat_and_reattach() -> None: assert_contains(ROUTE_FILE, "yield ': keep-alive\\n\\n'") assert_contains(ROUTE_FILE, "class ActiveConversationStreamSession:") assert_contains(ROUTE_FILE, "CHAT_STREAM_REGISTRY = ActiveConversationStreamRegistry()") - assert_contains(ROUTE_FILE, "@app.route('/api/chat/stream/status/', methods=['GET'])") - assert_contains(ROUTE_FILE, "@app.route('/api/chat/stream/reattach/', methods=['GET'])") + assert_contains(ROUTE_FILE, "@bp.route('/api/chat/stream/status/', methods=['GET'])") + assert_contains(ROUTE_FILE, "@bp.route('/api/chat/stream/reattach/', methods=['GET'])") assert_contains(ROUTE_FILE, "stream_with_context(stream_session.iter_events())") assert_contains(ROUTE_FILE, "import app_settings_cache") assert_contains(ROUTE_FILE, "app_settings_cache.initialize_stream_session_cache(") diff --git a/functional_tests/test_chat_stream_lifecycle_observability.py b/functional_tests/test_chat_stream_lifecycle_observability.py index e2866fb37..657072ce9 100644 --- a/functional_tests/test_chat_stream_lifecycle_observability.py +++ b/functional_tests/test_chat_stream_lifecycle_observability.py @@ -40,7 +40,7 @@ def test_chat_stream_lifecycle_observability() -> None: assert_contains(ROUTE_FILE, "def mark_reattached(self):") assert_contains(ROUTE_FILE, "self._stream_session.note_keepalive(source='bridge')") assert_contains(ROUTE_FILE, "self.note_keepalive(source='session')") - assert_contains(ROUTE_FILE, "@app.route('/api/chat/stream/client-event', methods=['POST'])") + assert_contains(ROUTE_FILE, "@bp.route('/api/chat/stream/client-event', methods=['POST'])") assert_contains(ROUTE_FILE, "def chat_stream_client_event_api():") assert_contains(ROUTE_FILE, "stream_status = stream_session.get_status_snapshot() if stream_session else _build_stream_status_payload(None)") assert_contains(ROUTE_FILE, "stream_session.mark_reattached()") diff --git a/functional_tests/test_chat_stream_retry_context_init_fix.py b/functional_tests/test_chat_stream_retry_context_init_fix.py index bcc75c432..661ab3caa 100644 --- a/functional_tests/test_chat_stream_retry_context_init_fix.py +++ b/functional_tests/test_chat_stream_retry_context_init_fix.py @@ -42,7 +42,7 @@ def test_stream_route_initializes_retry_context_before_use(): print('🔍 Testing streaming retry context initialization...') route_source = read_file_text(ROUTE_FILE) - stream_route_marker = "@app.route('/api/chat/stream', methods=['POST'])" + stream_route_marker = "@bp.route('/api/chat/stream', methods=['POST'])" stream_route_index = route_source.find(stream_route_marker) assert stream_route_index != -1, 'Expected to find the /api/chat/stream route definition.' diff --git a/functional_tests/test_chat_stream_retry_multiendpoint_resolution_fix.py b/functional_tests/test_chat_stream_retry_multiendpoint_resolution_fix.py index 655dc2b7f..2d7b6b009 100644 --- a/functional_tests/test_chat_stream_retry_multiendpoint_resolution_fix.py +++ b/functional_tests/test_chat_stream_retry_multiendpoint_resolution_fix.py @@ -57,11 +57,11 @@ def test_chat_api_uses_shared_multi_endpoint_resolution_for_retry_compatibility( route_source = read_file_text(ROUTE_FILE) chat_route_markers = [ "@bp.route('/api/chat', methods=['POST'])", - "@app.route('/api/chat', methods=['POST'])", + "@bp.route('/api/chat', methods=['POST'])", ] chat_stream_markers = [ "@bp.route('/api/chat/stream', methods=['POST'])", - "@app.route('/api/chat/stream', methods=['POST'])", + "@bp.route('/api/chat/stream', methods=['POST'])", ] chat_route_index = find_first_route_marker(route_source, chat_route_markers) diff --git a/functional_tests/test_collaboration_multi_user_reload_and_stream_fix.py b/functional_tests/test_collaboration_multi_user_reload_and_stream_fix.py index ce9fe9365..2aa023d1e 100644 --- a/functional_tests/test_collaboration_multi_user_reload_and_stream_fix.py +++ b/functional_tests/test_collaboration_multi_user_reload_and_stream_fix.py @@ -2,7 +2,7 @@ # test_collaboration_multi_user_reload_and_stream_fix.py """ Functional test for shared (multi-user) conversation reload and AI streaming. -Version: 0.250.224 +Version: 0.250.227 Implemented in: 0.250.224 This test ensures that: @@ -16,6 +16,8 @@ 3. loadConversationMessages() performs the search-highlight, task-document, and comparison-catalog side effects that the personal loader used to provide, so shared conversations keep parity instead of silently losing them. +4. Every shared stream error is tagged with conversation_kind, so the browser recovery + path can never fall back to the personal messages endpoint (added in 0.250.227). """ import ast @@ -250,6 +252,71 @@ def test_collaboration_loader_keeps_personal_loader_side_effects(): return True +def test_collaboration_stream_errors_always_carry_conversation_kind(): + """Every shared stream error must be tagged as collaborative for browser recovery.""" + print("Testing collaboration stream error attribution...") + + route_source = read_repo_file("application", "single_app", "route_backend_collaboration.py") + module_ast = ast.parse(route_source) + + stream_route_nodes = [ + node for node in ast.walk(module_ast) + if isinstance(node, ast.FunctionDef) and node.name == "stream_collaboration_message_api" + ] + assert len(stream_route_nodes) == 1, "Expected exactly one stream_collaboration_message_api definition." + stream_route_node = stream_route_nodes[0] + + raw_error_calls = [ + node for node in ast.walk(stream_route_node) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "_serialize_stream_error" + ] + assert len(raw_error_calls) == 1, ( + "Shared stream errors must funnel through a single serializer so conversation_kind " + f"can never be omitted, found {len(raw_error_calls)} raw _serialize_stream_error calls." + ) + + raw_error_call = raw_error_calls[0] + keyword_names = {keyword.arg for keyword in raw_error_call.keywords if keyword.arg} + assert "conversation_kind" in keyword_names, ( + "The shared stream error serializer must set conversation_kind." + ) + assert "conversation_id" in keyword_names, ( + "The shared stream error serializer must set conversation_id." + ) + + conversation_kind_value = next( + keyword.value for keyword in raw_error_call.keywords if keyword.arg == "conversation_kind" + ) + assert isinstance(conversation_kind_value, ast.Name) and conversation_kind_value.id == "COLLABORATION_KIND", ( + "conversation_kind must use the shared COLLABORATION_KIND constant." + ) + + helper_nodes = [ + node for node in ast.walk(stream_route_node) + if isinstance(node, ast.FunctionDef) and node.name == "collaboration_stream_error" + ] + assert len(helper_nodes) == 1, "Expected a single collaboration_stream_error helper." + + helper_call_count = sum( + 1 for node in ast.walk(stream_route_node) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "collaboration_stream_error" + ) + assert helper_call_count >= 7, ( + f"Expected every shared stream failure path to use the helper, found {helper_call_count}." + ) + + assert "from collaboration_models import COLLABORATION_KIND" in route_source, ( + "Expected COLLABORATION_KIND to be imported from collaboration_models." + ) + + print("Collaboration stream error attribution passed!") + return True + + def test_version_supports_fix(): """The application version must be at least the fix implementation version.""" print("Testing application version...") @@ -266,6 +333,7 @@ def test_version_supports_fix(): test_stream_view_resolves_through_blueprint_endpoint, test_stream_view_resolves_unprefixed_and_rejects_unknown, test_collaboration_route_uses_resolver_and_logs_failures, + test_collaboration_stream_errors_always_carry_conversation_kind, test_select_conversation_skips_personal_messages_endpoint, test_collaboration_loader_keeps_personal_loader_side_effects, test_version_supports_fix, diff --git a/functional_tests/test_control_center_caching_validation.py b/functional_tests/test_control_center_caching_validation.py index 569284d76..8d07cb25e 100644 --- a/functional_tests/test_control_center_caching_validation.py +++ b/functional_tests/test_control_center_caching_validation.py @@ -72,14 +72,14 @@ def validate_caching_implementation(): validation_results.append(False) # Check for refresh endpoints - if '@app.route(\'/api/admin/control-center/refresh\', methods=[\'POST\'])' in content: + if '@bp.route(\'/api/admin/control-center/refresh\', methods=[\'POST\'])' in content: print("✅ Data refresh endpoint implemented") validation_results.append(True) else: print("❌ Data refresh endpoint missing") validation_results.append(False) - if '@app.route(\'/api/admin/control-center/refresh-status\', methods=[\'GET\'])' in content: + if '@bp.route(\'/api/admin/control-center/refresh-status\', methods=[\'GET\'])' in content: print("✅ Refresh status endpoint implemented") validation_results.append(True) else: diff --git a/functional_tests/test_enhanced_citations_pdf_modal_fix.py b/functional_tests/test_enhanced_citations_pdf_modal_fix.py index 2cade1162..ad8fcb806 100644 --- a/functional_tests/test_enhanced_citations_pdf_modal_fix.py +++ b/functional_tests/test_enhanced_citations_pdf_modal_fix.py @@ -126,7 +126,7 @@ def test_route_enhanced_citations_unchanged(): print("✅ serve_enhanced_citation_content function found") # Check that PDF endpoint is still there - if '@app.route("/api/enhanced_citations/pdf", methods=["GET"])' not in route_content: + if '@bp.route("/api/enhanced_citations/pdf", methods=["GET"])' not in route_content: print("❌ PDF endpoint missing") return False print("✅ PDF endpoint found") diff --git a/functional_tests/test_fact_memory_profile_and_mini_sk.py b/functional_tests/test_fact_memory_profile_and_mini_sk.py index 9922c69e8..a18586b0c 100644 --- a/functional_tests/test_fact_memory_profile_and_mini_sk.py +++ b/functional_tests/test_fact_memory_profile_and_mini_sk.py @@ -358,10 +358,10 @@ def test_route_sources_wire_chat_and_profile_fact_memory_paths(): assert "memory_type': 'instruction'" in route_source or 'FACT_MEMORY_TYPE_INSTRUCTION' in route_source assert "'fact_memory'" in route_source - assert "@app.route('/api/profile/fact-memory', methods=['GET'])" in profile_route_source - assert "@app.route('/api/profile/fact-memory', methods=['POST'])" in profile_route_source - assert "@app.route('/api/profile/fact-memory/', methods=['PUT'])" in profile_route_source - assert "@app.route('/api/profile/fact-memory/', methods=['DELETE'])" in profile_route_source + assert "@bp.route('/api/profile/fact-memory', methods=['GET'])" in profile_route_source + assert "@bp.route('/api/profile/fact-memory', methods=['POST'])" in profile_route_source + assert "@bp.route('/api/profile/fact-memory/', methods=['PUT'])" in profile_route_source + assert "@bp.route('/api/profile/fact-memory/', methods=['DELETE'])" in profile_route_source assert 'FactMemoryStore()' in profile_route_source assert 'memory_type' in profile_route_source diff --git a/functional_tests/test_group_document_metrics_display.py b/functional_tests/test_group_document_metrics_display.py index 84bd07f52..39e361804 100644 --- a/functional_tests/test_group_document_metrics_display.py +++ b/functional_tests/test_group_document_metrics_display.py @@ -33,7 +33,7 @@ def test_group_document_metrics_api(): content = f.read() # Check for group API endpoint - if "@app.route('/api/admin/control-center/groups'" in content: + if "@bp.route('/api/admin/control-center/groups'" in content: print(" ✅ Found groups API endpoint") # Check for document metrics handling in groups endpoint diff --git a/functional_tests/test_group_manage_settings_tab_visibility.py b/functional_tests/test_group_manage_settings_tab_visibility.py index b8e1b1eda..71b4d75bc 100644 --- a/functional_tests/test_group_manage_settings_tab_visibility.py +++ b/functional_tests/test_group_manage_settings_tab_visibility.py @@ -93,7 +93,7 @@ def test_download_settings_patch_responses_match_frontend_contract() -> None: require_ordered_tokens( group_routes, [ - '@app.route("/api/groups//download-settings", methods=["PATCH"])', + '@bp.route("/api/groups//download-settings", methods=["PATCH"])', 'assert_group_role(user_id, group_id, allowed_roles=("Owner", "Admin"))', 'if not is_group_workspace_file_download_admin_enabled(get_settings(), group_doc):', '"success": True,', @@ -104,7 +104,7 @@ def test_download_settings_patch_responses_match_frontend_contract() -> None: require_ordered_tokens( public_routes, [ - '@app.route("/api/public_workspaces//download-settings", methods=["PATCH"])', + '@bp.route("/api/public_workspaces//download-settings", methods=["PATCH"])', 'if not is_public_workspace_file_download_admin_enabled(get_settings(), ws):', '"success": True,', '"disable_file_downloads": ws["disable_file_downloads"],', diff --git a/functional_tests/test_group_model_endpoint_membership_guard.py b/functional_tests/test_group_model_endpoint_membership_guard.py index 05415c3dc..705a24f10 100644 --- a/functional_tests/test_group_model_endpoint_membership_guard.py +++ b/functional_tests/test_group_model_endpoint_membership_guard.py @@ -25,7 +25,7 @@ def test_group_model_endpoint_read_requires_current_membership(): backend_content = read_file_text(backend_path) route_block_start = backend_content.index("def get_group_model_endpoints_route():") - route_block_end = backend_content.index("@app.route('/api/group/model-endpoints', methods=['POST'])") + route_block_end = backend_content.index("@bp.route('/api/group/model-endpoints', methods=['POST'])") route_block = backend_content[route_block_start:route_block_end] expected_guard = ''' assert_group_role( diff --git a/functional_tests/test_group_workflow_activity_view_gate.py b/functional_tests/test_group_workflow_activity_view_gate.py index d8d3b7400..93d9f2a63 100644 --- a/functional_tests/test_group_workflow_activity_view_gate.py +++ b/functional_tests/test_group_workflow_activity_view_gate.py @@ -38,7 +38,7 @@ def _assert_not_contains(content, unexpected, label): def _extract_workflow_activity_route(content): - route_marker = "@app.route('/workflow-activity', methods=['GET'])" + route_marker = "@bp.route('/workflow-activity', methods=['GET'])" start_index = content.find(route_marker) if start_index == -1: return "" diff --git a/functional_tests/test_group_workflows_feature.py b/functional_tests/test_group_workflows_feature.py index 900e1aca0..0069f0835 100644 --- a/functional_tests/test_group_workflows_feature.py +++ b/functional_tests/test_group_workflows_feature.py @@ -60,10 +60,10 @@ def test_group_workflow_feature_contracts(): _assert_contains(group_workflows, "is_file_sync_enabled_for_group", "group File Sync gate"), _assert_contains(group_workflows, "scope_type != FILE_SYNC_SCOPE_GROUP or scope_id != group_id", "group-only File Sync sources"), _assert_contains(group_workflows, "def get_due_group_workflows", "scheduled group workflow query"), - _assert_contains(workflow_routes, "@app.route('/api/group/workflows'", "group workflow list/save route"), - _assert_contains(workflow_routes, "@app.route('/api/group/workflows/agents'", "group workflow agents route"), - _assert_contains(workflow_routes, "@app.route('/api/group/workflows/file-sync-sources'", "group workflow File Sync route"), - _assert_contains(workflow_routes, "@app.route('/api/group/workflows/activity'", "group workflow activity route"), + _assert_contains(workflow_routes, "@bp.route('/api/group/workflows'", "group workflow list/save route"), + _assert_contains(workflow_routes, "@bp.route('/api/group/workflows/agents'", "group workflow agents route"), + _assert_contains(workflow_routes, "@bp.route('/api/group/workflows/file-sync-sources'", "group workflow File Sync route"), + _assert_contains(workflow_routes, "@bp.route('/api/group/workflows/activity'", "group workflow activity route"), _assert_contains(workflow_routes, "def _resolve_group_workflow_request_group", "group activity deep-link resolver"), _assert_contains(workflow_routes, "run_group_workflow", "group workflow runner route call"), _assert_contains(workflow_runner, "def run_group_workflow", "group workflow runner"), diff --git a/functional_tests/test_groups_refresh_comprehensive.py b/functional_tests/test_groups_refresh_comprehensive.py index bc5348fa9..5c5020b9e 100644 --- a/functional_tests/test_groups_refresh_comprehensive.py +++ b/functional_tests/test_groups_refresh_comprehensive.py @@ -80,7 +80,7 @@ def test_comprehensive_groups_refresh(): backend_content = f.read() # Check groups endpoint exists - if "@app.route('/api/admin/control-center/groups', methods=['GET'])" in backend_content: + if "@bp.route('/api/admin/control-center/groups', methods=['GET'])" in backend_content: print(" ✅ Groups API endpoint exists") else: print(" ❌ Groups API endpoint missing") diff --git a/functional_tests/test_groups_refresh_fix.py b/functional_tests/test_groups_refresh_fix.py index 2754227be..1906ce8e4 100644 --- a/functional_tests/test_groups_refresh_fix.py +++ b/functional_tests/test_groups_refresh_fix.py @@ -77,7 +77,7 @@ def test_groups_refresh_functionality(): with open(backend_file_path, 'r', encoding='utf-8') as f: backend_content = f.read() - if "@app.route('/api/admin/control-center/groups', methods=['GET'])" not in backend_content: + if "@bp.route('/api/admin/control-center/groups', methods=['GET'])" not in backend_content: print("❌ Backend groups endpoint not found") return False diff --git a/functional_tests/test_historical_enhanced_citation_revision_rendering_fix.py b/functional_tests/test_historical_enhanced_citation_revision_rendering_fix.py index 5c4ac7949..10cc8da25 100644 --- a/functional_tests/test_historical_enhanced_citation_revision_rendering_fix.py +++ b/functional_tests/test_historical_enhanced_citation_revision_rendering_fix.py @@ -55,7 +55,7 @@ def test_enhanced_citations_route_exposes_exact_document_metadata_lookup(): route_source = read_text(ROUTE_FILE) - assert '@app.route("/api/enhanced_citations/document_metadata", methods=["GET"])' in route_source + assert '@bp.route("/api/enhanced_citations/document_metadata", methods=["GET"])' in route_source assert 'doc_response, status_code = get_document(user_id, doc_id)' in route_source assert 'get_document_blob_storage_info(raw_doc)' in route_source assert '"file_name": raw_doc.get("file_name")' in route_source diff --git a/functional_tests/test_missing_swagger_routes_fix.py b/functional_tests/test_missing_swagger_routes_fix.py index 8ff9c1d19..956cee6dd 100644 --- a/functional_tests/test_missing_swagger_routes_fix.py +++ b/functional_tests/test_missing_swagger_routes_fix.py @@ -33,7 +33,7 @@ def test_backend_control_center_approvals_swagger(): try: content = read_file_contents('application', 'single_app', 'route_backend_control_center.py') required_block = ( - "@app.route('/api/approvals', methods=['GET'])\n" + "@bp.route('/api/approvals', methods=['GET'])\n" " @swagger_route(security=get_auth_security())\n" " @login_required\n" " def api_get_approvals():" @@ -64,7 +64,7 @@ def test_backend_speech_swagger(): return False required_block = ( - "@app.route('/api/speech/transcribe-chat', methods=['POST'])\n" + "@bp.route('/api/speech/transcribe-chat', methods=['POST'])\n" " @swagger_route(security=get_auth_security())\n" " @login_required\n" " def transcribe_chat_audio():" @@ -90,7 +90,7 @@ def test_frontend_approvals_swagger(): try: content = read_file_contents('application', 'single_app', 'route_frontend_control_center.py') required_block = ( - "@app.route('/approvals', methods=['GET'])\n" + "@bp.route('/approvals', methods=['GET'])\n" " @swagger_route(security=get_auth_security())\n" " @login_required\n" " @user_required\n" diff --git a/functional_tests/test_msg_file_upload_support.py b/functional_tests/test_msg_file_upload_support.py index 03cedc872..375f1ae90 100644 --- a/functional_tests/test_msg_file_upload_support.py +++ b/functional_tests/test_msg_file_upload_support.py @@ -229,14 +229,14 @@ def test_chat_upload_route_keeps_required_decorators(): print("Testing chat upload route decorators...") route_source = read_text(CHAT_ROUTE_PATH) - route_start = route_source.index("@app.route('/upload', methods=['POST'])") + route_start = route_source.index("@bp.route('/upload', methods=['POST'])") function_start = route_source.index("def upload_file():", route_start) route_header = route_source[route_start:function_start] assert_order( route_header, [ - "@app.route('/upload', methods=['POST'])", + "@bp.route('/upload', methods=['POST'])", "@swagger_route(security=get_auth_security())", "@login_required", "@user_required", diff --git a/functional_tests/test_openapi_upload_only_flow.py b/functional_tests/test_openapi_upload_only_flow.py index 6e31335c9..b61bf93c6 100644 --- a/functional_tests/test_openapi_upload_only_flow.py +++ b/functional_tests/test_openapi_upload_only_flow.py @@ -37,7 +37,7 @@ def assert_not_contains(file_path: Path, unexpected: str) -> None: def test_openapi_upload_only_flow() -> bool: print('Testing OpenAPI upload-only flow markers...') - assert_contains(ROUTE_FILE, "@app.route('/api/openapi/upload', methods=['POST'])") + assert_contains(ROUTE_FILE, "@bp.route('/api/openapi/upload', methods=['POST'])") assert_not_contains(ROUTE_FILE, "/api/openapi/validate-url") assert_not_contains(ROUTE_FILE, "/api/openapi/download-from-url") diff --git a/functional_tests/test_profile_and_admin_review_tabs.py b/functional_tests/test_profile_and_admin_review_tabs.py index 3fc5f0ca1..600d82917 100644 --- a/functional_tests/test_profile_and_admin_review_tabs.py +++ b/functional_tests/test_profile_and_admin_review_tabs.py @@ -198,20 +198,20 @@ def test_stats_and_export_endpoints_and_documentation(): assert_markers( backend_feedback_route, [ - '@app.route("/feedback/review/stats", methods=["GET"])', - '@app.route("/feedback/review/export", methods=["GET"])', - '@app.route("/feedback/my/stats", methods=["GET"])', - '@app.route("/feedback/my/export", methods=["GET"])', + '@bp.route("/feedback/review/stats", methods=["GET"])', + '@bp.route("/feedback/review/export", methods=["GET"])', + '@bp.route("/feedback/my/stats", methods=["GET"])', + '@bp.route("/feedback/my/export", methods=["GET"])', ], 'backend feedback route', ) assert_markers( backend_safety_route, [ - "@app.route('/api/safety/logs/stats', methods=['GET'])", - "@app.route('/api/safety/logs/export', methods=['GET'])", - "@app.route('/api/safety/logs/my/stats', methods=['GET'])", - "@app.route('/api/safety/logs/my/export', methods=['GET'])", + "@bp.route('/api/safety/logs/stats', methods=['GET'])", + "@bp.route('/api/safety/logs/export', methods=['GET'])", + "@bp.route('/api/safety/logs/my/stats', methods=['GET'])", + "@bp.route('/api/safety/logs/my/export', methods=['GET'])", ], 'backend safety route', ) diff --git a/functional_tests/test_support_menu_user_feature.py b/functional_tests/test_support_menu_user_feature.py index 72bb7e44c..c918858ed 100644 --- a/functional_tests/test_support_menu_user_feature.py +++ b/functional_tests/test_support_menu_user_feature.py @@ -312,13 +312,13 @@ def test_support_menu_navigation_and_routes(): assert top_nav_content.index('id="supportMenuDropdown"') < top_nav_content.index('id="externalLinksDropdown"'), 'Support menu should render before external links in top navigation' route_markers = [ - "@app.route('/support/latest-features')", + "@bp.route('/support/latest-features')", "def support_latest_features():", 'get_visible_support_latest_feature_groups', 'support_previous_release_feature_groups', "render_template(", "'latest_features.html'", - "@app.route('/support/send-feedback')", + "@bp.route('/support/send-feedback')", "def support_send_feedback():", "render_template('support_send_feedback.html')", "@enabled_required('enable_support_menu')", @@ -341,7 +341,7 @@ def test_support_menu_feedback_backend_and_templates(): support_js_content = read_text(SUPPORT_JS) backend_markers = [ - "@app.route('/api/support/send_feedback_email', methods=['POST'])", + "@bp.route('/api/support/send_feedback_email', methods=['POST'])", 'def send_support_feedback_email():', "return jsonify({'error': 'Support menu is available to signed-in app users only'}), 403", "application_title = str(settings.get('app_title') or '').strip() or 'Simple Chat'", diff --git a/functional_tests/test_tabular_generated_output_exports.py b/functional_tests/test_tabular_generated_output_exports.py index cef12be67..7cd1f51be 100644 --- a/functional_tests/test_tabular_generated_output_exports.py +++ b/functional_tests/test_tabular_generated_output_exports.py @@ -160,7 +160,7 @@ def test_generated_tabular_output_download_route() -> None: enhanced_citations_route_content = read_text(ENHANCED_CITATIONS_ROUTE_FILE) - assert '@app.route("/api/chat_artifacts/download", methods=["GET"])' in enhanced_citations_route_content, ( + assert '@bp.route("/api/chat_artifacts/download", methods=["GET"])' in enhanced_citations_route_content, ( "Expected route_enhanced_citations.py to register /api/chat_artifacts/download." ) assert 'def _get_authorized_chat_artifact_message(' in enhanced_citations_route_content, ( diff --git a/functional_tests/test_teams_app_sso.py b/functional_tests/test_teams_app_sso.py index e1419b72d..23f88689b 100644 --- a/functional_tests/test_teams_app_sso.py +++ b/functional_tests/test_teams_app_sso.py @@ -101,7 +101,7 @@ def test_auth_route_teams_token_exchange_contract(): assert not (required_functions - functions) required_snippets = [ - "@app.route('/auth/teams/token-exchange', methods=['POST'])", + "@bp.route('/auth/teams/token-exchange', methods=['POST'])", '@swagger_route(security=get_auth_security())', 'if not ENABLE_TEAMS_SSO:', 'request.get_json(silent=True)', diff --git a/functional_tests/test_workflow_instruction_drafting.py b/functional_tests/test_workflow_instruction_drafting.py index 471b7a44e..9a5de0118 100644 --- a/functional_tests/test_workflow_instruction_drafting.py +++ b/functional_tests/test_workflow_instruction_drafting.py @@ -31,7 +31,7 @@ def test_workflow_instruction_drafting_contract(): action_modal_content = read_text("application/single_app/templates/_plugin_modal.html") assert_app_version_at_least("0.250.028") - assert "@app.route('/api/workflows/draft-instructions', methods=['POST'])" in route_content, ( + assert "@bp.route('/api/workflows/draft-instructions', methods=['POST'])" in route_content, ( "Expected a shared workflow instruction drafting endpoint." ) assert "@swagger_route(security=get_auth_security())" in route_content, ( diff --git a/functional_tests/test_workspace_branding_hero_and_logo.py b/functional_tests/test_workspace_branding_hero_and_logo.py index 3709c7416..23d121944 100644 --- a/functional_tests/test_workspace_branding_hero_and_logo.py +++ b/functional_tests/test_workspace_branding_hero_and_logo.py @@ -152,8 +152,8 @@ def test_workspace_models_and_routes_include_branding_fields(): '"heroColor": normalize_workspace_hero_color', 'logo_metadata = get_workspace_logo_metadata(g)', '**logo_metadata,', - '@app.route("/api/groups//logo", methods=["GET"])', - '@app.route("/api/groups//logo", methods=["POST"])', + '@bp.route("/api/groups//logo", methods=["GET"])', + '@bp.route("/api/groups//logo", methods=["POST"])', ], "group route", ) @@ -163,8 +163,8 @@ def test_workspace_models_and_routes_include_branding_fields(): '"heroColor": normalize_workspace_hero_color', 'logo_metadata = get_workspace_logo_metadata(ws)', '**logo_metadata,', - '@app.route("/api/public_workspaces//logo", methods=["GET"])', - '@app.route("/api/public_workspaces//logo", methods=["POST"])', + '@bp.route("/api/public_workspaces//logo", methods=["GET"])', + '@bp.route("/api/public_workspaces//logo", methods=["POST"])', ], "public workspace route", )