diff --git a/application/single_app/config.py b/application/single_app/config.py index 6cc00241d..d1e7e92e9 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -101,7 +101,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.256" +VERSION = "0.261.257" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_collaboration.py b/application/single_app/functions_collaboration.py index ae39bdca1..1ad274248 100644 --- a/application/single_app/functions_collaboration.py +++ b/application/single_app/functions_collaboration.py @@ -56,6 +56,7 @@ get_user_groups, ) from functions_message_artifacts import filter_assistant_artifact_items +from functions_message_deletion import exclude_soft_deleted_messages from functions_message_image_revisions import ( IMAGE_REVISIONS_METADATA_KEY, resolve_image_message_content, @@ -967,6 +968,8 @@ def _copy_legacy_personal_messages_to_collaboration(source_conversation_id, coll partition_key=source_conversation_id, )) raw_messages = filter_assistant_artifact_items(raw_messages) + # Deleted while archiving was enabled: not part of the conversation being shared. + raw_messages = exclude_soft_deleted_messages(raw_messages) copied_messages = [] source_to_collaboration_message_ids = {} @@ -1164,6 +1167,7 @@ def _copy_legacy_group_messages_to_collaboration(source_conversation_id, collabo partition_key=source_conversation_id, )) raw_messages = filter_assistant_artifact_items(raw_messages) + raw_messages = exclude_soft_deleted_messages(raw_messages) copied_messages = [] source_to_collaboration_message_ids = {} @@ -1937,12 +1941,19 @@ def remove_personal_collaboration_member(conversation_id, owner_user_id, member_ def list_collaboration_messages(conversation_id): + """Return a shared conversation's messages, oldest first. + + A shared message is deleted outright, but a conversation shared from a personal one can + hold copies of messages that were deleted there while archiving was enabled; earlier + conversions copied them. They are not part of the conversation and are excluded. + Conversation deletion queries the container directly, so the copies still go with it. + """ query = 'SELECT * FROM c WHERE c.conversation_id = @conversation_id ORDER BY c.timestamp ASC' - return list(cosmos_collaboration_messages_container.query_items( + return exclude_soft_deleted_messages(list(cosmos_collaboration_messages_container.query_items( query=query, parameters=[{'name': '@conversation_id', 'value': conversation_id}], partition_key=conversation_id, - )) + ))) def persist_collaboration_message( diff --git a/application/single_app/functions_mcp_server_tools.py b/application/single_app/functions_mcp_server_tools.py index 3552c71dd..8cd24c6f7 100644 --- a/application/single_app/functions_mcp_server_tools.py +++ b/application/single_app/functions_mcp_server_tools.py @@ -31,6 +31,7 @@ sort_documents, ) from functions_message_artifacts import filter_assistant_artifact_items +from functions_message_deletion import is_soft_deleted_message from functions_personal_workflows import ( compute_next_run_at, get_personal_workflow, @@ -286,6 +287,8 @@ def _legacy_message_is_visible(message_item): metadata = message_item.get("metadata", {}) if isinstance(message_item.get("metadata"), dict) else {} if metadata.get("is_generated_chat_artifact", False): return False + if is_soft_deleted_message(message_item): + return False thread_info = metadata.get("thread_info", {}) if isinstance(metadata.get("thread_info"), dict) else {} active_thread = thread_info.get("active_thread") return active_thread is True or active_thread is None or "active_thread" not in thread_info diff --git a/application/single_app/functions_message_deletion.py b/application/single_app/functions_message_deletion.py new file mode 100644 index 000000000..636d70bd4 --- /dev/null +++ b/application/single_app/functions_message_deletion.py @@ -0,0 +1,48 @@ +# functions_message_deletion.py +"""Helpers for soft-deleted chat messages. + +With conversation archiving enabled, deleting a chat message keeps its document in the +messages container with ``metadata.is_deleted`` set, rather than removing it. The delete +route also masks the message, but only as a fail-safe: masking is a separate, reversible +user action, and a deleted message must not come back as a masked placeholder, be copied +into a fork or shared conversation, be searchable, or reach the model. + +Every reader that shows, copies, searches, or sends stored messages to a model therefore +has to drop soft-deleted documents itself. These helpers keep that check in one place. +""" + +from collections.abc import Mapping + + +SOFT_DELETE_METADATA_FIELDS = ( + 'is_deleted', + 'deleted_by_user_id', + 'deleted_timestamp', +) + +# For Cosmos queries that must exclude soft-deleted messages server-side, such as TOP 1 +# lookups where filtering afterwards could leave nothing. The container alias must be `c`. +NOT_SOFT_DELETED_COSMOS_FILTER = ( + '(NOT IS_DEFINED(c.metadata.is_deleted) OR c.metadata.is_deleted != true)' +) + + +def is_soft_deleted_message(message): + """Return True when a stored chat message was deleted while archiving was enabled.""" + if not isinstance(message, Mapping): + return False + metadata = message.get('metadata') + return isinstance(metadata, Mapping) and metadata.get('is_deleted') is True + + +def exclude_soft_deleted_messages(messages): + """Return the messages that were not soft-deleted, preserving their order.""" + return [message for message in messages or [] if not is_soft_deleted_message(message)] + + +def strip_soft_delete_metadata(metadata): + """Remove deletion markers from metadata copied onto a new message, in place.""" + if isinstance(metadata, dict): + for field_name in SOFT_DELETE_METADATA_FIELDS: + metadata.pop(field_name, None) + return metadata diff --git a/application/single_app/functions_orchestration_context.py b/application/single_app/functions_orchestration_context.py index e35153bf7..2a4e0e008 100644 --- a/application/single_app/functions_orchestration_context.py +++ b/application/single_app/functions_orchestration_context.py @@ -41,6 +41,7 @@ from functions_action_catalog import build_action_planner_projection from functions_assist_references import sanitize_reference_label from functions_message_block_revisions import resolve_block_sources_in_content +from functions_message_deletion import is_soft_deleted_message from functions_message_masking import remove_masked_content from functions_orchestration_registry import ( CAPABILITY_ACTION_INVOKE, @@ -1783,6 +1784,8 @@ def normalize_history_message(message): raise ConversationContextError('The conversation contains invalid thread metadata.') if ( metadata.get('masked') + # Deleted while archiving was enabled; its mask is only the delete route's fail-safe. + or is_soft_deleted_message(message) or metadata.get('is_generated_chat_artifact') # Answers built from a stored workflow result, or inheriting one, stay out of # orchestration history until orchestration can re-check that result itself. diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index ca89ea3ce..f171fb235 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -114,6 +114,7 @@ ) from functions_orchestration_output_store import OutputStorageError, OutputUnavailableError from functions_m365_action_cards import strip_pending_action_references +from functions_message_deletion import exclude_soft_deleted_messages from functions_personal_workflows import save_personal_workflow from functions_public_workspaces import ( check_public_workspace_status_allows_operation, @@ -461,7 +462,9 @@ def _collect_fork_documents( all_documents: List[Dict[str, Any]], selected_message_id: str, ) -> List[Dict[str, Any]]: - ordered_documents = sorted(all_documents, key=_message_fork_sort_key) + # Messages deleted while archiving was enabled are not part of the conversation and are + # not copied. The caller still compares the full set to detect concurrent changes. + ordered_documents = sorted(exclude_soft_deleted_messages(all_documents), key=_message_fork_sort_key) timeline_documents = [ document for document in ordered_documents diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index f7e3ca129..5e1207505 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -346,6 +346,11 @@ remove_masked_content, resolve_mask_display_name, ) +from functions_message_deletion import ( + NOT_SOFT_DELETED_COSMOS_FILTER, + exclude_soft_deleted_messages, + is_soft_deleted_message, +) from functions_message_visual_styles import ( UNSET as VISUAL_STYLE_HEIGHT_UNSET, VisualStyleError, @@ -2628,9 +2633,12 @@ def _resolve_prior_turn_history_window(settings=None): def _read_recent_assistant_messages(conversation_id, message_limit): + # A deleted reply's tool results and wording must not carry into later turns. Excluded in + # the query so TOP counts only replies that still exist, and checked again as a backstop. query = ( f'SELECT TOP {int(message_limit)} c.id, c.conversation_id, c.role, c.content, c.metadata, c.agent_citations, c.hybrid_citations FROM c ' 'WHERE c.conversation_id = @conversation_id AND c.role = @role ' + f'AND {NOT_SOFT_DELETED_COSMOS_FILTER} ' 'ORDER BY c.timestamp DESC' ) messages = list(cosmos_messages_container.query_items( @@ -2641,6 +2649,7 @@ def _read_recent_assistant_messages(conversation_id, message_limit): ], partition_key=conversation_id, )) + messages = exclude_soft_deleted_messages(messages) return _sanitize_saved_analysis_history(messages) @@ -26634,6 +26643,11 @@ def mask_message_api(message_id): message_doc = message_results[0] + # Deleted while archiving was enabled. Its mask is the delete route's fail-safe, + # so clearing it here would reveal deleted content to the reader and the model. + if is_soft_deleted_message(message_doc): + return jsonify({'error': 'Message not found'}), 404 + conversation_id = message_doc.get('conversation_id') # Verify ownership - only the message author can mask their message @@ -27014,7 +27028,8 @@ def _find_originating_user_request(conversation_id, message_doc): The nearest preceding user message. "Make it match what we discussed" nearly always means the request the diagram came from, and passing that one message is enough for it - to mean something without sending the whole conversation to redraw a flowchart. + to mean something without sending the whole conversation to redraw a flowchart. A + deleted message is skipped, so its text never reaches the model. Best effort: an edit works perfectly well with no grounding at all, so a failure here is swallowed rather than failing the edit. @@ -27028,6 +27043,7 @@ def _find_originating_user_request(conversation_id, message_doc): 'SELECT TOP 1 c.content FROM c ' 'WHERE c.conversation_id = @conversation_id ' "AND c.role = 'user' AND c.timestamp < @timestamp " + f'AND {NOT_SOFT_DELETED_COSMOS_FILTER} ' 'ORDER BY c.timestamp DESC' ), parameters=[ @@ -28408,6 +28424,10 @@ def build_conversation_history_segments( filtered_messages = filter_assistant_artifact_items(all_messages or []) filtered_messages = hydrate_agent_citations_from_artifacts(filtered_messages, artifact_payload_map) ordered_messages = sort_messages_by_thread(filtered_messages) + # Dropped after ordering, so the surviving messages keep the order the thread chain gives + # them. A deleted message must not reach the summary or the recent window, nor take a slot + # in the history limit; its mask is only a fail-safe, not what keeps it out. + ordered_messages = exclude_soft_deleted_messages(ordered_messages) total_messages = len(ordered_messages) num_recent_messages = min(total_messages, conversation_history_limit) diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index d679d9395..b3e5b9b63 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -75,6 +75,7 @@ copy_message_mask_metadata, resolve_mask_display_name, ) +from functions_message_deletion import NOT_SOFT_DELETED_COSMOS_FILTER, is_soft_deleted_message from functions_message_block_revisions import ( BLOCK_REVISIONS_METADATA_KEY, MAX_CHAT_CONTENT_LENGTH as BLOCK_MAX_CHAT_CONTENT_LENGTH, @@ -903,7 +904,7 @@ def _find_collaboration_originating_request(conversation_id, message_doc): The nearest preceding human message in the shared thread. Only that one message is used, and it is passed to the model as background rather than as instructions — sending a whole shared conversation to redraw one flowchart would be expensive and would hand the model a great - deal of other people's writing. + deal of other people's writing. A message copied in already deleted is skipped. Best effort: an edit works perfectly well with no grounding at all, so a failure here is swallowed rather than failing the edit. @@ -917,6 +918,7 @@ def _find_collaboration_originating_request(conversation_id, message_doc): 'SELECT TOP 1 c.content FROM c ' 'WHERE c.conversation_id = @conversation_id ' "AND c.role = 'user' AND c.timestamp < @timestamp " + f'AND {NOT_SOFT_DELETED_COSMOS_FILTER} ' 'ORDER BY c.timestamp DESC' ), parameters=[ @@ -1953,6 +1955,10 @@ def mask_collaboration_message_api(conversation_id, message_id): message_doc = get_collaboration_message(message_id) if str(message_doc.get('conversation_id') or '').strip() != str(conversation_id or '').strip(): return jsonify({'error': 'Collaborative message not found'}), 404 + # A copy of a message deleted before the conversation was shared. Unmasking it would + # reveal deleted content, and the sync below would unmask the original too. + if is_soft_deleted_message(message_doc): + return jsonify({'error': 'Collaborative message not found'}), 404 _assert_user_can_mask_collaboration_message(current_user['user_id'], message_doc) user_display_name = resolve_mask_display_name(current_user) diff --git a/application/single_app/route_backend_conversations.py b/application/single_app/route_backend_conversations.py index f652808b4..8aaadc331 100644 --- a/application/single_app/route_backend_conversations.py +++ b/application/single_app/route_backend_conversations.py @@ -68,6 +68,11 @@ filter_assistant_artifact_items, hydrate_agent_citations_from_artifacts, ) +from functions_message_deletion import ( + exclude_soft_deleted_messages, + is_soft_deleted_message, + strip_soft_delete_metadata, +) from functions_m365_context import M365PolicyError from functions_m365_pending_delivery import cancel_m365_conversation_deliveries import functions_msgraph_pending_actions @@ -507,7 +512,7 @@ def _query_matching_messages(container, search_term, match_mode): )) return [ - message for message in messages + message for message in exclude_soft_deleted_messages(messages) if _matches_search_text(message.get('content', ''), search_term, match_mode) ] @@ -879,6 +884,63 @@ def _collect_child_message_documents(conversation_id, root_message_ids): return child_docs +def _thread_attempt_number(message_doc): + """Return a message's thread attempt, defaulting to 0 like the attempt sorts do.""" + metadata = (message_doc or {}).get('metadata') or {} + thread_info = metadata.get('thread_info') or {} + return thread_info.get('thread_attempt', 0) + + +def _promote_remaining_thread_attempt(conversation_id, thread_id, deleted_attempt, deleted_message_ids): + """Activate another attempt after a delete removes the active attempt's question. + + Deleting only an answer leaves its question, and so its attempt, in place, and nothing is + promoted. Soft-deleted attempts are never promoted. The promoted attempt becomes the only + active one, so whatever is left of the deleted attempt stops showing beside it. Returns + the promoted attempt number, or None when no attempt was promoted. + """ + thread_messages = list(cosmos_messages_container.query_items( + query=( + 'SELECT * FROM c WHERE c.conversation_id = @conversation_id ' + 'AND c.metadata.thread_info.thread_id = @thread_id' + ), + parameters=[ + {'name': '@conversation_id', 'value': conversation_id}, + {'name': '@thread_id', 'value': thread_id}, + ], + partition_key=conversation_id, + )) + remaining_messages = [ + message for message in exclude_soft_deleted_messages(thread_messages) + if message.get('id') not in deleted_message_ids + ] + remaining_attempts = { + _thread_attempt_number(message) + for message in remaining_messages + if message.get('role') == 'user' + } + if not remaining_attempts or deleted_attempt in remaining_attempts: + return None + + promoted_attempt = min(remaining_attempts) + for message in remaining_messages: + metadata = message.get('metadata') + if not isinstance(metadata, dict): + metadata = {} + message['metadata'] = metadata + thread_info = metadata.get('thread_info') + if not isinstance(thread_info, dict): + thread_info = {} + metadata['thread_info'] = thread_info + should_be_active = _thread_attempt_number(message) == promoted_attempt + if thread_info.get('active_thread') is should_be_active: + continue + thread_info['active_thread'] = should_be_active + patch_chat_message_metadata(cosmos_messages_container, message) + + return promoted_attempt + + def _authorize_personal_conversation_read(user_id, conversation_id): """Load a personal conversation and ensure the caller owns it.""" try: @@ -1117,6 +1179,9 @@ def api_get_messages(): int(item.get('fork_sequence')) if str(item.get('fork_sequence') or '').isdigit() else 0, str(item.get('id') or ''), )) + # Deleted while archiving was enabled. They are masked too, but only as a + # fail-safe: returned here, they would render as masked messages. + all_items = exclude_soft_deleted_messages(all_items) artifact_payload_map = build_message_artifact_payload_map(all_items) all_items = filter_assistant_artifact_items(all_items) @@ -2410,6 +2475,7 @@ def generate_conversation_summary_api(conversation_id): enable_cross_partition_query=True )) raw_messages = filter_assistant_artifact_items(raw_messages) + raw_messages = exclude_soft_deleted_messages(raw_messages) except Exception as e: debug_print(f"Error querying messages for summary: {e}") return jsonify({'error': 'Failed to query messages'}), 500 @@ -2597,6 +2663,8 @@ def search_conversations(): 'per_page': per_page, 'access': access_parameters, 'analysis_result_policy_version': 1, + # Results cached before soft-deleted messages were excluded must not be served. + 'soft_deleted_message_policy_version': 1, } search_cache_key = None if cache_settings.get('enabled') and access_parameters is not None: @@ -2911,6 +2979,8 @@ def delete_message(message_id): return jsonify({'error': 'Message not found'}), 404 message_doc = message_results[0] + if is_soft_deleted_message(message_doc): + return jsonify({'error': 'Message not found'}), 404 conversation_id = message_doc.get('conversation_id') # Verify ownership - only the message author can delete their message @@ -2995,53 +3065,26 @@ def delete_message(message_id): if child_message_docs: messages_to_delete.extend(child_message_docs) - # THREAD ATTEMPT PROMOTION: If deleting an active thread attempt, promote next attempt + # THREAD ATTEMPT PROMOTION: if the delete removes the active attempt's question, + # another attempt takes its place. Deleting only an answer promotes nothing. if messages_to_delete: first_msg = messages_to_delete[0] - thread_id = first_msg.get('metadata', {}).get('thread_info', {}).get('thread_id') - is_active = first_msg.get('metadata', {}).get('thread_info', {}).get('active_thread', True) + first_thread_info = first_msg.get('metadata', {}).get('thread_info', {}) + thread_id = first_thread_info.get('thread_id') + is_active = first_thread_info.get('active_thread', True) if thread_id and is_active: - # Find all other attempts for this thread_id - other_attempts_query = f""" - SELECT * FROM c - WHERE c.conversation_id = '{conversation_id}' - AND c.metadata.thread_info.thread_id = '{thread_id}' - AND c.id NOT IN ({','.join([f"'{m['id']}'" for m in messages_to_delete])}) - AND c.role = 'user' - """ - other_attempts = list(cosmos_messages_container.query_items( - query=other_attempts_query, - partition_key=conversation_id - )) - - # If there are other attempts, promote the next one (lowest thread_attempt) - if other_attempts: - # Sort by thread_attempt to find the next one - other_attempts.sort(key=lambda m: m.get('metadata', {}).get('thread_info', {}).get('thread_attempt', 0)) - next_attempt_number = other_attempts[0].get('metadata', {}).get('thread_info', {}).get('thread_attempt', 0) - - # Activate all messages with this thread_attempt - activate_query = f""" - SELECT * FROM c - WHERE c.conversation_id = '{conversation_id}' - AND c.metadata.thread_info.thread_id = '{thread_id}' - AND c.metadata.thread_info.thread_attempt = {next_attempt_number} - """ - messages_to_activate = list(cosmos_messages_container.query_items( - query=activate_query, - partition_key=conversation_id - )) - - for msg_to_activate in messages_to_activate: - if 'metadata' not in msg_to_activate: - msg_to_activate['metadata'] = {} - if 'thread_info' not in msg_to_activate['metadata']: - msg_to_activate['metadata']['thread_info'] = {} - msg_to_activate['metadata']['thread_info']['active_thread'] = True - patch_chat_message_metadata(cosmos_messages_container, msg_to_activate) - - print(f"Promoted thread_attempt {next_attempt_number} to active after deleting active thread {thread_id}") + promoted_attempt = _promote_remaining_thread_attempt( + conversation_id, + thread_id, + _thread_attempt_number(first_msg), + {message.get('id') for message in messages_to_delete}, + ) + if promoted_attempt is not None: + debug_print( + f"[THREAD] Promoted thread_attempt {promoted_attempt} to active " + f"after deleting the active attempt of thread {thread_id}" + ) deleted_message_ids = [] @@ -3131,6 +3174,8 @@ def retry_message(message_id): return jsonify({'error': 'Message not found'}), 404 original_msg = message_results[0] + if is_soft_deleted_message(original_msg): + return jsonify({'error': 'Message not found'}), 404 conversation_id = original_msg.get('conversation_id') original_role = original_msg.get('role') @@ -3162,9 +3207,11 @@ def retry_message(message_id): if not thread_id: return jsonify({'error': 'Message has no thread_id'}), 400 - user_msg_results = list(cosmos_messages_container.query_items( + # The question is replayed from the earliest attempt that still exists. A deleted + # attempt is not a source: its metadata would make the new question deleted too. + user_msg_results = exclude_soft_deleted_messages(list(cosmos_messages_container.query_items( query=( - "SELECT TOP 1 * FROM c WHERE c.conversation_id = @conversation " + "SELECT * FROM c WHERE c.conversation_id = @conversation " "AND c.metadata.thread_info.thread_id = @thread AND c.role = 'user' " "ORDER BY c.metadata.thread_info.thread_attempt ASC" ), @@ -3173,7 +3220,7 @@ def retry_message(message_id): {"name": "@thread", "value": thread_id}, ], partition_key=conversation_id, - )) + ))) if not user_msg_results: return jsonify({"error": "User message not found in thread"}), 404 original_user_msg = user_msg_results[0] @@ -3254,6 +3301,7 @@ def retry_message(message_id): # Copy metadata but update thread_attempt and keep same thread_id and previous_thread_id from original new_metadata = dict(original_metadata) + strip_soft_delete_metadata(new_metadata) new_metadata['retried'] = True # Mark as retried new_metadata['thread_info'] = { 'thread_id': thread_id, # Keep same thread_id @@ -3375,6 +3423,8 @@ def edit_message(message_id): return jsonify({'error': 'Message not found'}), 404 original_msg = message_results[0] + if is_soft_deleted_message(original_msg): + return jsonify({'error': 'Message not found'}), 404 conversation_id = original_msg.get('conversation_id') original_role = original_msg.get('role') @@ -3464,7 +3514,8 @@ def edit_message(message_id): print(f" ✏️ Deactivated: {msg_id} (role={msg_role}, was_active={old_active}, now_active=False)") - # Get the FIRST user message in this thread (attempt=1) to get original metadata + # Get the earliest user message in this thread that still exists, for its metadata. + # A deleted attempt is not a source: its metadata would make the edit deleted too. user_msg_query = f""" SELECT * FROM c WHERE c.conversation_id = '{conversation_id}' @@ -3472,10 +3523,10 @@ def edit_message(message_id): AND c.role = 'user' ORDER BY c.metadata.thread_info.thread_attempt ASC """ - user_msg_results = list(cosmos_messages_container.query_items( + user_msg_results = exclude_soft_deleted_messages(list(cosmos_messages_container.query_items( query=user_msg_query, partition_key=conversation_id - )) + ))) if not user_msg_results: return jsonify({'error': 'User message not found in thread'}), 404 @@ -3499,6 +3550,7 @@ def edit_message(message_id): # Copy metadata but update thread_attempt, add edited flag, and keep same thread_id new_metadata = dict(original_metadata) + strip_soft_delete_metadata(new_metadata) new_metadata['edited'] = True # Mark as edited new_metadata['thread_info'] = { 'thread_id': thread_id, # Keep same thread_id @@ -3619,6 +3671,8 @@ def switch_attempt(message_id): return jsonify({'error': 'Message not found'}), 404 current_msg = message_results[0] + if is_soft_deleted_message(current_msg): + return jsonify({'error': 'Message not found'}), 404 conversation_id = current_msg.get('conversation_id') # Verify ownership @@ -3643,21 +3697,24 @@ def switch_attempt(message_id): if not thread_id: return jsonify({'error': 'Message has no thread_id'}), 400 - # Get all attempts for this thread_id, ordered by thread_attempt - attempts_query = f""" - SELECT DISTINCT c.metadata.thread_info.thread_attempt - FROM c - WHERE c.conversation_id = '{conversation_id}' - AND c.metadata.thread_info.thread_id = '{thread_id}' - AND c.role = 'user' - ORDER BY c.metadata.thread_info.thread_attempt ASC - """ + # Get the attempts for this thread_id whose question still exists. An attempt whose + # question was deleted is not offered: switching to it would show a deleted turn. attempts_results = list(cosmos_messages_container.query_items( - query=attempts_query, - partition_key=conversation_id + query=( + 'SELECT c.metadata FROM c WHERE c.conversation_id = @conversation_id ' + "AND c.metadata.thread_info.thread_id = @thread_id AND c.role = 'user'" + ), + parameters=[ + {'name': '@conversation_id', 'value': conversation_id}, + {'name': '@thread_id', 'value': thread_id}, + ], + partition_key=conversation_id, )) - available_attempts = sorted([r.get('thread_attempt', 0) for r in attempts_results]) + available_attempts = sorted({ + _thread_attempt_number(result) + for result in exclude_soft_deleted_messages(attempts_results) + }) if not available_attempts: return jsonify({'error': 'No attempts found'}), 404 diff --git a/application/v2_ui/src/lib/deletedMessages.ts b/application/v2_ui/src/lib/deletedMessages.ts new file mode 100644 index 000000000..8d66a9238 --- /dev/null +++ b/application/v2_ui/src/lib/deletedMessages.ts @@ -0,0 +1,19 @@ +// deletedMessages.ts +// Messages deleted while conversation archiving is enabled. +// +// With archiving on, deleting a message keeps its stored document with `metadata.is_deleted` +// set, and masks it only as a fail-safe (route_backend_conversations.py). The server leaves +// those documents out of what it returns. The thread still drops any that arrive, because a +// deleted message must never come back as a masked one, which is what the mask alone renders. + +import type { ChatMessage } from './types'; + +/** Whether a message was deleted while conversation archiving was enabled. */ +export function isDeletedMessage(message: ChatMessage | undefined): boolean { + return message?.metadata?.is_deleted === true; +} + +/** The messages that were not deleted, in their original order. */ +export function withoutDeletedMessages(messages: ChatMessage[] | null | undefined): ChatMessage[] { + return (messages ?? []).filter((message) => !isDeletedMessage(message)); +} diff --git a/application/v2_ui/src/lib/threads.ts b/application/v2_ui/src/lib/threads.ts index c9ac99d98..fbcec571b 100644 --- a/application/v2_ui/src/lib/threads.ts +++ b/application/v2_ui/src/lib/threads.ts @@ -7,7 +7,8 @@ // // `/api/get_messages` filters the list to the active attempt // (route_backend_conversations.py), so the number of attempts can never be counted from the -// messages on screen. Only the switch-attempt endpoint reports the full set. +// messages on screen. Only the switch-attempt endpoint reports the full set, and it leaves out +// attempts whose question was deleted, so the attempt numbers in that set can skip. import type { ChatMessage } from './types'; @@ -38,6 +39,10 @@ export function currentAttempt(message: ChatMessage | undefined): number { export interface AttemptState { /** Whether attempt navigation should be offered at all. */ show: boolean; + /** + * This attempt's position among the known attempts, or its attempt number when the set + * is not known. + */ current: number; /** Total attempts, when it is actually known. Null means "at least `current`". */ total: number | null; @@ -53,6 +58,10 @@ export interface AttemptState { * - this message is attempt 2 or later, which proves earlier attempts exist, or * - the switch-attempt endpoint has already reported the set for this thread. * + * A reported set is counted by position, because attempt numbers skip the attempts that were + * deleted: the third attempt of the two that remain is "2 of 2", not "3 of 2". A set that does + * not contain this attempt was reported before it existed, so it is not used. + * * Until an exact set is known, the total is null and the caller shows the attempt number * alone rather than inventing a denominator. */ @@ -63,9 +72,12 @@ export function attemptState( const current = currentAttempt(message); const threadId = messageThreadId(message); const known = threadId ? attemptsByThread[threadId] : undefined; + const position = Array.isArray(known) ? known.indexOf(current) : -1; - if (Array.isArray(known) && known.length > 1) { - return { show: true, current, total: known.length }; + if (Array.isArray(known) && position >= 0) { + return known.length > 1 + ? { show: true, current: position + 1, total: known.length } + : { show: false, current, total: null }; } if (current > 1) { diff --git a/application/v2_ui/src/stores/chatStore.ts b/application/v2_ui/src/stores/chatStore.ts index ff27e1ae3..e80277a80 100644 --- a/application/v2_ui/src/stores/chatStore.ts +++ b/application/v2_ui/src/stores/chatStore.ts @@ -124,6 +124,7 @@ import { type ContextItem, } from '../lib/chatContext'; import { messageThreadId } from '../lib/threads'; +import { withoutDeletedMessages } from '../lib/deletedMessages'; import { proposalSourceMessageId, type ImageProposalSpec } from '../lib/imageProposalSpec'; import { toast } from './toastStore'; import { ApiError } from '../lib/apiClient'; @@ -2241,20 +2242,23 @@ export const useChatStore = create((set, get) => ({ } set({ activeConversationKind: kind }); - const { messages } = + const { messages: loadedMessages } = kind === 'collaborative' ? await fetchCollaborationMessages(conversationId) : await fetchMessages(conversationId); + // A message deleted while archiving was enabled is never shown, not even as a + // masked one; see deletedMessages.ts. + const messages = withoutDeletedMessages(loadedMessages); if (get().activeConversationId !== conversationId) { return; } - set({ messages: messages ?? [], messagesLoading: false }); - const descriptor = latestSavedAnalysis(messages ?? []); + set({ messages, messagesLoading: false }); + const descriptor = latestSavedAnalysis(messages); if (descriptor && !get().analysisContextChosen) { get().selectAnalysisResult(descriptor, analysisRevision); } - const workflowResult = latestWorkflowResult(messages ?? []); + const workflowResult = latestWorkflowResult(messages); if (workflowResult && !get().analysisContextChosen) { get().selectWorkflowResult(workflowResult, conversationId, analysisRevision); } @@ -3638,7 +3642,7 @@ export const useChatStore = create((set, get) => ({ return 'done'; } try { - const { messages } = + const { messages: loadedMessages } = get().activeConversationKind === 'collaborative' ? await fetchCollaborationMessages(conversationId) : await fetchMessages(conversationId); @@ -3649,15 +3653,18 @@ export const useChatStore = create((set, get) => ({ if (options?.onlyIfUnchanged && (get().streaming || get().messages !== shownBefore)) { return 'superseded'; } - set({ messages: messages ?? [] }); + // A message deleted while archiving was enabled is never shown, not even as a + // masked one; see deletedMessages.ts. + const messages = withoutDeletedMessages(loadedMessages); + set({ messages }); const selected = get().analysisResultContext; - if (selected && get().analysisContextRevision === analysisRevision && !(messages ?? []).some((message) => + if (selected && get().analysisContextRevision === analysisRevision && !messages.some((message) => sameAnalysis(selected, latestSavedAnalysis([message])), )) { get().clearAnalysisResultContext(); } if (!get().analysisContextChosen) { - const descriptor = latestSavedAnalysis(messages ?? []); + const descriptor = latestSavedAnalysis(messages); if (descriptor) { get().selectAnalysisResult(descriptor, analysisRevision); } @@ -3666,7 +3673,7 @@ export const useChatStore = create((set, get) => ({ // with Ask in chat is in no message until its first answer arrives. A result that // became unavailable is refused, and its chip removed, when it is next asked about. if (!get().analysisContextChosen) { - const workflowResult = latestWorkflowResult(messages ?? []); + const workflowResult = latestWorkflowResult(messages); if (workflowResult) { get().selectWorkflowResult(workflowResult, conversationId, analysisRevision); } @@ -3696,9 +3703,9 @@ export const useChatStore = create((set, get) => ({ } else { await deleteMessageApi(messageId, deleteThread); } - // Deletion is soft when archiving is enabled: the server masks the message - // rather than removing it, so the authoritative list is re-read instead of - // trusting the optimistic removal. + // The authoritative list is re-read rather than trusting the optimistic removal: + // deleting a question removes its whole turn, every attempt included, which the + // client cannot work out from the one message it removed. await get().reloadMessages(); } catch (error) { set({ diff --git a/docs/admin/data-lifecycle.md b/docs/admin/data-lifecycle.md index 4246c2704..943d29cff 100644 --- a/docs/admin/data-lifecycle.md +++ b/docs/admin/data-lifecycle.md @@ -71,7 +71,7 @@ The Conversation Archiving section belongs to the Archiving tab. Use it with the | Setting | What it does | Default | Notes | | --- | --- | --- | --- | -| Enable Conversation Archiving | Changes conversation deletion behavior to archive conversations instead of permanently deleting them immediately. | Off | `enable_conversation_archiving`; capability toggle | +| Enable Conversation Archiving | Changes conversation deletion behavior to archive conversations instead of permanently deleting them immediately. Deleted chat messages are archived too, and no longer appear in the conversation, search results, or the context sent to the model. | Off | `enable_conversation_archiving`; capability toggle | ## Common tasks diff --git a/docs/explanation/fixes/SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md b/docs/explanation/fixes/SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md new file mode 100644 index 000000000..50c91c6e1 --- /dev/null +++ b/docs/explanation/fixes/SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md @@ -0,0 +1,80 @@ +# Deleted Messages Shown as Masked Fix + +Fixed in version: **0.261.257** + +Related issue: [#1649](https://github.com/microsoft/simplechat/issues/1649). Gaps in regular masking found while fixing it are tracked in [#1650](https://github.com/microsoft/simplechat/issues/1650). + +## Issue + +With **Enable Conversation Archiving** on, a message deleted in the V2 chat disappeared and then came back as "This message is masked." It stayed that way every time the conversation was opened. With archiving off the same delete worked, which is why the problem only appeared sometimes. + +## Root Cause + +When archiving is enabled, `DELETE /api/message/` is a soft delete. The message document stays in the messages container with `metadata.is_deleted` set, a copy goes to the archive container, and the message is also masked as a fail-safe. Readers were expected to drop deleted documents, but most did not: + +- `/api/get_messages` returned them. The classic chat skipped them in the browser, but V2 did not, so the reload that follows a delete rendered them as masked messages. +- Attempt switching counted deleted attempts and could make one active again. +- Retry and edit copied attempt 1's metadata onto the new question. When attempt 1 had been deleted, the new question was created already deleted and masked. +- Forks and convert-to-shared copied deleted messages into the new conversation, where they also showed as masked. +- Conversation search snippets and the inbound MCP `get_conversation_messages` tool returned their full text. +- Several model paths read them: the summary of older messages (which does not check masks), reuse of earlier tool results, conversation summaries, and diagram-edit grounding. +- The mask routes could clear the fail-safe mask, which V2 offered on the masked placeholder. + +Two related defects in the same flow were also fixed: + +- Deleting only the latest answer of a retried turn re-activated the lowest-numbered earlier attempt. The old question and answer then showed next to the latest question, which no longer had an answer. This happened with permanent deletes too. +- The V2 attempt control showed the attempt number over the number of attempts, so it could read "3/2" once an attempt had been removed. + +## Technical Details + +### Files modified + +- `application/single_app/functions_message_deletion.py` (new): `is_soft_deleted_message`, `exclude_soft_deleted_messages`, `strip_soft_delete_metadata`, and `NOT_SOFT_DELETED_COSMOS_FILTER` for queries that use `TOP`. +- `application/single_app/route_backend_conversations.py`: message list, search (with a new search-cache key), conversation summary, delete, switch-attempt, retry, and edit. +- `application/single_app/route_backend_chats.py`: history segments, recent assistant replies, the personal mask route, and personal diagram-edit grounding. +- `application/single_app/route_backend_collaboration.py`: the shared mask route and shared diagram-edit grounding. +- `application/single_app/functions_collaboration.py`: personal and group conversion copies, and `list_collaboration_messages`. +- `application/single_app/functions_simplechat_operations.py`: fork document collection. +- `application/single_app/functions_mcp_server_tools.py`: inbound MCP message reads. +- `application/single_app/functions_orchestration_context.py`: orchestration history projection. +- `application/v2_ui/src/lib/deletedMessages.ts` (new), `application/v2_ui/src/stores/chatStore.ts`, `application/v2_ui/src/lib/threads.ts`. +- `application/single_app/config.py`: version `0.261.257`. + +### Behavior after the fix + +- A soft-deleted message is left out wherever messages are shown, copied, searched, or sent to a model. Its document and archive copy are unchanged, and it keeps its mask as the fail-safe. +- Delete, retry, edit, switch-attempt, and both mask routes return 404 for a soft-deleted message, the same answer as for one that no longer exists. +- Retry and edit take the question's metadata from the earliest attempt that still exists, and never copy deletion markers. +- Attempt promotion happens only when a delete removes the active attempt's question. It never promotes a deleted attempt, and the promoted attempt becomes the only active one. +- Shared conversations created before this fix can still hold copies of deleted messages. `list_collaboration_messages` hides them at read time, so no data migration is needed. Deleting the shared conversation still removes them, because that path queries the container directly. +- V2 drops deleted messages when a conversation loads and when it is re-read, and counts attempts by position within the set the server reports. + +The classic chat already skipped deleted messages in the browser. The server-side filtering also fixes its shared-conversation view, so no classic JavaScript changed. + +## Validation + +### Tests + +- `functional_tests/test_chat_soft_deleted_message_visibility_fix.py` imports the real modules in a fresh process with network access blocked and runs them against in-memory Cosmos containers, under normal and optimized Python. It covers every path above, the promotion rules for archived and permanent deletes, and the V2 source contracts. +- `ui_tests/test_v2_chat_deleted_messages.py` runs the production message list, composer, and chat store in Chromium. Its HTTP stub returns deleted messages the way the server did before the fix. The test checks that no "This message is masked" placeholder appears on load or after a delete, and that the attempt control reads "1/2" and "2/2" rather than "3/2". +- `functional_tests/test_v2_message_actions.py`: rationale text updated for the re-read that follows a delete. +- `functional_tests/test_analyze_backend_saved_integration.py`, `functional_tests/test_content_screening_history.py`, and `functional_tests/test_group_collaboration_source_storage_fix.py` load production functions into namespaces they build themselves. They now bind `exclude_soft_deleted_messages` from the real helper module. + +### Results + +- The new functional test passes under normal and optimized Python. With the server changes reverted, all 21 of its path scenarios fail, each with the symptom it describes. +- The UI test passes. With the V2 changes reverted, it shows two "This message is masked" placeholders after load, and the attempt control does not read "2/2". +- The related functional suites (conversation routes, retry and edit, forks, sharing, search, MCP, history, orchestration context, masking, export) show no failure that the unmodified code does not also show. +- The V2 typecheck and production build pass. + +### Before and after + +| Scenario (archiving on) | Before | After | +| --- | --- | --- | +| Delete a message in V2 | Returns as "This message is masked" | Stays removed | +| Reopen the conversation | Deleted messages show as masked | Not shown | +| Delete the latest answer of a retried turn | An older attempt reappears beside the latest question | The latest question stays, without the deleted answer | +| Switch attempts after deleting one | Can land on the deleted attempt | Deleted attempts are skipped | +| Fork or share a conversation | Deleted messages are copied | Not copied | +| Search or MCP read | Deleted text is returned | Not returned | +| Summary of older messages sent to the model | Includes deleted text | Excludes it | diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index 68e4a71af..a3ca6cf0d 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -6,6 +6,7 @@ order: 120 category: Version History --- +- [Deleted Messages Shown as Masked Fix](SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md) - [Shared Conversation Diagram Editing Fix](SHARED_CONVERSATION_DIAGRAM_EDITING_FIX.md) - [Chat Document Search File Name and Divider Artifact Fix](CHAT_DOCUMENT_SEARCH_FILENAME_AND_DIVIDER_FIX.md) - [Distroless Runtime Overlay Path Fix](DISTROLESS_RUNTIME_OVERLAY_PATH_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 1d018486b..11b0f1bfd 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](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.257)** + +#### Bug Fixes + +* **Deleted Messages No Longer Come Back As Masked** + * With **Enable Conversation Archiving** on, a message deleted in the V2 chat no longer reappears as "This message is masked", either right after the delete or when the conversation is opened again. Deleted messages are left out wherever messages are shown, copied, searched or sent to the model: the message list, attempt switching, retry and edit, forks, sharing a conversation, conversation search, the inbound MCP message tool, conversation summaries and chat history. Their archived copies are kept as before. + * Delete, retry, edit, attempt switching and both mask actions treat a deleted message as gone, so its fail-safe mask can't be cleared. Shared conversations that already hold copies of deleted messages hide them, with no data migration. + * (Ref: #1649, `functions_message_deletion.py`, `route_backend_conversations.py`, `route_backend_chats.py`, `functions_collaboration.py`, `chatStore.ts`, `deletedMessages.ts`, [Deleted Messages Shown as Masked Fix](fixes/SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md)) + +* **Deleting An Answer No Longer Brings Back An Earlier Attempt** + * Deleting only the latest answer of a retried turn re-activated the lowest-numbered earlier attempt, so an old question and answer showed beside the latest question. Another attempt now takes over only when the active attempt's question is deleted, a deleted attempt is never chosen, and only one attempt stays active. This applies whether or not archiving is on. + * The V2 attempt control counts the attempts that remain, so it reads "2/2" rather than "3/2" after one is removed. + * (Ref: #1649, `_promote_remaining_thread_attempt`, `threads.ts`, [Deleted Messages Shown as Masked Fix](fixes/SOFT_DELETED_MESSAGES_SHOWN_AS_MASKED_FIX.md)) + ### **(v0.261.256)** #### Bug Fixes diff --git a/functional_tests/test_analyze_backend_saved_integration.py b/functional_tests/test_analyze_backend_saved_integration.py index 1bb405ca0..668208f91 100644 --- a/functional_tests/test_analyze_backend_saved_integration.py +++ b/functional_tests/test_analyze_backend_saved_integration.py @@ -1,10 +1,11 @@ # test_analyze_backend_saved_integration.py """ Behavioral integration tests for Analyze presentation and saved-data chat reuse. -Version: 0.261.234 +Version: 0.261.257 Implemented in: 0.261.109 Container-only saved-result access covered in: 0.261.232 Message family constants added to the chat harness in: 0.261.234 +Soft-deleted message filter added to the chat harness in: 0.261.257 Real adapter, artifact, history, chat route and shared section-reader functions run against serialized storage, Flask requests and deterministic model doubles. @@ -57,6 +58,7 @@ agent_runtime = import_app_module("agent_delegation_runtime") result_storage = import_app_module("functions_workflow_result_store") tabular = import_app_module("functions_tabular_orchestration") +deletion = import_app_module("functions_message_deletion") def load_m365_runtime(): @@ -480,6 +482,7 @@ def read_m365_conversation(user_id, conversation_id): ), "extract_chat_completion_response_text": lambda value: value.choices[0].message.content, "filter_assistant_artifact_items": lambda values: list(values), + "exclude_soft_deleted_messages": deletion.exclude_soft_deleted_messages, "build_message_artifact_payload_map": lambda values: {}, "hydrate_agent_citations_from_artifacts": lambda values, payloads: values, "sort_messages_by_thread": lambda values: values, diff --git a/functional_tests/test_chat_soft_deleted_message_visibility_fix.py b/functional_tests/test_chat_soft_deleted_message_visibility_fix.py new file mode 100644 index 000000000..3f961589d --- /dev/null +++ b/functional_tests/test_chat_soft_deleted_message_visibility_fix.py @@ -0,0 +1,745 @@ +#!/usr/bin/env python3 +# test_chat_soft_deleted_message_visibility_fix.py +""" +Functional test for deleted chat messages that came back as masked messages. +Version: 0.261.257 +Implemented in: 0.261.257 + +With conversation archiving enabled, deleting a chat message keeps its document with +``metadata.is_deleted`` set, and masks it only as a fail-safe. This test ensures such a +message stays deleted everywhere: the message list, attempt switching and promotion, retry +and edit, both mask routes, forks, shared-conversation copies, search, MCP reads, the +conversation summary, and every history path that reaches the model. It also checks that +deleting only an answer no longer re-activates an older attempt, and that the V2 client +filters deleted messages and counts attempts by position. Refs #1649. + +The probe runs in a fresh process with the real application modules, network access +blocked (``test_support.offline_bootstrap``), and in-memory Cosmos containers. Every +scenario runs and reports on its own, and the checks are explicit, so they also run under +optimized Python. +""" + +import copy +import json +import os +import re +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + + +ROOT = Path(__file__).resolve().parents[1] +APP_ROOT = ROOT / "application" / "single_app" +V2_SRC = ROOT / "application" / "v2_ui" / "src" +PROBE_FLAG = "--probe" +PROBE_OK = "SOFT_DELETE_PROBE_OK" +PROBE_FAILED = "SOFT_DELETE_PROBE_FAILURES" + +USER = "user-owner" +CONVERSATION = "conversation-1" +SHARED_CONVERSATION = "shared-1" +NOT_SOFT_DELETED = "(NOT IS_DEFINED(c.metadata.is_deleted) OR c.metadata.is_deleted != true)" + + +def check(condition, message): + """Raise rather than assert, so the check survives python -O.""" + if not condition: + raise AssertionError(message) + + +# -------------------------------------------------------------------------------------- +# In-memory Cosmos container used by the probe +# -------------------------------------------------------------------------------------- + +def _path_value(document, path): + value = document + for part in path.split("."): + if not isinstance(value, dict): + return None + value = value.get(part) + return value + + +class FakeContainer: + """Evaluates the small set of query shapes the code under test issues.""" + + def __init__(self, items=()): + self.items = {} + self.queries = [] + self.revision = 0 + self.reset(items) + + def reset(self, items=()): + self.items = {} + self.queries = [] + for item in items: + self._store(item) + + def _store(self, item): + self.revision += 1 + saved = copy.deepcopy(item) + saved["_etag"] = str(self.revision) + self.items[saved["id"]] = saved + return copy.deepcopy(saved) + + def get(self, item_id): + return copy.deepcopy(self.items.get(item_id)) + + def read_item(self, item=None, partition_key=None, **kwargs): + from azure.cosmos.exceptions import CosmosResourceNotFoundError + + document = self.items.get(item) + if document is None: + raise CosmosResourceNotFoundError(status_code=404, message="Not found") + return copy.deepcopy(document) + + def upsert_item(self, body, **kwargs): + return self._store(body) + + create_item = upsert_item + + def replace_item(self, item=None, body=None, etag=None, match_condition=None, **kwargs): + from azure.cosmos.exceptions import CosmosAccessConditionFailedError + + item_id = item if isinstance(item, str) else (item or body)["id"] + current = self.items.get(item_id) + if etag is not None and current is not None and current.get("_etag") != etag: + raise CosmosAccessConditionFailedError(status_code=412, message="Changed") + return self._store(body) + + def delete_item(self, item=None, partition_key=None, **kwargs): + item_id = item if isinstance(item, str) else item["id"] + self.items.pop(item_id, None) + + def query_items(self, query=None, parameters=None, partition_key=None, **kwargs): + self.queries.append(query) + params = {entry["name"]: entry["value"] for entry in parameters or []} + documents = [copy.deepcopy(document) for document in self.items.values()] + if partition_key is not None: + documents = [document for document in documents if document.get("conversation_id") == partition_key] + + def equals(field): + match = re.search(rf"c\.{re.escape(field)}\s*=\s*(?:'([^']*)'|(@\w+))", query) + if not match: + return None + return match.group(1) if match.group(1) is not None else params.get(match.group(2)) + + for field in ( + "conversation_id", + "id", + "role", + "parent_message_id", + "metadata.thread_info.thread_id", + "metadata.thread_info.previous_thread_id", + ): + expected = equals(field) + if expected is not None: + documents = [document for document in documents if _path_value(document, field) == expected] + + before = re.search(r"c\.timestamp\s*<\s*(@\w+)", query) + if before: + limit = params.get(before.group(1)) + documents = [document for document in documents if str(document.get("timestamp") or "") < limit] + if NOT_SOFT_DELETED in query: + documents = [document for document in documents if _path_value(document, "metadata.is_deleted") is not True] + + if "ORDER BY c.metadata.thread_info.thread_attempt" in query: + documents.sort(key=lambda document: _path_value(document, "metadata.thread_info.thread_attempt") or 0) + elif "ORDER BY c.timestamp DESC" in query: + documents.sort(key=lambda document: str(document.get("timestamp") or ""), reverse=True) + else: + documents.sort(key=lambda document: str(document.get("timestamp") or "")) + + selected = re.search(r"SELECT\s+(?:TOP\s+(\d+)\s+)?(.+?)\s+FROM\s+c\b", query, re.DOTALL) + top, fields = (selected.group(1), selected.group(2).strip()) if selected else (None, "*") + if fields.startswith("VALUE MAX("): + path = re.search(r"MAX\(c\.([\w.]+)\)", fields).group(1) + values = [_path_value(document, path) for document in documents] + values = [value for value in values if value is not None] + return [max(values) if values else None] + if top: + documents = documents[: int(top)] + if fields == "*": + return documents + + projected = [] + for document in documents: + row = {} + for field in fields.split(","): + expression, _, alias = field.strip().partition(" as ") + path = expression.strip()[len("c."):] + row[alias.strip() or path.split(".")[-1]] = _path_value(document, path) + projected.append(row) + return projected + + +def soft_deleted(document, timestamp="2026-01-01T09:00:00"): + """Mark a message the way the delete route does when archiving is enabled.""" + document = copy.deepcopy(document) + document["metadata"].update({ + "is_deleted": True, + "deleted_by_user_id": USER, + "deleted_timestamp": timestamp, + "masked": True, + "masked_by_user_id": USER, + "masked_timestamp": timestamp, + }) + return document + + +def message(message_id, role, content, timestamp, thread=None, attempt=1, active=True, conversation=CONVERSATION): + metadata = {} + if thread: + metadata["thread_info"] = { + "thread_id": thread, + "thread_attempt": attempt, + "active_thread": active, + "previous_thread_id": None, + } + if role == "user": + metadata["user_info"] = {"user_id": USER, "display_name": "Owner"} + return { + "id": message_id, + "conversation_id": conversation, + "role": role, + "content": content, + "timestamp": timestamp, + "metadata": metadata, + } + + +def retried_turn(deleted_first_question=False): + """One turn answered twice: attempt 1 inactive, attempt 2 active.""" + first_question = message("q1", "user", "First wording", "2026-01-01T00:00:01", "t1", 1, False) + if deleted_first_question: + first_question = soft_deleted(first_question) + return [ + first_question, + message("a1", "assistant", "First answer", "2026-01-01T00:00:02", "t1", 1, False), + message("q2", "user", "Second wording", "2026-01-01T00:00:03", "t1", 2, True), + message("a2", "assistant", "Second answer", "2026-01-01T00:00:04", "t1", 2, True), + ] + + +def active_flags(store): + return { + item_id: _path_value(document, "metadata.thread_info.active_thread") + for item_id, document in store.items.items() + } + + +# -------------------------------------------------------------------------------------- +# Probe: runs in a fresh process +# -------------------------------------------------------------------------------------- + +def run_probe(): + sys.path.insert(0, str(ROOT / "functional_tests")) + sys.path.insert(0, str(APP_ROOT)) + + from contextlib import ExitStack + from unittest.mock import patch + + from flask import Flask + + from test_support.offline_bootstrap import offline_app_imports + + failures = [] + + with offline_app_imports(), ExitStack() as stack: + import functions_collaboration as collaboration_functions + import functions_mcp_server_tools as mcp_tools + import functions_orchestration_context as orchestration_context + import functions_simplechat_operations as operations + import route_backend_chats as chats + import route_backend_collaboration as collaboration_routes + import route_backend_conversation_export as conversation_export + import route_backend_conversations as conversations + from functions_chat_content_checks import ChatContentDecision + from functions_message_deletion import ( + NOT_SOFT_DELETED_COSMOS_FILTER, + exclude_soft_deleted_messages, + is_soft_deleted_message, + strip_soft_delete_metadata, + ) + + check(NOT_SOFT_DELETED_COSMOS_FILTER == NOT_SOFT_DELETED, "The probe's predicate is out of date") + + messages = FakeContainer() + shared_messages = FakeContainer() + conversation_store = FakeContainer([{"id": CONVERSATION, "user_id": USER, "title": "Review"}]) + archive = FakeContainer() + settings = {"enable_conversation_archiving": True} + not_required = ChatContentDecision("chat_input", "not_required", "allow", {}) + owner = {"user_id": USER, "display_name": "Owner", "email": "owner@example.test"} + ghost = soft_deleted(message( + "ghost", "user", "Deleted before sharing", "2026-01-01T00:00:05", conversation=SHARED_CONVERSATION, + )) + + for module in (conversations, chats, collaboration_routes): + for decorator in ("login_required", "user_required"): + stack.enter_context(patch.object(module, decorator, lambda function: function)) + for module, replacements in ( + (conversations, { + "get_current_user_id": lambda: USER, + "get_settings": lambda *args, **kwargs: settings, + "cosmos_messages_container": messages, + "cosmos_conversations_container": conversation_store, + "cosmos_archived_messages_container": archive, + "cleanup_chat_analysis_messages": lambda *args, **kwargs: None, + "_rebuild_authorized_personal_conversation_used_documents": lambda *args, **kwargs: None, + "_invalidate_conversation_cache_after_message_mutation": lambda *args, **kwargs: None, + "check_chat_content": lambda *args, **kwargs: not_required, + "debug_print": lambda *args, **kwargs: None, + }), + (chats, { + "get_current_user_id": lambda: USER, + "get_current_user_info": lambda: {"displayName": "Owner"}, + "cosmos_messages_container": messages, + "cosmos_conversations_container": conversation_store, + "persist_chat_reply": lambda container, document, *args, **kwargs: container.upsert_item(document), + }), + (collaboration_routes, { + "_require_collaboration_feature_enabled": lambda: {}, + "_get_current_collaboration_user": lambda: dict(owner), + "get_collaboration_conversation": lambda conversation_id: {"id": conversation_id}, + "assert_user_can_participate_in_collaboration_conversation": lambda *args, **kwargs: {}, + "get_collaboration_message": lambda message_id: copy.deepcopy(ghost), + "cosmos_collaboration_messages_container": shared_messages, + }), + (collaboration_functions, {"cosmos_collaboration_messages_container": shared_messages}), + (mcp_tools, {"cosmos_messages_container": messages}), + ): + for name, value in replacements.items(): + stack.enter_context(patch.object(module, name, value)) + + conversations_app = Flask("soft-delete-conversations") + conversations_app.config["TESTING"] = True + conversations.register_route_backend_conversations(conversations_app) + chats_app = Flask("soft-delete-chats") + chats_app.config["TESTING"] = True + chats.register_route_backend_chats(chats_app) + collaboration_app = Flask("soft-delete-collaboration") + collaboration_app.config["TESTING"] = True + collaboration_routes.register_route_backend_collaboration(collaboration_app) + client = conversations_app.test_client() + + def visible_ids(): + response = client.get(f"/api/get_messages?conversation_id={CONVERSATION}") + check(response.status_code == 200, f"get_messages failed: {response.status_code} {response.get_data(as_text=True)}") + return [item["id"] for item in response.get_json()["messages"]] + + def scenario(function): + """Run one scenario now and record its failure, so every failing path is reported.""" + try: + function() + except Exception as error: + failures.append(f"{function.__name__}: {type(error).__name__}: {error}") + finally: + settings["enable_conversation_archiving"] = True + return function + + @scenario + def helper_semantics(): + live = message("live", "user", "Hello", "2026-01-01T00:00:01") + gone = soft_deleted(message("gone", "user", "Bye", "2026-01-01T00:00:02")) + check(is_soft_deleted_message(gone) and not is_soft_deleted_message(live), "Soft-delete predicate is wrong") + check(not is_soft_deleted_message({"metadata": {"is_deleted": "true"}}), "Only a real True marks deletion") + check(not is_soft_deleted_message(None) and not is_soft_deleted_message({"metadata": None}), "Malformed input") + check([item["id"] for item in exclude_soft_deleted_messages([live, gone])] == ["live"], "Filter kept a deleted message") + stripped = strip_soft_delete_metadata(dict(gone["metadata"])) + check(not {"is_deleted", "deleted_by_user_id", "deleted_timestamp"} & set(stripped), "Deletion markers were kept") + check(stripped.get("masked") is True, "Stripping deletion markers must not touch masks") + + @scenario + def message_list_excludes_deleted(): + messages.reset([ + message("q0", "user", "Kept question", "2026-01-01T00:00:01", "t0"), + message("a0", "assistant", "Kept answer", "2026-01-01T00:00:02", "t0"), + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:03", "t9")), + soft_deleted(message("a9", "assistant", "Deleted answer", "2026-01-01T00:00:04", "t9")), + ]) + check(visible_ids() == ["q0", "a0"], "Soft-deleted messages were returned and would render as masked") + + @scenario + def deleting_an_answer_promotes_nothing(): + messages.reset(retried_turn()) + response = client.delete("/api/message/a2", json={"delete_thread": False}) + check(response.status_code == 200, f"Delete failed: {response.get_data(as_text=True)}") + check(response.get_json()["archived"] is True, "Archiving was not used") + check(is_soft_deleted_message(messages.get("a2")), "The answer was not soft-deleted") + check("a2" in archive.items, "The deleted answer was not archived") + flags = active_flags(messages) + check(flags["q1"] is False and flags["a1"] is False, "Deleting an answer re-activated an older attempt") + check(flags["q2"] is True, "The latest question lost its active attempt") + check(visible_ids() == ["q2"], "After deleting an answer, only its question should remain") + + @scenario + def deleting_a_deleted_message_is_not_found(): + messages.reset([soft_deleted(message("a2", "assistant", "Gone", "2026-01-01T00:00:04", "t1", 2))]) + response = client.delete("/api/message/a2", json={"delete_thread": False}) + check(response.status_code == 404, "A soft-deleted message could be deleted again") + + @scenario + def permanent_delete_of_an_answer_promotes_nothing(): + settings["enable_conversation_archiving"] = False + messages.reset(retried_turn()) + response = client.delete("/api/message/a2", json={"delete_thread": False}) + check(response.status_code == 200 and response.get_json()["archived"] is False, "Hard delete failed") + check("a2" not in messages.items, "The answer was not removed") + check(active_flags(messages)["q1"] is False, "A hard delete of an answer re-activated an older attempt") + + @scenario + def deleting_the_question_promotes_the_other_attempt_alone(): + messages.reset(retried_turn()) + response = client.delete("/api/message/q2", json={"delete_thread": False}) + check(response.status_code == 200, f"Question delete failed: {response.get_data(as_text=True)}") + flags = active_flags(messages) + check(flags["q1"] is True and flags["a1"] is True, "The remaining attempt was not promoted") + check(flags["a2"] is False, "The deleted question's answer stayed active beside the promoted attempt") + check(visible_ids() == ["q1", "a1"], "Promotion left the thread showing two attempts") + + @scenario + def a_deleted_attempt_is_never_promoted(): + # With no other question left, the answer whose question was deleted is all that + # remains of the turn. + messages.reset(retried_turn(deleted_first_question=True)) + response = client.delete("/api/message/q2", json={"delete_thread": False}) + check(response.status_code == 200, "Question delete failed") + check(active_flags(messages)["q1"] is False, "A soft-deleted attempt was promoted") + check(visible_ids() == ["a2"], "A deleted attempt came back after promotion") + + @scenario + def attempt_switching_skips_deleted_attempts(): + messages.reset([ + soft_deleted(message("q1", "user", "Deleted wording", "2026-01-01T00:00:01", "t1", 1, False)), + soft_deleted(message("a1", "assistant", "Deleted answer", "2026-01-01T00:00:02", "t1", 1, False)), + message("q2", "user", "Second wording", "2026-01-01T00:00:03", "t1", 2, True), + message("a2", "assistant", "Second answer", "2026-01-01T00:00:04", "t1", 2, True), + message("q3", "user", "Third wording", "2026-01-01T00:00:05", "t1", 3, False), + message("a3", "assistant", "Third answer", "2026-01-01T00:00:06", "t1", 3, False), + ]) + response = client.post("/api/message/a2/switch-attempt", json={"direction": "next"}) + payload = response.get_json() + check(response.status_code == 200, f"Switch failed: {payload}") + check(payload["available_attempts"] == [2, 3], f"Deleted attempts were offered: {payload}") + check(payload["target_attempt"] == 3 and visible_ids() == ["q3", "a3"], "Switching did not reach attempt 3") + response = client.post("/api/message/a3/switch-attempt", json={"direction": "next"}) + check(response.get_json()["target_attempt"] == 2, "Switching wrapped onto a deleted attempt") + check(visible_ids() == ["q2", "a2"], "A deleted attempt was shown after switching") + response = client.post("/api/message/a1/switch-attempt", json={"direction": "next"}) + check(response.status_code == 404, "A soft-deleted message could drive attempt switching") + + @scenario + def retry_does_not_inherit_deletion(): + messages.reset(retried_turn(deleted_first_question=True)) + response = client.post("/api/message/a2/retry", json={}) + payload = response.get_json() + check(response.status_code == 200, f"Retry failed: {payload}") + retried = messages.get(payload["user_message_id"]) + check(not is_soft_deleted_message(retried), "The retried question was created already deleted") + check(retried["metadata"].get("masked") is not True, "The retried question inherited the deletion mask") + check(retried["content"] == "Second wording", "Retry replayed the deleted question") + check(payload["chat_request"]["message"] == "Second wording", "Retry sent the deleted question to the model") + check(client.post("/api/message/q1/retry", json={}).status_code == 404, "A deleted message could be retried") + + @scenario + def edit_does_not_inherit_deletion(): + messages.reset(retried_turn(deleted_first_question=True)) + response = client.post("/api/message/q2/edit", json={"content": "Edited wording"}) + payload = response.get_json() + check(response.status_code == 200, f"Edit failed: {payload}") + edited = messages.get(payload["user_message_id"]) + check(not is_soft_deleted_message(edited), "The edited question was created already deleted") + check(edited["metadata"].get("masked") is not True, "The edited question inherited the deletion mask") + check(edited["content"] == "Edited wording", "The edit lost its content") + response = client.post("/api/message/q1/edit", json={"content": "Bring it back"}) + check(response.status_code == 404, "A deleted message could be edited") + + @scenario + def personal_mask_route_keeps_the_deletion_mask(): + messages.reset([ + message("q0", "user", "Kept question", "2026-01-01T00:00:01"), + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:02")), + ]) + chats_client = chats_app.test_client() + response = chats_client.post( + "/api/message/q9/mask", json={"action": "unmask_message", "conversation_id": CONVERSATION}, + ) + check(response.status_code == 404, f"A deleted message could be unmasked: {response.status_code}") + check(messages.get("q9")["metadata"]["masked"] is True, "The deleted message's mask was cleared") + response = chats_client.post( + "/api/message/q0/mask", json={"action": "mask_all", "conversation_id": CONVERSATION}, + ) + check(response.status_code == 200, f"Masking a live message broke: {response.get_data(as_text=True)}") + check(messages.get("q0")["metadata"]["masked"] is True, "The live message was not masked") + + @scenario + def shared_mask_route_keeps_the_deletion_mask(): + shared_messages.reset() + response = collaboration_app.test_client().post( + f"/api/collaboration/conversations/{SHARED_CONVERSATION}/messages/ghost/mask", + json={"action": "unmask_message"}, + ) + check(response.status_code == 404, f"A deleted shared copy could be unmasked: {response.status_code}") + check(not shared_messages.items, "The deleted shared copy was written") + + @scenario + def forks_do_not_copy_deleted_messages(): + fork_source = [ + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:01", "t9")), + soft_deleted(message("a9", "assistant", "Deleted answer", "2026-01-01T00:00:02", "t9")), + message("q0", "user", "Kept question", "2026-01-01T00:00:03", "t0"), + message("a0", "assistant", "Kept answer", "2026-01-01T00:00:04", "t0"), + ] + forked = operations._collect_fork_documents(fork_source, "a0") + check([item["id"] for item in forked] == ["q0", "a0"], "A fork copied soft-deleted messages") + try: + operations._collect_fork_documents(fork_source, "a9") + except LookupError: + return + raise AssertionError("A fork could start from a deleted message") + + @scenario + def sharing_does_not_copy_deleted_messages(): + source_messages = [ + message("q0", "user", "Kept question", "2026-01-01T00:00:01"), + message("a0", "assistant", "Kept answer", "2026-01-01T00:00:02"), + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:03")), + ] + for copier in ( + collaboration_functions._copy_legacy_personal_messages_to_collaboration, + collaboration_functions._copy_legacy_group_messages_to_collaboration, + ): + shared_messages.reset() + copied = copier("source-1", SHARED_CONVERSATION, owner, raw_messages=copy.deepcopy(source_messages)) + check(len(copied) == 2 and len(shared_messages.items) == 2, f"{copier.__name__} copied deleted messages") + check(not any(is_soft_deleted_message(item) for item in shared_messages.items.values()), "A deleted copy was stored") + + @scenario + def shared_list_hides_existing_deleted_copies(): + shared_messages.reset([ + message("s1", "user", "Shared question", "2026-01-01T00:00:01", conversation=SHARED_CONVERSATION), + ghost, + ]) + listed = collaboration_functions.list_collaboration_messages(SHARED_CONVERSATION) + check([item["id"] for item in listed] == ["s1"], "A shared conversation listed a deleted copy") + + @scenario + def search_skips_deleted_messages(): + searchable = FakeContainer([ + message("q0", "user", "quarterly budget review", "2026-01-01T00:00:01"), + soft_deleted(message("q9", "user", "quarterly budget secret", "2026-01-01T00:00:02")), + ]) + matches = conversations._query_matching_messages( + searchable, "quarterly budget", conversations._normalize_search_match_mode(None), + ) + check([item["id"] for item in matches] == ["q0"], "Search matched a deleted message") + + @scenario + def mcp_skips_deleted_messages(): + messages.reset([ + message("q0", "user", "Kept question", "2026-01-01T00:00:01"), + message("a0", "assistant", "Kept answer", "2026-01-01T00:00:02"), + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:03")), + ]) + mcp_ids = [item["id"] for item in mcp_tools._load_legacy_messages(CONVERSATION)] + check(mcp_ids == ["q0", "a0"], "MCP read a deleted message") + + @scenario + def conversation_summary_skips_deleted_messages(): + messages.reset([ + message("q0", "user", "Kept question", "2026-01-01T00:00:01"), + message("a0", "assistant", "Kept answer", "2026-01-01T00:00:02"), + soft_deleted(message("q9", "user", "Deleted question", "2026-01-01T00:00:03")), + ]) + captured = {} + + def fake_summary(**kwargs): + captured["messages"] = kwargs["messages"] + return {"summary": "ok"} + + with patch.object(conversation_export, "generate_conversation_summary", fake_summary): + response = client.post(f"/api/conversations/{CONVERSATION}/summary", json={}) + check(response.status_code == 200, f"Summary failed: {response.get_data(as_text=True)}") + summary_text = json.dumps(captured["messages"]) + check("Kept question" in summary_text, "The summary lost a live message") + check("Deleted question" not in summary_text, "The summary read a deleted message") + + @scenario + def model_history_skips_deleted_messages(): + prompts = [] + + def create(**kwargs): + prompts.append(json.dumps(kwargs["messages"])) + return SimpleNamespace(choices=[SimpleNamespace(message=SimpleNamespace(content="Earlier: kept turn."))]) + + gpt_client = SimpleNamespace(chat=SimpleNamespace(completions=SimpleNamespace(create=create))) + history = [ + message("h1", "user", "Kept older question", "2026-01-01T00:00:01"), + message("h2", "assistant", "Kept older answer", "2026-01-01T00:00:02"), + soft_deleted(message("h3", "user", "Deleted secret question", "2026-01-01T00:00:03")), + soft_deleted(message("h4", "assistant", "Deleted secret answer", "2026-01-01T00:00:04")), + message("h5", "assistant", "Recent answer", "2026-01-01T00:00:05"), + message("h6", "user", "Current question", "2026-01-01T00:00:06"), + ] + with Flask("soft-delete-history").test_request_context("/"): + segments = chats.build_conversation_history_segments( + history, + 2, + enable_summarize_older_messages=True, + gpt_client=gpt_client, + gpt_model="offline-model", + user_message_id="h6", + fallback_user_message="Current question", + ) + check(prompts and "Kept older question" in prompts[0], "Older messages were not summarized") + check("Deleted secret" not in prompts[0], "A deleted message was summarized for the model") + recent_text = json.dumps(segments["history_messages"]) + check("Deleted secret" not in recent_text, "A deleted message reached the model history") + check("Recent answer" in recent_text and "Current question" in recent_text, "The recent window lost live messages") + check(segments["debug_info"]["stored_total_messages"] == 4, "Deleted messages still counted toward history") + + @scenario + def orchestration_history_skips_deleted_messages(): + deleted_unmasked = {"role": "user", "content": "Deleted", "metadata": {"is_deleted": True, "thread_info": {}}} + check(orchestration_context.normalize_history_message(deleted_unmasked) is None, "Orchestration read a deleted message") + live = {"role": "user", "content": "Live", "metadata": {"thread_info": {}}} + check(orchestration_context.normalize_history_message(live) is not None, "Orchestration dropped a live message") + + @scenario + def recent_replies_skip_deleted_ones(): + messages.reset([ + message("r1", "assistant", "Older live reply", "2026-01-01T00:00:01"), + soft_deleted(message("r2", "assistant", "Newer deleted reply", "2026-01-01T00:00:02")), + ]) + recent = chats._read_recent_assistant_messages(CONVERSATION, 1) + check([item["id"] for item in recent] == ["r1"], "Recent replies included a deleted one or counted it toward TOP") + check(NOT_SOFT_DELETED in messages.queries[-1], "Recent replies are not filtered in the query") + + @scenario + def shared_grounding_skips_a_deleted_request(): + shared_messages.reset([ + message("s1", "user", "Draw the live flow", "2026-01-01T00:00:01", conversation=SHARED_CONVERSATION), + ghost, + ]) + grounding = collaboration_routes._find_collaboration_originating_request( + SHARED_CONVERSATION, {"timestamp": "2026-01-01T00:10:00"}, + ) + check(grounding == "Draw the live flow", f"Grounding used a deleted request: {grounding!r}") + check(NOT_SOFT_DELETED in shared_messages.queries[-1], "Shared grounding is not filtered in the query") + + if failures: + print(PROBE_FAILED) + for failure in failures: + print(f" - {failure}") + raise SystemExit(1) + print(PROBE_OK) + + +# -------------------------------------------------------------------------------------- +# Tests +# -------------------------------------------------------------------------------------- + +def _run_probe_process(optimized=False): + command = [sys.executable] + if optimized: + command.append("-O") + command.extend([str(Path(__file__).resolve()), PROBE_FLAG]) + environment = dict(os.environ, PYTHONIOENCODING="utf-8") + result = subprocess.run( + command, + cwd=str(ROOT), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=900, + env=environment, + ) + if result.returncode != 0 or PROBE_OK not in result.stdout: + failures = result.stdout[result.stdout.find(PROBE_FAILED):] if PROBE_FAILED in result.stdout else "" + raise AssertionError( + f"Probe failed (optimized={optimized}, exit={result.returncode}).\n" + f"{failures or 'STDOUT tail:' + chr(10) + result.stdout[-4000:]}\n" + f"STDERR tail:\n{result.stderr[-6000:]}" + ) + + +def test_soft_deleted_messages_stay_deleted_with_real_modules(): + """Drive the real modules in a fresh process, normally and under python -O.""" + print("Testing soft-deleted message visibility with real modules...") + _run_probe_process(optimized=False) + _run_probe_process(optimized=True) + print("Soft-deleted messages stayed deleted on every path.") + return True + + +def _slice(source, start_marker, end_marker): + start = source.index(start_marker) + return source[start:source.index(end_marker, start)] + + +def test_v2_client_and_personal_grounding_contracts(): + """The V2 store filters deleted messages, attempts count by position, grounding skips deleted.""" + print("Testing V2 client and personal grounding contracts...") + sys.path.insert(0, str(ROOT / "functional_tests")) + from test_support.versioning import assert_app_version_at_least + + assert_app_version_at_least("0.261.257") + + helper = (V2_SRC / "lib" / "deletedMessages.ts").read_text(encoding="utf-8") + check("message?.metadata?.is_deleted === true" in helper, "isDeletedMessage must test is_deleted === true") + check("export function withoutDeletedMessages" in helper, "withoutDeletedMessages is missing") + + store = (V2_SRC / "stores" / "chatStore.ts").read_text(encoding="utf-8") + select_block = _slice(store, "selectConversation: async", "\n },\n") + reload_block = _slice(store, "reloadMessages: async", "\n },\n") + for name, block in (("selectConversation", select_block), ("reloadMessages", reload_block)): + check("withoutDeletedMessages(loadedMessages)" in block, f"{name} does not drop deleted messages") + check("messages: messages ?? []" not in block, f"{name} still stores the unfiltered list") + + threads = (V2_SRC / "lib" / "threads.ts").read_text(encoding="utf-8") + attempt_block = _slice(threads, "export function attemptState", "\n}\n") + check("known.indexOf(current)" in attempt_block, "Attempts must be counted by position in the reported set") + check("current: position + 1" in attempt_block, "The attempt label must use the position, not the attempt number") + check("total: known.length" in attempt_block, "The total must come from the reported set") + + chats_source = (APP_ROOT / "route_backend_chats.py").read_text(encoding="utf-8") + grounding = _slice(chats_source, "def _find_originating_user_request", "def _answer_raced_block_submission") + check("NOT_SOFT_DELETED_COSMOS_FILTER" in grounding, "Personal diagram grounding can read a deleted request") + + conversations_source = (APP_ROOT / "route_backend_conversations.py").read_text(encoding="utf-8") + check("'soft_deleted_message_policy_version': 1" in conversations_source, "Cached search results are not re-keyed") + + legacy = (APP_ROOT / "static" / "js" / "chat" / "chat-messages.js").read_text(encoding="utf-8") + check("msg.metadata.is_deleted === true" in legacy, "The classic client stopped skipping deleted messages") + + print("V2 client and grounding contracts hold.") + return True + + +if __name__ == "__main__": + if PROBE_FLAG in sys.argv: + run_probe() + sys.exit(0) + + tests = [ + test_v2_client_and_personal_grounding_contracts, + test_soft_deleted_messages_stay_deleted_with_real_modules, + ] + results = [] + for test in tests: + print(f"\nRunning {test.__name__}...") + try: + results.append(test()) + except Exception as error: + import traceback + + print(f"Test failed: {error}") + traceback.print_exc() + results.append(False) + + passed = sum(1 for result in results if result) + print(f"\nResults: {passed}/{len(results)} tests passed") + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_content_screening_history.py b/functional_tests/test_content_screening_history.py index a8ad2a99b..4cf2446a8 100644 --- a/functional_tests/test_content_screening_history.py +++ b/functional_tests/test_content_screening_history.py @@ -1,9 +1,10 @@ # test_content_screening_history.py """ Functional regressions for public history and native evidence quarantine. -Version: 0.261.232 +Version: 0.261.257 Implemented in: 0.261.106 Upload-only history checks covered in: 0.261.232 +Soft-deleted message filter seeded in the history route in: 0.261.257 Executes the real history route, artifact hydration, and model-history guard against fake Cosmos. Removed metadata aliases and missing release proofs must @@ -182,6 +183,9 @@ def test_history_api_suppresses_held_attachments_but_keeps_replies_and_citations "cosmos_messages_container": SimpleNamespace(query_items=lambda **kwargs: deepcopy(items)), "build_message_artifact_payload_map": lambda messages: {"artifact-one": {"citation": citation}}, "filter_assistant_artifact_items": lambda messages: messages, + "exclude_soft_deleted_messages": import_app_module( + "functions_message_deletion", + ).exclude_soft_deleted_messages, "hydrate_image_messages": lambda messages, **kwargs: messages, "public_history_messages": access.public_history_messages, "sanitize_saved_analysis_messages": import_app_module("functions_saved_analysis").sanitize_saved_analysis_messages, diff --git a/functional_tests/test_group_collaboration_source_storage_fix.py b/functional_tests/test_group_collaboration_source_storage_fix.py index 8b2872664..8fe9b0283 100644 --- a/functional_tests/test_group_collaboration_source_storage_fix.py +++ b/functional_tests/test_group_collaboration_source_storage_fix.py @@ -2,9 +2,10 @@ #!/usr/bin/env python3 """ Functional regression for group conversation source storage. -Version: 0.261.214 +Version: 0.261.257 Implemented in: 0.261.024 Ported to the React branch in: 0.261.106 +Soft-delete helper seeded in the harness in: 0.261.257 Related issue: microsoft/simplechat#1472 Exercise production conversion and route helpers against isolated Cosmos stores. @@ -74,6 +75,7 @@ # These modules are pure helpers; loading by path avoids application bootstrap. MODEL_HELPERS = runpy.run_path(str(APP_ROOT / 'collaboration_models.py')) MASK_HELPERS = runpy.run_path(str(APP_ROOT / 'functions_message_masking.py')) +DELETION_HELPERS = runpy.run_path(str(APP_ROOT / 'functions_message_deletion.py')) ARTIFACT_HELPERS = load_source_members( str(APP_ROOT / 'functions_message_artifacts.py'), {'is_assistant_artifact_role', 'filter_assistant_artifact_items'}, @@ -355,6 +357,7 @@ def __init__(self, storage='regular', history=True): 'logging': logging, 'CosmosResourceNotFoundError': FakeCosmosResourceNotFoundError, 'filter_assistant_artifact_items': ARTIFACT_HELPERS['filter_assistant_artifact_items'], + 'exclude_soft_deleted_messages': DELETION_HELPERS['exclude_soft_deleted_messages'], 'invalidate_conversation_cache_for_item': self.invalidate, 'log_event': lambda *args, **kwargs: self.logs.append((args, kwargs)), 'log_conversation_archival': lambda **kwargs: None, diff --git a/functional_tests/test_v2_message_actions.py b/functional_tests/test_v2_message_actions.py index 9ef8f871e..60efe7206 100644 --- a/functional_tests/test_v2_message_actions.py +++ b/functional_tests/test_v2_message_actions.py @@ -151,8 +151,8 @@ def test_delete_sends_its_option_in_the_body(): remove_start = store.index("removeMessage: async") remove_block = store[remove_start : store.index("retryMessage: async", remove_start)] assert "reloadMessages()" in remove_block, ( - "Deletion is soft when archiving is enabled, so the list must be re-read rather " - "than trusting the optimistic removal" + "Deleting a question removes its whole turn, every attempt included, so the list " + "must be re-read rather than trusting the optimistic removal" ) print("Delete request shape test passed!") diff --git a/ui_tests/test_v2_chat_deleted_messages.py b/ui_tests/test_v2_chat_deleted_messages.py new file mode 100644 index 000000000..674112f95 --- /dev/null +++ b/ui_tests/test_v2_chat_deleted_messages.py @@ -0,0 +1,281 @@ +# test_v2_chat_deleted_messages.py +""" +UI test for deleted messages in the V2 chat. +Version: 0.261.257 +Implemented in: 0.261.257 + +With conversation archiving enabled, deleting a message keeps its stored document with +`metadata.is_deleted` set and masks it as a fail-safe. The V2 chat rendered those documents +as "This message is masked" both when a conversation opened and in the re-read that follows a +delete (#1649). This test ensures that: + +- a deleted message never renders, on load or after the re-read that follows deleting one; +- deleting an answer sends `delete_thread: false`, and the answer stays gone afterwards; +- the attempt control counts the attempts that remain by position ("2/2"), rather than by + attempt number ("3/2") once a deleted attempt is left out of the reported set. + +The production MessageList, Composer and chat store run in Chromium with production CSS. Only +HTTP is stubbed, and the stub answers `/api/get_messages` the way the server did before the +fix, deleted messages included, so the test proves the client never renders one on its own. + +Build CSS with the existing V2 build, keeping outputs in UI test artifacts: +npm --prefix .\\application\\v2_ui run build -- --outDir ..\\..\\ui_tests\\artifacts\\orchestration-plan-editor +Run: python -m pytest .\\ui_tests\\test_v2_chat_deleted_messages.py -q +""" + +import copy +import time +from urllib.parse import unquote, urlsplit + +import pytest +from playwright.sync_api import expect + +import test_v2_orchestration_plan_editor as editor_tests +from test_v2_orchestration_plan_editor import ( # noqa: F401 + connect_options, + editor_assets, + editor_browser, +) + + +pytestmark = pytest.mark.ui +CHAT = "budget-review" +TITLE = "Budget review" +MASKED_NOTICE = "This message is masked" +TRAVEL_QUESTION = "What is the travel budget?" +TRAVEL_ANSWER = "The travel budget is 12,000." +DELETED_QUESTION = "Share the old pricing sheet." +DELETED_ANSWER = "Here is the old pricing sheet." +TRAINING_QUESTION = "And the training budget?" +TRAINING_ANSWER = "The training budget is 4,000." + + +def message(message_id, role, content, thread=None, attempt=1, deleted=False): + metadata = {} + if thread: + metadata["thread_info"] = {"thread_id": thread, "thread_attempt": attempt, "active_thread": True} + if deleted: + soft_delete(metadata) + return {"id": message_id, "conversation_id": CHAT, "role": role, "content": content, "metadata": metadata} + + +def soft_delete(metadata): + """What the delete route stores when archiving is enabled.""" + metadata.update({ + "is_deleted": True, + "deleted_by_user_id": "tester", + "masked": True, + "masked_by_user_id": "tester", + }) + + +class DeletedMessagesApi: + """Serves the harness and answers the chat's reads; anything else fails the test.""" + + def __init__(self, assets): + self.assets = assets + self.errors = [] + self.reads = 0 + self.deletes = [] + self.switches = [] + self.messages = [] + self.attempts = {} + self.active_attempt = None + + def visible_messages(self): + """The conversation as the server returned it before the fix: deleted messages included.""" + messages = list(self.messages) + if self.attempts: + messages.extend(self.attempts[self.active_attempt]) + return copy.deepcopy(messages) + + def handle(self, route): + request = route.request + path = urlsplit(request.url).path + if request.method == "GET" and path in self.assets: + route.fulfill(path=str(self.assets[path])) + return + if path == "/favicon.ico": + route.fulfill(status=204) + return + if path == "/api/get_messages": + self.reads += 1 + route.fulfill(json={"messages": self.visible_messages()}) + return + if request.method == "DELETE" and path.startswith("/api/message/"): + message_id = unquote(path.rsplit("/", 1)[-1]) + self.deletes.append((message_id, request.post_data_json or {})) + for stored in self.messages: + if stored["id"] == message_id: + soft_delete(stored["metadata"]) + route.fulfill(json={"success": True, "deleted_message_ids": [message_id], "archived": True}) + return + if request.method == "POST" and path.endswith("/switch-attempt"): + direction = (request.post_data_json or {}).get("direction") + available = sorted(self.attempts) + step = -1 if direction == "prev" else 1 + self.active_attempt = available[(available.index(self.active_attempt) + step) % len(available)] + self.switches.append(direction) + route.fulfill(json={ + "success": True, "target_attempt": self.active_attempt, "available_attempts": available, + }) + return + if path.startswith("/api/conversations/") and path.endswith("/metadata"): + route.fulfill(json={"conversation_id": CHAT, "title": TITLE}) + return + if path.startswith("/api/chat/stream/status/"): + route.fulfill(json={"active": False, "pending": False, "reattachable": False}) + return + if path == "/api/v2/orchestration/runs": + route.fulfill(json={"runs": []}) + return + if path == "/api/collaboration/file-approvals": + route.fulfill(json={"approvals": []}) + return + if path == "/api/user/settings": + route.fulfill(json={"settings": {}}) + return + if path == "/api/user/collaboration-suggestions": + route.fulfill(json={"results": []}) + return + self.errors.append(f"Unexpected request: {request.method} {path}") + route.fulfill(status=404, json={"error": "Unexpected test request."}) + + +@pytest.fixture +def chat_ui(editor_browser, editor_assets): + context = editor_browser.new_context(viewport={"width": 1280, "height": 900}) + page = context.new_page() + api = DeletedMessagesApi(editor_assets) + page.route("**/*", api.handle) + page.on("pageerror", lambda error: api.errors.append(str(error))) + page.on("console", lambda entry: api.errors.append(entry.text) if entry.type == "error" else None) + try: + yield page, api + finally: + context.close() + assert not api.errors, api.errors + + +def open_conversation(page, api): + """Mount the real message list and composer, then open the conversation through the store.""" + page.goto(editor_tests.ORIGIN + editor_tests.HARNESS) + page.wait_for_function("() => Boolean(window.OrchHarness)") + for url in api.assets: + if url.endswith(".css"): + page.add_style_tag(url=editor_tests.ORIGIN + url) + page.evaluate( + """(spec) => { + const H = window.OrchHarness; + H.reset(); + H.stores.bootstrap.useBootstrapStore.setState({data: { + features: {enable_chat_orchestration: false}, + settings: {}, user: {id: 'tester', display_name: 'Tester'}, + catalogs: {models: [], agents: [], prompts: []}, + scope: {groups: [], public_workspaces: []}, + orchestration: {enabled: false, capabilities: [], default_approval_mode: 'manual'}, + }}); + H.stores.chat.useChatStore.setState({ + activeConversationId: null, activeConversationKind: null, messages: [], + messagesLoading: false, messagesError: null, streaming: false, + streamError: null, streamingContent: '', thoughts: [], attemptsByThread: {}, + conversations: [{id: spec.chat, title: spec.title}], + }); + H.mount('mount-a', 'PromptExperience', {}, {initialEntries: ['/chat']}); + }""", + {"chat": CHAT, "title": TITLE}, + ) + page.evaluate("(id) => window.OrchHarness.stores.chat.useChatStore.getState().selectConversation(id)", CHAT) + + +def stored_message_ids(page): + return page.evaluate( + "() => window.OrchHarness.stores.chat.useChatStore.getState().messages.map((item) => item.id)" + ) + + +def wait_until(page, predicate, description, timeout=10): + """Let Playwright dispatch routes and renders until a Python-side condition holds.""" + deadline = time.monotonic() + timeout + while not predicate(): + if time.monotonic() > deadline: + raise AssertionError(f"Timed out waiting for {description}") + page.wait_for_timeout(50) + + +def open_actions(page, message_id): + row = page.locator(f"#message-{message_id}") + row.hover() + row.get_by_role("button", name="More actions", exact=True).click() + return row + + +def test_deleted_messages_never_render_on_load_or_after_a_delete(chat_ui): + """A deleted message is never shown, not even as a masked one.""" + page, api = chat_ui + api.messages = [ + message("q0", "user", TRAVEL_QUESTION), + message("a0", "assistant", TRAVEL_ANSWER), + message("q9", "user", DELETED_QUESTION, deleted=True), + message("a9", "assistant", DELETED_ANSWER, deleted=True), + message("q1", "user", TRAINING_QUESTION), + message("a1", "assistant", TRAINING_ANSWER), + ] + + open_conversation(page, api) + + expect(page.get_by_text(TRAINING_ANSWER, exact=True)).to_be_visible() + expect(page.get_by_text(TRAVEL_QUESTION, exact=True)).to_be_visible() + expect(page.get_by_text(MASKED_NOTICE)).to_have_count(0) + expect(page.get_by_text(DELETED_QUESTION)).to_have_count(0) + expect(page.get_by_text(DELETED_ANSWER)).to_have_count(0) + assert stored_message_ids(page) == ["q0", "a0", "q1", "a1"] + + row = open_actions(page, "a1") + row.get_by_role("button", name="Delete", exact=True).click() + + # The delete is followed by a re-read, which comes back with the answer marked deleted. + wait_until(page, lambda: api.reads >= 2, "the re-read after the delete") + wait_until( + page, + lambda: page.evaluate("() => window.OrchHarness.stores.chat.useChatStore.getState().metadata === null"), + "the re-read to finish", + ) + assert api.deletes == [("a1", {"delete_thread": False})] + expect(page.get_by_text(TRAINING_QUESTION, exact=True)).to_be_visible() + expect(page.get_by_text(TRAINING_ANSWER)).to_have_count(0) + expect(page.get_by_text(MASKED_NOTICE)).to_have_count(0) + assert stored_message_ids(page) == ["q0", "a0", "q1"] + + +def test_attempt_control_counts_remaining_attempts_by_position(chat_ui): + """With attempt 2 deleted, attempts 1 and 3 read as 1/2 and 2/2, never 3/2.""" + page, api = chat_ui + api.attempts = { + 1: [ + message("q1", "user", "Summarise the budget.", thread="t1", attempt=1), + message("a1", "assistant", "First summary of the budget.", thread="t1", attempt=1), + ], + 3: [ + message("q3", "user", "Summarise the budget.", thread="t1", attempt=3), + message("a3", "assistant", "Third summary of the budget.", thread="t1", attempt=3), + ], + } + api.active_attempt = 3 + + open_conversation(page, api) + expect(page.get_by_text("Third summary of the budget.", exact=True)).to_be_visible() + + page.locator("#message-a3").hover() + page.locator("#message-a3").get_by_role("button", name="Next attempt", exact=True).click() + expect(page.get_by_text("First summary of the budget.", exact=True)).to_be_visible() + first = page.locator("#message-a1").get_by_title("Attempt 1 of 2", exact=True) + expect(first).to_have_text("1/2") + + page.locator("#message-a1").hover() + page.locator("#message-a1").get_by_role("button", name="Next attempt", exact=True).click() + expect(page.get_by_text("Third summary of the budget.", exact=True)).to_be_visible() + third = page.locator("#message-a3").get_by_title("Attempt 2 of 2", exact=True) + expect(third).to_have_text("2/2") + expect(page.get_by_text("3/2", exact=True)).to_have_count(0) + assert api.switches == ["next", "next"]