From 0efdf495311e2d7ab456d12cc787b0317ef1b88f Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Thu, 30 Jul 2026 14:19:43 -0400 Subject: [PATCH] Fix conversation fork workspace context Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../functions_simplechat_operations.py | 108 ++++++++++--- .../single_app/route_backend_conversations.py | 8 +- .../static/js/chat/chat-messages.js | 15 +- .../explanation/features/FORK_CONVERSATION.md | 51 +++--- .../fixes/CONVERSATION_FORK_HTTP_500_FIX.md | 71 +++++++++ docs/explanation/release_notes.md | 10 ++ functional_tests/test_conversation_fork.py | 146 ++++++++++++++++-- ...versations_read_ownership_authorization.py | 73 ++++++++- ui_tests/test_chat_conversation_fork.py | 62 +++++++- 10 files changed, 476 insertions(+), 70 deletions(-) create mode 100644 docs/explanation/fixes/CONVERSATION_FORK_HTTP_500_FIX.md diff --git a/application/single_app/config.py b/application/single_app/config.py index d3e416629..86e9a5c32 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -95,7 +95,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.100" +VERSION = "0.250.101" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index 0f66231c7..0210b3133 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -61,13 +61,24 @@ ) from functions_notifications import create_notification from functions_personal_workflows import save_personal_workflow +from functions_public_workspaces import ( + check_public_workspace_status_allows_operation, + find_public_workspace_by_id, +) from functions_settings import get_settings, is_user_workflows_enabled_for_user from utils_cache import invalidate_group_search_cache, invalidate_personal_search_cache SIMPLECHAT_PLUGIN_TYPE = "simplechat" SIMPLECHAT_DEFAULT_ENDPOINT = "simplechat://internal" -PERSONAL_FORK_CHAT_TYPES = {"", "new", "personal", "personal_single_user"} +FORKABLE_SINGLE_USER_CHAT_TYPES = { + "", + "new", + "personal", + "personal_single_user", + "group-single-user", + "public", +} PERSONAL_FORK_CONVERSATION_FIELDS = ( "context", "tags", @@ -244,20 +255,77 @@ class ConversationForkConflictError(RuntimeError): """Raised when the source changes or cannot be forked safely.""" -def is_forkable_personal_conversation(conversation_item: Dict[str, Any]) -> bool: - """Return whether a legacy personal conversation is eligible for forking.""" +def is_forkable_single_user_conversation(conversation_item: Dict[str, Any]) -> bool: + """Return whether a user-owned single-user conversation can be considered for forking.""" chat_type = str((conversation_item or {}).get("chat_type") or "").strip().lower() - if chat_type not in PERSONAL_FORK_CHAT_TYPES: - return False + return chat_type in FORKABLE_SINGLE_USER_CHAT_TYPES + + +def _authorize_fork_conversation_context( + conversation_item: Dict[str, Any], + user_id: str, +) -> str: + """Revalidate stored workspace context and return the destination chat type.""" + if not is_forkable_single_user_conversation(conversation_item): + raise ConversationForkConflictError("Only supported single-user conversations can be forked") - for context_item in (conversation_item or {}).get("context") or []: + authorized_scopes = set() + for context_item in conversation_item.get("context") or []: if not isinstance(context_item, dict): - continue + raise ConversationForkConflictError("The conversation context is invalid") + scope = str(context_item.get("scope") or "").strip().lower() - if scope and scope != "personal": - return False + if scope in {"", "personal"}: + authorized_scopes.add("personal") + continue - return True + context_id = str(context_item.get("id") or "").strip() + if not context_id: + raise ConversationForkConflictError("The conversation workspace context is incomplete") + + if scope == "group": + group_doc = find_group_by_id(context_id) + if not group_doc: + raise ConversationForkConflictError("The conversation group is no longer available") + allowed, _ = check_group_status_allows_operation(group_doc, "chat") + if not allowed: + raise ConversationForkConflictError("The conversation group no longer allows chat") + try: + assert_group_role( + user_id, + context_id, + allowed_roles=("Owner", "Admin", "DocumentManager", "User"), + ) + except (LookupError, PermissionError) as access_error: + raise ConversationForkConflictError( + "The conversation group is no longer available to this user" + ) from access_error + authorized_scopes.add(scope) + continue + + if scope == "public": + workspace_doc = find_public_workspace_by_id(context_id) + if not workspace_doc: + raise ConversationForkConflictError("The public workspace is no longer available") + allowed, _ = check_public_workspace_status_allows_operation(workspace_doc, "chat") + if not allowed: + raise ConversationForkConflictError("The public workspace no longer allows chat") + authorized_scopes.add(scope) + continue + + raise ConversationForkConflictError("The conversation has an unsupported workspace context") + + if "group" in authorized_scopes and "public" in authorized_scopes: + raise ConversationForkConflictError("Mixed group and public workspace context cannot be forked") + if "group" in authorized_scopes: + return "group-single-user" + if "public" in authorized_scopes: + return "public" + + source_chat_type = str(conversation_item.get("chat_type") or "").strip().lower() + if source_chat_type in {"group-single-user", "public"}: + raise ConversationForkConflictError("The conversation workspace context is incomplete") + return "personal_single_user" def _message_fork_sort_key(message_doc: Dict[str, Any]) -> Tuple[str, int, str]: @@ -462,6 +530,7 @@ def _build_fork_conversation_document( selected_message_id: str, fork_conversation_id: str, user_id: str, + destination_chat_type: str, ) -> Dict[str, Any]: source_title = str(source_conversation.get("title") or "New Conversation").strip() or "New Conversation" fork_title = f"Fork of {source_title}" @@ -476,7 +545,7 @@ def _build_fork_conversation_document( "strict": False, "is_pinned": False, "is_hidden": False, - "chat_type": "personal_single_user", + "chat_type": destination_chat_type, "has_unread_assistant_response": False, "last_unread_assistant_message_id": None, "last_unread_assistant_at": None, @@ -602,7 +671,7 @@ def _cleanup_failed_fork( log_event( f"[ConversationFork] Failed to clean up fork message: {cleanup_error}", level=logging.ERROR, - properties={ + extra={ "fork_conversation_id": fork_conversation_id, "message_id": message_id, }, @@ -620,7 +689,7 @@ def _cleanup_failed_fork( log_event( f"[ConversationFork] Failed to clean up fork blob: {cleanup_error}", level=logging.ERROR, - properties={ + extra={ "fork_conversation_id": fork_conversation_id, "blob_container": blob_container, "blob_path": blob_path, @@ -635,13 +704,13 @@ def _cleanup_failed_fork( log_event( "[ConversationFork] Fork conversation already absent during cleanup", level=logging.INFO, - properties={"fork_conversation_id": fork_conversation_id}, + extra={"fork_conversation_id": fork_conversation_id}, ) except Exception as cleanup_error: log_event( f"[ConversationFork] Failed to clean up fork conversation: {cleanup_error}", level=logging.ERROR, - properties={"fork_conversation_id": fork_conversation_id}, + extra={"fork_conversation_id": fork_conversation_id}, ) @@ -658,8 +727,10 @@ def fork_personal_conversation_for_user( raise ValueError("Source conversation, assistant message, and user are required") if source_conversation.get("user_id") != normalized_user_id: raise PermissionError("Forbidden") - if not is_forkable_personal_conversation(source_conversation): - raise ConversationForkConflictError("Only personal conversations can be forked") + destination_chat_type = _authorize_fork_conversation_context( + source_conversation, + normalized_user_id, + ) query = ( "SELECT * FROM c " @@ -711,6 +782,7 @@ def fork_personal_conversation_for_user( normalized_message_id, fork_conversation_id, normalized_user_id, + destination_chat_type, ) written_message_ids = [] created_blob_targets: List[Tuple[str, str]] = [] @@ -746,7 +818,7 @@ def fork_personal_conversation_for_user( log_event( f"[ConversationFork] Fork created but activity logging failed: {log_error}", level=logging.WARNING, - properties={"fork_conversation_id": fork_conversation_id}, + extra={"fork_conversation_id": fork_conversation_id}, ) return { diff --git a/application/single_app/route_backend_conversations.py b/application/single_app/route_backend_conversations.py index 4aeeb3676..edcb42f3a 100644 --- a/application/single_app/route_backend_conversations.py +++ b/application/single_app/route_backend_conversations.py @@ -1176,7 +1176,7 @@ def fork_conversation(conversation_id): log_event( f'[ConversationFork] Fork created but cache invalidation failed: {cache_error}', level=logging.WARNING, - custom_dimensions={ + extra={ 'fork_conversation_id': fork_conversation['id'], 'user_id': user_id, }, @@ -1193,7 +1193,7 @@ def fork_conversation(conversation_id): f'[ConversationFork] Validation failed while creating conversation fork: {validation_error}', level=logging.WARNING, exceptionTraceback=True, - custom_dimensions={ + extra={ 'source_conversation_id': conversation_id, 'selected_message_id': selected_message_id, 'user_id': user_id, @@ -1204,7 +1204,7 @@ def fork_conversation(conversation_id): log_event( f'[ConversationFork] Conflict while creating conversation fork: {conflict_error}', level=logging.WARNING, - custom_dimensions={ + extra={ 'source_conversation_id': conversation_id, 'selected_message_id': selected_message_id, 'user_id': user_id, @@ -1216,7 +1216,7 @@ def fork_conversation(conversation_id): f'[ConversationFork] Failed to create conversation fork: {error}', level=logging.ERROR, exceptionTraceback=True, - custom_dimensions={ + extra={ 'source_conversation_id': conversation_id, 'selected_message_id': selected_message_id, 'user_id': user_id, diff --git a/application/single_app/static/js/chat/chat-messages.js b/application/single_app/static/js/chat/chat-messages.js index a0ecd2e8f..1e6822c71 100644 --- a/application/single_app/static/js/chat/chat-messages.js +++ b/application/single_app/static/js/chat/chat-messages.js @@ -3153,7 +3153,7 @@ function renderReplyQuoteHtml(fullMessageObject = null) { return !isStreamingAssistantPlaceholder(messageId, fullMessageObject); } - function getForkablePersonalConversationId(fullMessageObject = null) { + function getForkableSingleUserConversationId(fullMessageObject = null) { const conversationId = resolveMessageConversationId(fullMessageObject); const activeConversationId = String( window.chatConversations?.getCurrentConversationId?.() @@ -3173,7 +3173,14 @@ function renderReplyQuoteHtml(fullMessageObject = null) { } const chatType = String(conversationItem.dataset.chatType || '').trim().toLowerCase(); - return ['', 'new', 'personal', 'personal_single_user'].includes(chatType) + return [ + '', + 'new', + 'personal', + 'personal_single_user', + 'group-single-user', + 'public', + ].includes(chatType) ? conversationId : ''; } @@ -3185,7 +3192,7 @@ function renderReplyQuoteHtml(fullMessageObject = null) { normalizedMessageId && persistedMessageId === normalizedMessageId && shouldRenderCompletedAssistantActions(messageId, fullMessageObject) - && getForkablePersonalConversationId(fullMessageObject) + && getForkableSingleUserConversationId(fullMessageObject) ); } @@ -3204,7 +3211,7 @@ function renderReplyQuoteHtml(fullMessageObject = null) { } const messageId = String(messageDiv?.dataset?.messageId || '').trim(); - const conversationId = getForkablePersonalConversationId(fullMessageObject); + const conversationId = getForkableSingleUserConversationId(fullMessageObject); if (!messageId || !conversationId) { showToast('This message is not available to fork.', 'warning'); return; diff --git a/docs/explanation/features/FORK_CONVERSATION.md b/docs/explanation/features/FORK_CONVERSATION.md index 397d99ab5..f60bd8222 100644 --- a/docs/explanation/features/FORK_CONVERSATION.md +++ b/docs/explanation/features/FORK_CONVERSATION.md @@ -3,11 +3,13 @@ ## Overview **Implemented in version: 0.250.074** +**Workspace-context support fixed in version: 0.250.101** -Fork Conversation lets a user branch a personal conversation from a persisted -assistant response without changing the source conversation. The fork contains -the active message history from the beginning through the selected assistant -message, inclusive, and opens immediately as an independent conversation. +Fork Conversation lets a user branch an owned single-user conversation from a +persisted assistant response without changing the source conversation. The fork +contains the active message history from the beginning through the selected +assistant message, inclusive, and opens immediately as an independent +conversation. ### Purpose @@ -28,7 +30,7 @@ message, inclusive, and opens immediately as an independent conversation. ### Architecture 1. The assistant message action is rendered only for a persisted, completed - assistant message in an active personal conversation. + assistant message in a supported active single-user conversation. 2. A Bootstrap confirmation modal submits the source conversation ID and selected message ID. 3. The backend authorizes ownership and resolves the authoritative persisted @@ -72,15 +74,19 @@ The endpoint can return: - `400` for a missing message ID or a non-assistant fork point. - `403` when the current user does not own the source conversation. - `404` when the selected message is not on the active persisted timeline. -- `409` when the source is not an eligible personal conversation or changes - during the operation. +- `409` when the source is not an eligible single-user conversation, its + workspace context is no longer authorized, or it changes during the + operation. - `500` when destination persistence fails. ### Security and authorization - Source ownership is revalidated at the API boundary. -- Group, public, and collaborative conversation types are rejected. -- Stored context is eligible only when it remains personal to the same owner. +- Collaborative, multi-user, workflow, and unknown conversation types are + rejected. +- Personal, group-single-user, and public context is copied only after the + current user's group role or public workspace access and chat status are + revalidated. - The selected message must belong to the authorized source conversation and must be an active assistant message. - Source conversation and message snapshots are checked again before writes to @@ -90,11 +96,12 @@ The endpoint can return: ### Preserved and reset state -The fork preserves personal context, tags, strict mode, classification, scope -lock state, supported model/agent selections, message content, attachments, -citations, and assistant artifact metadata. It assigns new conversation, -message, artifact, and thread identifiers, and copies blob-backed message files -to independent destination paths. +The fork preserves authorized personal, group, or public context, its normalized +single-user chat type, tags, strict mode, classification, scope lock state, +supported model/agent selections, message content, attachments, citations, and +assistant artifact metadata. It assigns new conversation, message, artifact, +and thread identifiers, and copies blob-backed message files to independent +destination paths. Pin, hidden, unread, Cosmos system, streaming, and other destination lifecycle state is reset. The source conversation and all later messages remain @@ -111,14 +118,15 @@ unchanged. ## Usage -1. Open a personal conversation containing a completed assistant response. +1. Open an owned personal, group-single-user, or public single-user conversation + containing a completed assistant response. 2. Open the response's **More actions** menu. 3. Select **Fork conversation**. 4. Confirm the operation. 5. Continue in the newly opened `Fork of ` conversation. The action is unavailable for streaming or transient assistant messages and for -group, public, or collaborative conversations. +collaborative, multi-user, workflow, or unknown conversation types. ## Testing and Validation @@ -129,8 +137,11 @@ group, public, or collaborative conversations. - Independent and remapped message, artifact, reply, conversation, and thread identifiers. - Source immutability. -- Missing, non-assistant, inactive, foreign, and non-personal fork requests. +- Missing, non-assistant, inactive, foreign, and unsupported fork requests. +- Authorized group/public context and stale, inactive, or inaccessible + workspace context. - Concurrent source changes and failed-write cleanup. +- Route-level conflict logging that preserves the intended HTTP 409 response. - Browser action visibility, confirmation and cancellation, failure feedback, duplicate-click prevention, list insertion, and fork activation. @@ -142,8 +153,8 @@ conversation is not listed until all destination message writes complete. ### Limitations -- Only personal, single-user conversations are supported. -- Group, public, and collaborative conversations require separate resource and - participant authorization designs. +- Only owned single-user conversations are supported. +- Collaborative and multi-user conversations require separate participant + authorization and destination-membership designs. - Cross-container atomic transactions are not available. The implementation uses publish-last behavior plus cleanup to provide an atomic user experience. diff --git a/docs/explanation/fixes/CONVERSATION_FORK_HTTP_500_FIX.md b/docs/explanation/fixes/CONVERSATION_FORK_HTTP_500_FIX.md new file mode 100644 index 000000000..cf74eab01 --- /dev/null +++ b/docs/explanation/fixes/CONVERSATION_FORK_HTTP_500_FIX.md @@ -0,0 +1,71 @@ +# Conversation Fork HTTP 500 Fix + +## Issue + +Forking an owned conversation with group or public workspace context returned +HTTP 500 instead of creating the fork or reporting an eligibility conflict. + +**Fixed in version: 0.250.101** + +## Root Cause + +The fork operation allowed only personal context, even though SimpleChat stores +owned group- and public-grounded chats as single-user conversations. When that +eligibility check raised `ConversationForkConflictError`, the route attempted to +log it with the unsupported `custom_dimensions` argument. `log_event()` raised +`TypeError`, masking the intended HTTP 409 response as HTTP 500. + +Additional fork cleanup and activity log calls used the similarly unsupported +`properties` argument and could mask persistence failures. + +## Technical Details + +### Files Modified + +- `application/single_app/functions_simplechat_operations.py` +- `application/single_app/route_backend_conversations.py` +- `application/single_app/static/js/chat/chat-messages.js` +- `functional_tests/test_conversation_fork.py` +- `functional_tests/test_conversations_read_ownership_authorization.py` +- `ui_tests/test_chat_conversation_fork.py` +- `docs/explanation/features/FORK_CONVERSATION.md` +- `application/single_app/config.py` + +### Code Changes + +- Replaced invalid fork logging keyword arguments with `extra`, the structured + context parameter supported by `log_event()`. +- Added group and public workspace context revalidation before any destination + message, blob, or conversation is written. +- Continued to require exact source ownership and reject collaborative, + multi-user, workflow, mixed-scope, and unknown conversation types. +- Preserved the authorized group/public context and normalized destination chat + type. +- Aligned the browser Fork action with the supported backend conversation types. + +### Impact + +Owned single-user conversations grounded in an active group or public workspace +can now be forked when the current user still has access. Stale or unauthorized +workspace context returns a controlled conflict without creating partial data. +Expected validation and conflict errors can no longer be converted into HTTP +500 by fork-specific logging. + +## Validation + +### Coverage + +- Personal, group-single-user, and public fork success paths. +- Missing, inactive, and unauthorized workspace context. +- Unsupported multi-user conversation rejection. +- Route-level conflict logging and HTTP 409 response behavior. +- Browser action visibility for supported and unsupported conversation types. +- Existing message ordering, source immutability, artifact remapping, blob copy, + concurrency, and failed-write cleanup behavior. + +### Before and After + +Before the fix, an expected workspace eligibility conflict triggered a logging +`TypeError` and returned HTTP 500. After the fix, authorized workspace-grounded +conversations fork successfully, while genuine conflicts return HTTP 409 with no +partial destination records. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 4dea97247..f38dcadba 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,16 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.101)** + +#### Bug Fixes + +* **Conversation Fork Workspace Context and HTTP 500 Fix** + * Fixed conversation forks returning HTTP 500 when an owned single-user conversation used group or public workspace knowledge. + * Forking now revalidates current workspace access, preserves the authorized context and chat type, and returns controlled conflicts when access is stale or unavailable. + * Corrected fork-specific structured logging so validation, conflict, cleanup, and cache errors retain their intended response behavior. + * (Ref: microsoft/simplechat#1025, `functions_simplechat_operations.py`, `route_backend_conversations.py`, `chat-messages.js`, `CONVERSATION_FORK_HTTP_500_FIX.md`) + ### **(v0.250.100)** #### Bug Fixes diff --git a/functional_tests/test_conversation_fork.py b/functional_tests/test_conversation_fork.py index 052ff9a26..306de33d8 100644 --- a/functional_tests/test_conversation_fork.py +++ b/functional_tests/test_conversation_fork.py @@ -2,12 +2,12 @@ # test_conversation_fork.py """ Functional test for personal conversation forking. -Version: 0.250.074 +Version: 0.250.101 Implemented in: 0.250.074 This test ensures persisted personal conversation history is copied through an assistant boundary with independent identifiers, deterministic ordering, -authorization, concurrency protection, and failed-write cleanup. +workspace-context authorization, concurrency protection, and failed-write cleanup. """ import copy @@ -132,6 +132,13 @@ def load_operations_module_for_test(): 'functions_personal_workflows', {'save_personal_workflow': no_op}, ), + 'functions_public_workspaces': _stub_module( + 'functions_public_workspaces', + { + 'check_public_workspace_status_allows_operation': no_op, + 'find_public_workspace_by_id': no_op, + }, + ), 'functions_settings': _stub_module( 'functions_settings', { @@ -478,6 +485,7 @@ def run_fork( source_messages=None, user_id='owner-user', selected_message_id='message-003-target', + access_replacements=None, **message_container_options, ): """Execute the fork helper with isolated fake storage.""" @@ -490,16 +498,16 @@ def run_fork( message_container = FakeMessageContainer(messages, **message_container_options) blob_service = FakeBlobService() - with PatchSet( - operations_module, - { - 'cosmos_conversations_container': conversation_container, - 'cosmos_messages_container': message_container, - 'CLIENTS': {'storage_account_office_docs_client': blob_service}, - 'log_conversation_creation': lambda **kwargs: None, - 'log_event': lambda *args, **kwargs: None, - }, - ): + replacements = { + 'cosmos_conversations_container': conversation_container, + 'cosmos_messages_container': message_container, + 'CLIENTS': {'storage_account_office_docs_client': blob_service}, + 'log_conversation_creation': lambda **kwargs: None, + 'log_event': lambda *args, **kwargs: None, + } + replacements.update(access_replacements or {}) + + with PatchSet(operations_module, replacements): result = operations_module.fork_personal_conversation_for_user( source_conversation=conversation, selected_message_id=selected_message_id, @@ -610,18 +618,126 @@ def test_fork_rejects_invalid_message_boundaries(selected_message_id, expected_e run_fork(selected_message_id=selected_message_id) -def test_fork_rejects_foreign_and_non_personal_conversations(): - """Require source ownership and personal-only context.""" +def test_fork_rejects_foreign_and_unsupported_conversations(): + """Require source ownership and reject multi-user conversation kinds.""" with pytest.raises(PermissionError): run_fork(user_id='different-user') - group_conversation = build_source_conversation(chat_type='group-single-user') + group_conversation = build_source_conversation(chat_type='group_multi_user') group_conversation['context'] = [{'type': 'primary', 'scope': 'group', 'id': 'group-1'}] operations_module = load_operations_module_for_test() with pytest.raises(operations_module.ConversationForkConflictError): run_fork(source_conversation=group_conversation) +@pytest.mark.parametrize( + ('chat_type', 'context', 'expected_chat_type', 'access_replacements'), + [ + ( + 'group-single-user', + {'type': 'primary', 'scope': 'group', 'id': 'group-1'}, + 'group-single-user', + { + 'find_group_by_id': lambda group_id: {'id': group_id, 'status': 'active'}, + 'check_group_status_allows_operation': lambda group, operation: (True, ''), + 'assert_group_role': lambda *args, **kwargs: 'User', + }, + ), + ( + 'public', + {'type': 'primary', 'scope': 'public', 'id': 'public-1'}, + 'public', + { + 'find_public_workspace_by_id': lambda workspace_id: { + 'id': workspace_id, + 'status': 'active', + }, + 'check_public_workspace_status_allows_operation': ( + lambda workspace, operation: (True, '') + ), + }, + ), + ], +) +def test_fork_revalidates_and_preserves_workspace_context( + chat_type, + context, + expected_chat_type, + access_replacements, +): + """Fork supported workspace-grounded conversations after access checks.""" + conversation = build_source_conversation(chat_type=chat_type) + conversation['context'] = [context] + + test_state = run_fork( + source_conversation=conversation, + access_replacements=access_replacements, + ) + + assert test_state['result']['conversation']['chat_type'] == expected_chat_type + assert test_state['result']['conversation']['context'] == [context] + + +@pytest.mark.parametrize( + ('conversation', 'access_replacements', 'error_match'), + [ + ( + { + **build_source_conversation(chat_type='group-single-user'), + 'context': [{'type': 'primary', 'scope': 'group', 'id': 'missing-group'}], + }, + {'find_group_by_id': lambda group_id: None}, + 'group is no longer available', + ), + ( + { + **build_source_conversation(chat_type='group-single-user'), + 'context': [{'type': 'primary', 'scope': 'group', 'id': 'inactive-group'}], + }, + { + 'find_group_by_id': lambda group_id: {'id': group_id, 'status': 'inactive'}, + 'check_group_status_allows_operation': lambda group, operation: (False, 'inactive'), + }, + 'group no longer allows chat', + ), + ( + { + **build_source_conversation(chat_type='group-single-user'), + 'context': [{'type': 'primary', 'scope': 'group', 'id': 'former-group'}], + }, + { + 'find_group_by_id': lambda group_id: {'id': group_id, 'status': 'active'}, + 'check_group_status_allows_operation': lambda group, operation: (True, ''), + 'assert_group_role': ( + lambda *args, **kwargs: (_ for _ in ()).throw(PermissionError('removed')) + ), + }, + 'no longer available to this user', + ), + ( + { + **build_source_conversation(chat_type='public'), + 'context': [{'type': 'primary', 'scope': 'public', 'id': 'missing-public'}], + }, + {'find_public_workspace_by_id': lambda workspace_id: None}, + 'public workspace is no longer available', + ), + ], +) +def test_fork_rejects_unavailable_workspace_context( + conversation, + access_replacements, + error_match, +): + """Reject stale workspace context before copying any fork records.""" + operations_module = load_operations_module_for_test() + with pytest.raises(operations_module.ConversationForkConflictError, match=error_match): + run_fork( + source_conversation=conversation, + access_replacements=access_replacements, + ) + + def test_fork_rejects_concurrent_source_changes(): """Abort when the authoritative source changes during the copy.""" operations_module = load_operations_module_for_test() diff --git a/functional_tests/test_conversations_read_ownership_authorization.py b/functional_tests/test_conversations_read_ownership_authorization.py index 90be1ba72..497d81234 100644 --- a/functional_tests/test_conversations_read_ownership_authorization.py +++ b/functional_tests/test_conversations_read_ownership_authorization.py @@ -2,14 +2,15 @@ # test_conversations_read_ownership_authorization.py """ Functional test for personal conversation read authorization hardening. -Version: 0.250.074 -Implemented in: 0.241.011; 0.241.022; 0.241.032; 0.250.033; 0.250.035; 0.250.074 +Version: 0.250.101 +Implemented in: 0.241.011; 0.241.022; 0.241.032; 0.250.033; 0.250.035; 0.250.074; 0.250.101 This test ensures authenticated users can only read messages and images from their own personal conversations, and that foreign conversation reads fail with 403 without querying the message container. It also validates blob-backed image messages stream only after conversation authorization succeeds. It validates -that mark-read cache invalidation is idempotent for already-read conversations. +that mark-read cache invalidation is idempotent for already-read conversations +and that fork conflict logging preserves the intended HTTP 409 response. """ import copy @@ -694,6 +695,71 @@ def test_mark_read_only_invalidates_cache_when_unread_state_changes(): restore() +def test_fork_conflict_logging_preserves_409_response(): + """Verify structured conflict logging cannot turn a fork conflict into HTTP 500.""" + print("๐Ÿ” Testing fork conflict logging response...") + + app, _message_container, restore = build_test_app( + 'user-owner', + [ + { + 'id': 'conversation-owner', + 'user_id': 'user-owner', + 'chat_type': 'group-single-user', + } + ], + [], + ) + + try: + route_module = app.config['route_backend_conversations'] + logged_events = [] + + def raise_fork_conflict(**kwargs): + raise route_module.ConversationForkConflictError('Workspace access changed') + + def capture_log_event( + message, + extra=None, + level=None, + includeStack=False, + stacklevel=2, + exceptionTraceback=None, + debug_only=False, + category='INFO', + flush=False, + message_args=None, + ): + logged_events.append({'message': message, 'extra': extra, 'level': level}) + + route_module.fork_personal_conversation_for_user = raise_fork_conflict + route_module.log_event = capture_log_event + + with app.test_client() as client: + response = client.post( + '/api/conversations/conversation-owner/fork', + json={'message_id': 'assistant-message'}, + ) + + payload = response.get_json() + assert response.status_code == 409, ( + f"Expected 409, got {response.status_code}: {payload}" + ) + assert payload == {'error': 'Conversation fork conflict'}, ( + f"Expected fork conflict payload, got {payload}" + ) + assert logged_events and logged_events[0]['extra'] == { + 'source_conversation_id': 'conversation-owner', + 'selected_message_id': 'assistant-message', + 'user_id': 'user-owner', + }, f"Expected structured fork conflict context, got {logged_events}" + + print("โœ… Fork conflict logging preserved the intended 409 response") + return True if __name__ == '__main__' else None + finally: + restore() + + if __name__ == '__main__': tests = [ test_owner_can_read_messages, @@ -704,6 +770,7 @@ def test_mark_read_only_invalidates_cache_when_unread_state_changes(): test_foreign_image_return_forbidden_before_query, test_missing_image_preserves_not_found_response, test_mark_read_only_invalidates_cache_when_unread_state_changes, + test_fork_conflict_logging_preserves_409_response, ] print('๐Ÿงช Running conversation read ownership authorization tests...') diff --git a/ui_tests/test_chat_conversation_fork.py b/ui_tests/test_chat_conversation_fork.py index 92a384c28..898600221 100644 --- a/ui_tests/test_chat_conversation_fork.py +++ b/ui_tests/test_chat_conversation_fork.py @@ -1,12 +1,12 @@ # test_chat_conversation_fork.py """ UI test for personal conversation forking. -Version: 0.250.074 +Version: 0.250.101 Implemented in: 0.250.074 -This test ensures only persisted completed assistant messages in personal -conversations expose the fork action and validates confirmation, failure -feedback, duplicate-click prevention, and successful fork activation. +This test ensures persisted completed assistant messages in supported +single-user conversations expose the fork action and validates confirmation, +failure feedback, duplicate-click prevention, and successful fork activation. """ import json @@ -190,17 +190,69 @@ def fulfill_synthetic_api(route): metadata: {}, } ); + + sourceItem.dataset.chatType = 'public'; + chatMessages.appendMessage( + 'AI', + 'Public response', + null, + 'public-assistant', + false, + [], + [], + [], + null, + null, + { + id: 'public-assistant', + conversation_id: 'fork-source', + role: 'assistant', + content: 'Public response', + metadata: {} + } + ); + + sourceItem.dataset.chatType = 'personal_single_user'; + sourceItem.dataset.conversationKind = 'collaborative'; + chatMessages.appendMessage( + 'AI', + 'Collaborative response', + null, + 'collaborative-assistant', + false, + [], + [], + [], + null, + null, + { + id: 'collaborative-assistant', + conversation_id: 'fork-source', + role: 'assistant', + content: 'Collaborative response', + metadata: {} + } + ); + delete sourceItem.dataset.conversationKind; sourceItem.dataset.chatType = 'personal_single_user'; return { persisted: Boolean(document.querySelector('[data-message-id="persisted-assistant"] .dropdown-fork-conversation-btn')), streaming: Boolean(document.querySelector('[data-message-id="temp_ai_streaming"] .dropdown-fork-conversation-btn')), group: Boolean(document.querySelector('[data-message-id="group-assistant"] .dropdown-fork-conversation-btn')), + public: Boolean(document.querySelector('[data-message-id="public-assistant"] .dropdown-fork-conversation-btn')), + collaborative: Boolean(document.querySelector('[data-message-id="collaborative-assistant"] .dropdown-fork-conversation-btn')), }; }""" ) - assert visibility == {'persisted': True, 'streaming': False, 'group': False} + assert visibility == { + 'persisted': True, + 'streaming': False, + 'group': True, + 'public': True, + 'collaborative': False, + } fork_action = page.locator( '[data-message-id="persisted-assistant"] .dropdown-fork-conversation-btn'