From ad6ed864a12f9ce5e25bb3c2e1ed67bfc73057f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Fri, 30 Oct 2026 02:35:00 +0000 Subject: [PATCH] refactor(api): share console conversation queries and deletion (#44043) --- api/.importlinter | 13 + api/controllers/console/app/conversation.py | 487 +++---- api/extensions/ext_application_services.py | 7 +- api/openapi/markdown/console-openapi.md | 28 +- api/repositories/conversation_repository.py | 336 ++++- api/repositories/message_projection.py | 163 +++ api/repositories/message_repository.py | 153 +-- api/services/conversation_command_service.py | 29 +- api/services/conversation_query_service.py | 72 +- api/services/conversation_service.py | 26 - .../entities/conversation_entities.py | 124 ++ .../app/test_conversation_read_timestamp.py | 100 +- .../services/test_conversation_service.py | 111 -- .../console/app/test_conversation_delete.py | 337 +++++ .../console/app/test_conversation_detail.py | 477 +++++++ .../console/app/test_conversation_list.py | 457 +++++++ .../service_api/app/test_conversation_list.py | 2 + .../controllers/web/test_conversation_list.py | 2 + .../console/app/test_conversation.py | 234 +--- .../console/explore/test_conversation.py | 5 +- .../service_api/app/test_conversation.py | 9 - .../test_ext_application_services.py | 1 + .../test_conversation_repository.py | 1125 ++++++++++++++++- .../test_conversation_command_service.py | 181 ++- .../test_conversation_query_service.py | 464 ++++++- .../services/test_conversation_service.py | 130 +- ...test_installed_app_conversation_service.py | 5 +- .../generated/api/console/apps/errors.gen.ts | 28 +- .../generated/api/console/apps/orpc.gen.ts | 25 +- .../generated/api/console/apps/types.gen.ts | 23 +- .../generated/api/console/apps/zod.gen.ts | 12 + .../api/console/installed-apps/zod.gen.ts | 2 +- 32 files changed, 4149 insertions(+), 1019 deletions(-) create mode 100644 api/repositories/message_projection.py create mode 100644 api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_delete.py create mode 100644 api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_detail.py create mode 100644 api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_list.py diff --git a/api/.importlinter b/api/.importlinter index a7d14791f03..63c90c9f621 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -1061,6 +1061,19 @@ forbidden_modules = sqlalchemy werkzeug +[importlinter:contract:console-conversation-controller-boundary] +name = Console conversation controllers reach persistence through the composition root +type = forbidden +source_modules = + controllers.console.app.conversation +forbidden_modules = + extensions.ext_database + models + repositories + services.conversation_service + sqlalchemy +allow_indirect_imports = True + [importlinter:contract:billing-application-boundary] name = Billing application services and ports are framework and infrastructure neutral type = forbidden diff --git a/api/controllers/console/app/conversation.py b/api/controllers/console/app/conversation.py index 93f4cf195be..6abde16df10 100644 --- a/api/controllers/console/app/conversation.py +++ b/api/controllers/console/app/conversation.py @@ -1,29 +1,19 @@ +from http import HTTPStatus from typing import Literal from uuid import UUID -import sqlalchemy as sa -from flask import abort from flask_restx import Resource from pydantic import BaseModel, Field, field_validator -from sqlalchemy import func, or_ -from sqlalchemy.orm import Session, selectinload -from werkzeug.exceptions import NotFound +from controllers.common.errors import ConversationNotFoundHTTPError, InvalidArgumentError, UnauthorizedError from controllers.common.rbac import PlainApp, RBACCheck from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models -from controllers.common.session import with_session from controllers.console import console_ns -from controllers.console.app.wraps import get_app_model -from controllers.console.wraps import ( - RBACPermission, - account_initialization_required, - edit_permission_required, - model_validate, - rbac_permission_required, - setup_required, - with_current_user, -) -from core.app.entities.app_invoke_entities import InvokeFrom +from controllers.console.app.error import AppNotFoundError +from controllers.console.flask_admission import console_account_admission +from controllers.console.wraps import RBACPermission, validate_request +from enums.account import TenantAccountRole +from extensions.ext_application_services import application_services from fields.conversation_fields import ( Conversation as ConversationResponse, ) @@ -36,18 +26,16 @@ from fields.conversation_fields import ( from fields.conversation_fields import ( ConversationPagination as ConversationPaginationResponse, ) -from fields.conversation_fields import ConversationResponseSource from fields.conversation_fields import ( ConversationWithSummaryPagination as ConversationWithSummaryPaginationResponse, ) -from libs.datetime_utils import naive_utc_now, parse_time_range +from libs.datetime_utils import parse_time_range +from libs.external_api import error_responses from libs.helper import dump_response -from libs.login import login_required -from libs.pagination import paginate_query -from models import Conversation, EndUser, Message, MessageAnnotation -from models.account import Account -from models.model import App, AppMode -from services.conversation_service import ConversationService +from machinery.context import RequestContext +from services.app.console_service import ConsoleAppNotFoundError +from services.entities.conversation_entities import ConsoleConversationFilters +from services.errors.base import NoPermissionError from services.errors.conversation import ConversationNotExistsError @@ -96,322 +84,205 @@ register_response_schema_models( @console_ns.route("/apps//completion-conversations") class CompletionConversationApi(Resource): + @error_responses(AppNotFoundError) @console_ns.doc("list_completion_conversations") @console_ns.doc(description="Get completion conversations with pagination and filtering") @console_ns.doc(params={"app_id": "Application ID", **query_params_from_model(CompletionConversationQuery)}) - @console_ns.response(200, "Success", console_ns.models[ConversationPaginationResponse.__name__]) - @console_ns.response(403, "Insufficient permissions") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @with_current_user - @rbac_permission_required(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp())) - @with_session(write=False) - @get_app_model(mode=AppMode.COMPLETION) - @model_validate(CompletionConversationQuery) - def get(self, req_data: CompletionConversationQuery, session: Session, current_user: Account, app_model: App): - query = sa.select(Conversation).where( - Conversation.app_id == app_model.id, Conversation.mode == "completion", Conversation.is_deleted.is_(False) - ) - - if req_data.keyword: - from libs.helper import escape_like_pattern - - escaped_keyword = escape_like_pattern(req_data.keyword) - query = query.join(Message, Message.conversation_id == Conversation.id).where( - or_( - Message.query.ilike(f"%{escaped_keyword}%", escape="\\"), - Message.answer.ilike(f"%{escaped_keyword}%", escape="\\"), - ) - ) - - account = current_user - assert account.timezone is not None - + @console_ns.response(HTTPStatus.OK, "Success", console_ns.models[ConversationPaginationResponse.__name__]) + @console_ns.response(HTTPStatus.BAD_REQUEST, "Invalid date range") + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App not found") + @console_ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, "Invalid query parameters") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp()),), + ) + def get(self, request_context: RequestContext, app_id: UUID) -> dict[str, object]: + queries = application_services().conversation_queries try: - start_datetime_utc, end_datetime_utc = parse_time_range(req_data.start, req_data.end, account.timezone) - except ValueError as e: - abort(400, description=str(e)) - - if start_datetime_utc: - query = query.where(Conversation.created_at >= start_datetime_utc) - - if end_datetime_utc: - end_datetime_utc = end_datetime_utc.replace(second=59) - query = query.where(Conversation.created_at <= end_datetime_utc) - - # FIXME, the type ignore in this file - if req_data.annotation_status == "annotated": - query = ( - query.options(selectinload(Conversation.message_annotations)) # type: ignore[arg-type] - .join( # type: ignore - MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id - ) - .group_by(Conversation.id) + scope = queries.get_console_scope(request_context, str(app_id), kind="completion") + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except NoPermissionError as exc: + raise UnauthorizedError(str(exc)) from exc + req_data = validate_request(CompletionConversationQuery) + try: + start, end = parse_time_range(req_data.start, req_data.end, scope.timezone) + except ValueError as exc: + raise InvalidArgumentError(str(exc)) from exc + try: + page = queries.get_console_page( + scope=scope, + filters=ConsoleConversationFilters( + keyword=req_data.keyword, + start=start, + end=end.replace(second=59) if end else None, + annotation_status=req_data.annotation_status, + sort_by="-created_at", + page=req_data.page, + limit=req_data.limit, + ), ) - elif req_data.annotation_status == "not_annotated": - query = ( - query.outerjoin(MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id) - .group_by(Conversation.id) - .having(func.count(MessageAnnotation.id) == 0) - ) - - query = query.order_by(Conversation.created_at.desc()) - - conversations = paginate_query(query, session=session, page=req_data.page, per_page=req_data.limit) - - return dump_response( - ConversationPaginationResponse, - { - "page": conversations.page, - "per_page": conversations.per_page, - "total": conversations.total, - "has_next": conversations.has_next, - "items": [ConversationResponseSource(item, session=session) for item in conversations.items], - }, - ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + return dump_response(ConversationPaginationResponse, page) @console_ns.route("/apps//completion-conversations/") class CompletionConversationDetailApi(Resource): + @error_responses(AppNotFoundError, ConversationNotFoundHTTPError) @console_ns.doc("get_completion_conversation") @console_ns.doc(description="Get completion conversation details with messages") @console_ns.doc(params={"app_id": "Application ID", "conversation_id": "Conversation ID"}) - @console_ns.response(200, "Success", console_ns.models[ConversationMessageDetailResponse.__name__]) - @console_ns.response(403, "Insufficient permissions") - @console_ns.response(404, "Conversation not found") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @with_current_user - @rbac_permission_required(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp())) - @with_session - @get_app_model(mode=AppMode.COMPLETION) - def get(self, session: Session, current_user: Account, app_model: App, conversation_id: UUID): - conversation_id_str = str(conversation_id) - return dump_response( - ConversationMessageDetailResponse, - ConversationResponseSource( - _get_conversation(session, current_user, app_model, conversation_id_str), session=session - ), - ) + @console_ns.response(HTTPStatus.OK, "Success", console_ns.models[ConversationMessageDetailResponse.__name__]) + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App or conversation not found") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp()),), + ) + def get(self, request_context: RequestContext, app_id: UUID, conversation_id: UUID) -> dict[str, object]: + services = application_services() + try: + detail = services.conversation_queries.get_console_detail( + request_context, str(app_id), str(conversation_id), kind="completion" + ) + response = dump_response(ConversationMessageDetailResponse, detail) + # Preserve the Console read receipt only after rendering succeeds. + services.conversation_commands.mark_console_read( + request_context, str(app_id), str(conversation_id), kind="completion" + ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except ConversationNotExistsError as exc: + raise ConversationNotFoundHTTPError() from exc + return response + @error_responses(AppNotFoundError, ConversationNotFoundHTTPError) @console_ns.doc("delete_completion_conversation") @console_ns.doc(description="Delete a completion conversation") @console_ns.doc(params={"app_id": "Application ID", "conversation_id": "Conversation ID"}) - @console_ns.response(204, "Conversation deleted successfully") - @console_ns.response(403, "Insufficient permissions") - @console_ns.response(404, "Conversation not found") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @rbac_permission_required(RBACCheck(RBACPermission.APP_EDIT, PlainApp())) - @with_current_user - @with_session - @get_app_model(mode=AppMode.COMPLETION) - def delete(self, session: Session, current_user: Account, app_model: App, conversation_id: UUID): - conversation_id_str = str(conversation_id) - + @console_ns.response(HTTPStatus.NO_CONTENT, "Conversation deleted successfully") + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App or conversation not found") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_EDIT, PlainApp()),), + ) + def delete(self, request_context: RequestContext, app_id: UUID, conversation_id: UUID) -> tuple[str, HTTPStatus]: try: - ConversationService.delete(app_model, conversation_id_str, current_user, session=session) - except ConversationNotExistsError as caught_error: - raise NotFound("Conversation Not Exists.") from caught_error - - return "", 204 + application_services().conversation_commands.delete_console( + request_context, str(app_id), str(conversation_id), kind="completion" + ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except ConversationNotExistsError as exc: + raise ConversationNotFoundHTTPError() from exc + return "", HTTPStatus.NO_CONTENT @console_ns.route("/apps//chat-conversations") class ChatConversationApi(Resource): + @error_responses(AppNotFoundError) @console_ns.doc("list_chat_conversations") - @console_ns.doc(description="Get chat conversations with pagination, filtering and summary") + @console_ns.doc(description="Get chat conversations with pagination and filtering") @console_ns.doc(params={"app_id": "Application ID", **query_params_from_model(ChatConversationQuery)}) - @console_ns.response(200, "Success", console_ns.models[ConversationWithSummaryPaginationResponse.__name__]) - @console_ns.response(403, "Insufficient permissions") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @with_current_user - @rbac_permission_required(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp())) - @with_session(write=False) - @get_app_model(mode=[AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT]) - @model_validate(ChatConversationQuery) - def get(self, req_data: ChatConversationQuery, session: Session, current_user: Account, app_model: App): - subquery = ( - sa.select(Conversation.id.label("conversation_id"), EndUser.session_id.label("from_end_user_session_id")) - .outerjoin(EndUser, Conversation.from_end_user_id == EndUser.id) - .subquery() - ) - - query = sa.select(Conversation).where(Conversation.app_id == app_model.id, Conversation.is_deleted.is_(False)) - - if req_data.keyword: - from libs.helper import escape_like_pattern - - escaped_keyword = escape_like_pattern(req_data.keyword) - keyword_filter = f"%{escaped_keyword}%" - query = ( - query.join( - Message, - Message.conversation_id == Conversation.id, - ) - .join(subquery, subquery.c.conversation_id == Conversation.id) - .where( - or_( - Message.query.ilike(keyword_filter, escape="\\"), - Message.answer.ilike(keyword_filter, escape="\\"), - Conversation.name.ilike(keyword_filter, escape="\\"), - Conversation.introduction.ilike(keyword_filter, escape="\\"), - subquery.c.from_end_user_session_id.ilike(keyword_filter, escape="\\"), - ), - ) - .group_by(Conversation.id) - ) - - account = current_user - assert account.timezone is not None - + @console_ns.response( + HTTPStatus.OK, "Success", console_ns.models[ConversationWithSummaryPaginationResponse.__name__] + ) + @console_ns.response(HTTPStatus.BAD_REQUEST, "Invalid date range") + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App not found") + @console_ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, "Invalid query parameters") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp()),), + ) + def get(self, request_context: RequestContext, app_id: UUID) -> dict[str, object]: + queries = application_services().conversation_queries try: - start_datetime_utc, end_datetime_utc = parse_time_range(req_data.start, req_data.end, account.timezone) - except ValueError as e: - abort(400, description=str(e)) - - if start_datetime_utc: - match req_data.sort_by: - case "updated_at" | "-updated_at": - query = query.where(Conversation.updated_at >= start_datetime_utc) - case "created_at" | "-created_at" | _: - query = query.where(Conversation.created_at >= start_datetime_utc) - - if end_datetime_utc: - end_datetime_utc = end_datetime_utc.replace(second=59) - match req_data.sort_by: - case "updated_at" | "-updated_at": - query = query.where(Conversation.updated_at <= end_datetime_utc) - case "created_at" | "-created_at" | _: - query = query.where(Conversation.created_at <= end_datetime_utc) - - match req_data.annotation_status: - case "annotated": - query = ( - query.options(selectinload(Conversation.message_annotations)) # type: ignore[arg-type] - .join( # type: ignore - MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id - ) - .group_by(Conversation.id) - ) - case "not_annotated": - query = ( - query.outerjoin(MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id) - .group_by(Conversation.id) - .having(func.count(MessageAnnotation.id) == 0) - ) - case "all": - pass - - if app_model.mode == AppMode.ADVANCED_CHAT: - query = query.where(Conversation.invoke_from != InvokeFrom.DEBUGGER) - - match req_data.sort_by: - case "created_at": - query = query.order_by(Conversation.created_at.asc()) - case "-created_at": - query = query.order_by(Conversation.created_at.desc()) - case "updated_at": - query = query.order_by(Conversation.updated_at.asc()) - case "-updated_at": - query = query.order_by(Conversation.updated_at.desc()) - case _: - query = query.order_by(Conversation.created_at.desc()) - - conversations = paginate_query(query, session=session, page=req_data.page, per_page=req_data.limit) - - return dump_response( - ConversationWithSummaryPaginationResponse, - { - "page": conversations.page, - "per_page": conversations.per_page, - "total": conversations.total, - "has_next": conversations.has_next, - "items": [ConversationResponseSource(item, session=session) for item in conversations.items], - }, - ) + scope = queries.get_console_scope(request_context, str(app_id), kind="chat") + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except NoPermissionError as exc: + raise UnauthorizedError(str(exc)) from exc + req_data = validate_request(ChatConversationQuery) + try: + start, end = parse_time_range(req_data.start, req_data.end, scope.timezone) + except ValueError as exc: + raise InvalidArgumentError(str(exc)) from exc + try: + page = queries.get_console_page( + scope=scope, + filters=ConsoleConversationFilters( + keyword=req_data.keyword, + start=start, + end=end.replace(second=59) if end else None, + annotation_status=req_data.annotation_status, + sort_by=req_data.sort_by, + page=req_data.page, + limit=req_data.limit, + ), + ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + return dump_response(ConversationWithSummaryPaginationResponse, page) @console_ns.route("/apps//chat-conversations/") class ChatConversationDetailApi(Resource): + @error_responses(AppNotFoundError, ConversationNotFoundHTTPError) @console_ns.doc("get_chat_conversation") @console_ns.doc(description="Get chat conversation details") @console_ns.doc(params={"app_id": "Application ID", "conversation_id": "Conversation ID"}) - @console_ns.response(200, "Success", console_ns.models[ConversationDetailResponse.__name__]) - @console_ns.response(403, "Insufficient permissions") - @console_ns.response(404, "Conversation not found") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @with_current_user - @rbac_permission_required(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp())) - @with_session - @get_app_model(mode=[AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT]) - def get(self, session: Session, current_user: Account, app_model: App, conversation_id: UUID): - conversation_id_str = str(conversation_id) - return dump_response( - ConversationDetailResponse, - ConversationResponseSource( - _get_conversation(session, current_user, app_model, conversation_id_str), session=session - ), - ) + @console_ns.response(HTTPStatus.OK, "Success", console_ns.models[ConversationDetailResponse.__name__]) + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App or conversation not found") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_VIEW_LAYOUT, PlainApp()),), + ) + def get(self, request_context: RequestContext, app_id: UUID, conversation_id: UUID) -> dict[str, object]: + services = application_services() + try: + detail = services.conversation_queries.get_console_detail( + request_context, str(app_id), str(conversation_id), kind="chat" + ) + response = dump_response(ConversationDetailResponse, detail) + # Preserve the Console read receipt only after rendering succeeds. + services.conversation_commands.mark_console_read( + request_context, str(app_id), str(conversation_id), kind="chat" + ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except ConversationNotExistsError as exc: + raise ConversationNotFoundHTTPError() from exc + return response + @error_responses(AppNotFoundError, ConversationNotFoundHTTPError) @console_ns.doc("delete_chat_conversation") @console_ns.doc(description="Delete a chat conversation") @console_ns.doc(params={"app_id": "Application ID", "conversation_id": "Conversation ID"}) - @console_ns.response(204, "Conversation deleted successfully") - @console_ns.response(403, "Insufficient permissions") - @console_ns.response(404, "Conversation not found") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @rbac_permission_required(RBACCheck(RBACPermission.APP_EDIT, PlainApp())) - @with_current_user - @with_session - @get_app_model(mode=[AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT]) - def delete(self, session: Session, current_user: Account, app_model: App, conversation_id: UUID): - conversation_id_str = str(conversation_id) - + @console_ns.response(HTTPStatus.NO_CONTENT, "Conversation deleted successfully") + @console_ns.response(HTTPStatus.UNAUTHORIZED, "Authentication required") + @console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient permissions") + @console_ns.response(HTTPStatus.NOT_FOUND, "App or conversation not found") + @console_account_admission( + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}), + rbac_checks=(RBACCheck(RBACPermission.APP_EDIT, PlainApp()),), + ) + def delete(self, request_context: RequestContext, app_id: UUID, conversation_id: UUID) -> tuple[str, HTTPStatus]: try: - ConversationService.delete(app_model, conversation_id_str, current_user, session=session) - except ConversationNotExistsError as caught_error: - raise NotFound("Conversation Not Exists.") from caught_error - - return "", 204 - - -def _get_conversation(session: Session, current_user: Account, app_model, conversation_id): - conversation = session.scalar( - sa.select(Conversation).where(Conversation.id == conversation_id, Conversation.app_id == app_model.id).limit(1) - ) - - if not conversation: - raise NotFound("Conversation Not Exists.") - - session.execute( - sa.update(Conversation) - .where(Conversation.id == conversation_id, Conversation.read_at.is_(None)) - # Keep updated_at unchanged when only marking a conversation as read. - .values( - { - Conversation.read_at: naive_utc_now(), - Conversation.read_account_id: current_user.id, - Conversation.updated_at: Conversation.updated_at, - } - ) - ) - session.flush() - session.refresh(conversation) - - return conversation + application_services().conversation_commands.delete_console( + request_context, str(app_id), str(conversation_id), kind="chat" + ) + except ConsoleAppNotFoundError as exc: + raise AppNotFoundError(str(exc)) from exc + except ConversationNotExistsError as exc: + raise ConversationNotFoundHTTPError() from exc + return "", HTTPStatus.NO_CONTENT diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index 29f371875ff..86f5b4bc3f3 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -588,7 +588,10 @@ def build_application_services( conversation_commands = ConversationCommandService( conversations=conversations, enqueue_delete_cleanup=ConversationService.enqueue_delete_cleanup ) - conversation_queries = ConversationQueryService(conversations=conversations, restore_inputs=restore_input_files) + message_files = MessageFileResolver() + conversation_queries = ConversationQueryService( + conversations=conversations, restore_inputs=restore_input_files, files=message_files + ) conversation_variables = ConversationVariableRepository(session_factory=database_client) annotations = AnnotationRepository(session_factory=database_client) messages = MessageRepository(session_factory=database_client) @@ -750,7 +753,7 @@ def build_application_services( message_feedbacks=MessageFeedbackService(repository=messages), message_queries=MessageQueryService( messages=messages, - files=MessageFileResolver(), + files=message_files, extra_contents=ExecutionExtraContentReader( repository=SQLAlchemyExecutionExtraContentRepository(session_maker=database_client) ), diff --git a/api/openapi/markdown/console-openapi.md b/api/openapi/markdown/console-openapi.md index f5d442cf4b3..a4dba9dcd61 100644 --- a/api/openapi/markdown/console-openapi.md +++ b/api/openapi/markdown/console-openapi.md @@ -2840,7 +2840,7 @@ Transcript audio to text for chat messages | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [GET] /apps/{app_id}/chat-conversations -Get chat conversations with pagination, filtering and summary +Get chat conversations with pagination and filtering #### Parameters @@ -2860,8 +2860,11 @@ Get chat conversations with pagination, filtering and summary | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Success | **application/json**: [ConversationWithSummaryPagination](#conversationwithsummarypagination)
| +| 400 | Invalid date range | **application/json**: [ErrorResponse](#errorresponse)
| +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Not Found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 422 | Invalid query parameters | **application/json**: [ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [DELETE] /apps/{app_id}/chat-conversations/{conversation_id} @@ -2879,8 +2882,9 @@ Delete a chat conversation | Code | Description | Schema | | ---- | ----------- | ------ | | 204 | Conversation deleted successfully | | +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App or conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ConversationNotFoundError](#conversationnotfounderror)
[ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [GET] /apps/{app_id}/chat-conversations/{conversation_id} @@ -2898,8 +2902,9 @@ Get chat conversation details | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Success | **application/json**: [ConversationDetail](#conversationdetail)
| +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App or conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ConversationNotFoundError](#conversationnotfounderror)
[ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [GET] /apps/{app_id}/chat-messages @@ -2983,8 +2988,11 @@ Get completion conversations with pagination and filtering | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Success | **application/json**: [ConversationPagination](#conversationpagination)
| +| 400 | Invalid date range | **application/json**: [ErrorResponse](#errorresponse)
| +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Not Found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 422 | Invalid query parameters | **application/json**: [ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [DELETE] /apps/{app_id}/completion-conversations/{conversation_id} @@ -3002,8 +3010,9 @@ Delete a completion conversation | Code | Description | Schema | | ---- | ----------- | ------ | | 204 | Conversation deleted successfully | | +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App or conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ConversationNotFoundError](#conversationnotfounderror)
[ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [GET] /apps/{app_id}/completion-conversations/{conversation_id} @@ -3021,8 +3030,9 @@ Get completion conversation details with messages | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Success | **application/json**: [ConversationMessageDetail](#conversationmessagedetail)
| +| 401 | Authentication required | **application/json**: [ErrorResponse](#errorresponse)
| | 403 | Insufficient permissions | **application/json**: [ErrorResponse](#errorresponse)
| -| 404 | Conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ErrorResponse](#errorresponse)
| +| 404 | App or conversation not found | **application/json**: [AppNotFoundError](#appnotfounderror)
[ConversationNotFoundError](#conversationnotfounderror)
[ErrorResponse](#errorresponse)
| | default | Error | **application/json**: [ErrorResponse](#errorresponse)
| ### [POST] /apps/{app_id}/completion-messages @@ -22915,11 +22925,11 @@ A name is required when automatic naming is disabled. #### ConversationNotFoundError -The conversation was not found for this account and app. +The conversation was not found for this user and app. | Name | Type | Description | Required | | ---- | ---- | ----------- | -------- | -| ConversationNotFoundError | | The conversation was not found for this account and app. | | +| ConversationNotFoundError | | The conversation was not found for this user and app. | | #### ConversationPagination diff --git a/api/repositories/conversation_repository.py b/api/repositories/conversation_repository.py index 2724d1e3619..44cc9a2a3c0 100644 --- a/api/repositories/conversation_repository.py +++ b/api/repositories/conversation_repository.py @@ -1,17 +1,41 @@ -"""Persist conversations within their end-user or installed-account owner chain.""" +"""Persist conversations within their end-user, installed-account, or Console app scope.""" from copy import deepcopy from dataclasses import replace from datetime import datetime +from typing import cast -from sqlalchemy import Select, exists, or_, select +from pydantic import JsonValue +from sqlalchemy import Select, exists, func, or_, select, update from sqlalchemy.orm import Session, sessionmaker -from models.enums import ConversationFromSource, CreatorUserRole -from models.model import App, Conversation, EndUser, InstalledApp, Message +from libs.datetime_utils import naive_utc_now +from libs.helper import escape_like_pattern +from libs.pagination import paginate_query +from machinery.context import RequestContext +from models.account import Account +from models.enums import ConversationFromSource, CreatorUserRole, InvokeFrom +from models.model import App, AppMode, Conversation, EndUser, InstalledApp, Message, MessageAnnotation from models.web import PinnedConversation +from repositories.app.console_repository import find_console_app from repositories.conversation_lifecycle import retire_conversation +from repositories.message_projection import snapshot_console_message +from services.app.console_service import ConsoleAppNotFoundError from services.entities.conversation_entities import ( + ConsoleChatConversationDetail, + ConsoleChatConversationRecord, + ConsoleCompletionConversationDetail, + ConsoleCompletionConversationRecord, + ConsoleConversationAccount, + ConsoleConversationAnnotation, + ConsoleConversationDetail, + ConsoleConversationDetailSource, + ConsoleConversationFilters, + ConsoleConversationKind, + ConsoleConversationMessage, + ConsoleConversationPage, + ConsoleConversationRecord, + ConsoleConversationScope, ConversationDeletion, ConversationInputValue, ConversationListSource, @@ -24,6 +48,7 @@ from services.entities.conversation_entities import ( InstalledAppConversationRef, InstalledAppConversationScope, ) +from services.errors.base import NoPermissionError from services.errors.conversation import ConversationNotExistsError, LastConversationNotExistsError from services.errors.message import MessageNotExistsError from services.installed_app_access_service import InstalledAppNotFoundError, InstalledAppRef @@ -129,6 +154,275 @@ class ConversationRepository: ), ) + def get_console_scope( + self, context: RequestContext, app_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleConversationScope: + with self._session_factory() as session: + app = self._get_console_app(session=session, context=context, app_id=app_id, kind=kind) + account = session.get(Account, context.account_id) + if account is None: + raise NoPermissionError("The admitted account no longer exists.") + assert account.timezone is not None + return ConsoleConversationScope( + app_id=app.id, + tenant_id=app.tenant_id, + app_mode=app.mode, + timezone=account.timezone, + ) + + def get_console_page( + self, *, scope: ConsoleConversationScope, filters: ConsoleConversationFilters + ) -> ConsoleConversationPage: + with self._session_factory() as session: + app = find_console_app(session, workspace_id=scope.tenant_id, app_id=scope.app_id) + if app is None: + raise ConsoleAppNotFoundError("App not found.") + if app.mode != scope.app_mode: + raise ConsoleAppNotFoundError( + f"App mode changed from '{scope.app_mode}' to '{app.mode}' after admission." + ) + completion = app.mode == AppMode.COMPLETION + statement = select(Conversation).where(Conversation.app_id == app.id, Conversation.is_deleted.is_(False)) + if completion: + statement = statement.where(Conversation.mode == AppMode.COMPLETION) + if filters.keyword: + pattern = f"%{escape_like_pattern(filters.keyword)}%" + statement = statement.join(Message, Message.conversation_id == Conversation.id) + matches = [ + Message.query.ilike(pattern, escape="\\"), + Message.answer.ilike(pattern, escape="\\"), + ] + if not completion: + statement = statement.outerjoin(EndUser, Conversation.from_end_user_id == EndUser.id) + matches.extend( + [ + Conversation.name.ilike(pattern, escape="\\"), + Conversation.introduction.ilike(pattern, escape="\\"), + EndUser.session_id.ilike(pattern, escape="\\"), + ] + ) + statement = statement.group_by(Conversation.id) + # Completion historically counts each matching message join row. + statement = statement.where(or_(*matches)) + + column = ( + Conversation.created_at + if completion or filters.sort_by in {"created_at", "-created_at"} + else Conversation.updated_at + ) + if filters.start is not None: + statement = statement.where(column >= filters.start) + if filters.end is not None: + statement = statement.where(column <= filters.end) + if filters.annotation_status == "annotated": + statement = statement.join( + MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id + ).group_by(Conversation.id) + elif filters.annotation_status == "not_annotated": + statement = ( + statement.outerjoin(MessageAnnotation, MessageAnnotation.conversation_id == Conversation.id) + .group_by(Conversation.id) + .having(func.count(MessageAnnotation.id) == 0) + ) + if app.mode == AppMode.ADVANCED_CHAT: + # SQL != also excludes NULL, matching the existing Console query. + statement = statement.where(Conversation.invoke_from != InvokeFrom.DEBUGGER) + statement = statement.order_by( + column.desc() if completion or filters.sort_by.startswith("-") else column.asc() + ) + page = paginate_query(statement, session=session, page=filters.page, per_page=filters.limit) + return ConsoleConversationPage( + tenant_id=app.tenant_id, + page=page.page, + per_page=page.per_page, + total=page.total, + has_next=page.has_next, + items=tuple( + self._console_record(conversation=item, session=session, completion=completion) + for item in page.items + ), + ) + + def get_console_detail( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleConversationDetailSource: + with self._session_factory() as session: + app = self._get_console_app(session=session, context=context, app_id=app_id, kind=kind) + conversation = self._get_console_conversation( + session=session, app_id=app.id, conversation_id=conversation_id + ) + common = self._console_detail_record(conversation=conversation, session=session) + if kind == "completion": + message = conversation.first_message_with_session(session=session) + message_source = snapshot_console_message(message=message, session=session) if message else None + return ConsoleConversationDetailSource( + tenant_id=app.tenant_id, + record=ConsoleCompletionConversationDetail( + id=common.id, + status=common.status, + from_source=common.from_source, + from_end_user_id=common.from_end_user_id, + from_account_id=common.from_account_id, + created_at=common.created_at, + model_config=common.model_config, + first_message=message_source.record if message_source is not None else None, + ), + files=message_source.files if message_source is not None else (), + ) + return ConsoleConversationDetailSource( + tenant_id=app.tenant_id, + record=ConsoleChatConversationDetail( + id=common.id, + status=common.status, + from_source=common.from_source, + from_end_user_id=common.from_end_user_id, + from_account_id=common.from_account_id, + created_at=common.created_at, + model_config=common.model_config, + updated_at=conversation.updated_at, + annotated=conversation.annotated_with_session(session=session), + introduction=conversation.introduction, + message_count=conversation.message_count_with_session(session=session), + user_feedback_stats=conversation.user_feedback_stats_with_session(session=session), + admin_feedback_stats=conversation.admin_feedback_stats_with_session(session=session), + ), + files=(), + ) + + def mark_console_read( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> None: + with self._session_factory.begin() as session: + app = self._get_console_app(session=session, context=context, app_id=app_id, kind=kind) + self._get_console_conversation(session=session, app_id=app.id, conversation_id=conversation_id) + session.execute( + update(Conversation) + .where( + Conversation.id == conversation_id, + Conversation.app_id == app.id, + Conversation.read_at.is_(None), + ) + .values( + { + Conversation.read_at: naive_utc_now(), + Conversation.read_account_id: context.account_id, + # Marking a log read must not reorder the conversation list. + Conversation.updated_at: Conversation.updated_at, + } + ) + ) + + @staticmethod + def _get_console_conversation(*, session: Session, app_id: str, conversation_id: str) -> Conversation: + # Console details include every actor's history and soft-deleted rows. + conversation = session.scalar( + select(Conversation).where(Conversation.id == conversation_id, Conversation.app_id == app_id) + ) + if conversation is None: + raise ConversationNotExistsError(f"Conversation {conversation_id} is unavailable for app {app_id}.") + return conversation + + @staticmethod + def _console_detail_record(*, conversation: Conversation, session: Session) -> ConsoleConversationDetail: + return ConsoleConversationDetail( + id=conversation.id, + status=conversation.status, + from_source=conversation.from_source, + from_end_user_id=conversation.from_end_user_id, + from_account_id=conversation.from_account_id, + created_at=conversation.created_at, + model_config=deepcopy(cast(dict[str, JsonValue], conversation.model_config_with_session(session=session))), + ) + + @classmethod + def _console_record( + cls, *, conversation: Conversation, session: Session, completion: bool + ) -> ConsoleCompletionConversationRecord | ConsoleChatConversationRecord: + # Keep established config/feedback/summary policies in their current + # DB-only owners; this snapshot must not restore attachments or URLs. + detail = cls._console_detail_record(conversation=conversation, session=session) + common = ConsoleConversationRecord( + id=detail.id, + status=detail.status, + from_source=detail.from_source, + from_end_user_id=detail.from_end_user_id, + from_end_user_session_id=conversation.from_end_user_session_id_with_session(session=session), + from_account_id=detail.from_account_id, + from_account_name=conversation.from_account_name_with_session(session=session), + read_at=conversation.read_at, + created_at=detail.created_at, + updated_at=conversation.updated_at, + model_config=detail.model_config, + user_feedback_stats=conversation.user_feedback_stats_with_session(session=session), + admin_feedback_stats=conversation.admin_feedback_stats_with_session(session=session), + ) + if completion: + annotation = conversation.annotation_with_session(session=session) + account = annotation.account_with_session(session=session) if annotation is not None else None + annotation_record = ( + ConsoleConversationAnnotation( + id=annotation.id, + question=annotation.question, + content=annotation.content, + account=( + ConsoleConversationAccount(id=account.id, name=account.name, email=account.email) + if account is not None + else None + ), + created_at=annotation.created_at, + ) + if annotation is not None + else None + ) + message = conversation.first_message_with_session(session=session) + first_message = ( + ConsoleConversationMessage( + inputs=deepcopy(message._inputs), + query=message.query, + message=deepcopy(cast(JsonValue, message.message)), + answer=message.answer, + ) + if message is not None + else None + ) + return ConsoleCompletionConversationRecord( + id=common.id, + status=common.status, + from_source=common.from_source, + from_end_user_id=common.from_end_user_id, + from_end_user_session_id=common.from_end_user_session_id, + from_account_id=common.from_account_id, + from_account_name=common.from_account_name, + read_at=common.read_at, + created_at=common.created_at, + updated_at=common.updated_at, + model_config=common.model_config, + user_feedback_stats=common.user_feedback_stats, + admin_feedback_stats=common.admin_feedback_stats, + annotation=annotation_record, + first_message=first_message, + ) + return ConsoleChatConversationRecord( + id=common.id, + status=common.status, + from_source=common.from_source, + from_end_user_id=common.from_end_user_id, + from_end_user_session_id=common.from_end_user_session_id, + from_account_id=common.from_account_id, + from_account_name=common.from_account_name, + read_at=common.read_at, + created_at=common.created_at, + updated_at=common.updated_at, + model_config=common.model_config, + user_feedback_stats=common.user_feedback_stats, + admin_feedback_stats=common.admin_feedback_stats, + name=conversation.name, + summary_or_query=conversation.summary_or_query_with_session(session=session), + annotated=conversation.annotated_with_session(session=session), + message_count=conversation.message_count_with_session(session=session), + status_count=conversation.status_count_with_session(session=session), + ) + def get_name_source(self, *, ref: ConversationRef) -> ConversationNameSource: with self._session_factory() as session: app, conversation = self._resolve(session=session, ref=ref) @@ -167,6 +461,23 @@ class ConversationRepository: retired_workspace_ids=retired_workspace_ids, ) + def delete_console( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> ConversationDeletion: + with self._session_factory.begin() as session: + app = self._get_console_app(session=session, context=context, app_id=app_id, kind=kind) + # Console log queries see all users; deletion still requires the + # current account's Console conversation, as in the original API. + conversation = self._get_conversation( + session=session, app_id=app.id, account_id=context.account_id, conversation_id=conversation_id + ) + retired_workspace_ids = retire_conversation(app_model=app, conversation=conversation, session=session) + return ConversationDeletion( + tenant_id=app.tenant_id, + conversation_id=conversation.id, + retired_workspace_ids=retired_workspace_ids, + ) + def set_pinned(self, *, ref: ConversationRef, is_pinned: bool) -> None: with self._session_factory.begin() as session: if isinstance(ref, InstalledAppConversationRef): @@ -248,6 +559,23 @@ class ConversationRepository: ) return result[0], result[1] + @staticmethod + def _get_console_app( + *, session: Session, context: RequestContext, app_id: str, kind: ConsoleConversationKind + ) -> App: + app = find_console_app(session, workspace_id=context.active_workspace_id, app_id=app_id) + allowed_modes = ( + {AppMode.COMPLETION} + if kind == "completion" + else {AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT} + ) + if app is None: + raise ConsoleAppNotFoundError("App not found.") + if app.mode not in allowed_modes: + mode_values = sorted(mode.value for mode in allowed_modes) + raise ConsoleAppNotFoundError(f"App mode is not in the supported list: {mode_values}") + return app + @staticmethod def _get_app(*, session: Session, installed_app: InstalledAppRef) -> App: app = session.scalar( diff --git a/api/repositories/message_projection.py b/api/repositories/message_projection.py new file mode 100644 index 00000000000..55089e74f4c --- /dev/null +++ b/api/repositories/message_projection.py @@ -0,0 +1,163 @@ +"""Snapshot message data in the caller's session before file or extra-content I/O.""" + +from copy import deepcopy +from typing import cast + +from pydantic import JsonValue +from sqlalchemy import select +from sqlalchemy.orm import Session + +from models.account import Account +from models.model import Message, MessageAgentThought, MessageFile +from services.entities.message_entities import ( + ConsoleMessageFeedbackRecord, + ConsoleMessageRecord, + MessageAccountRecord, + MessageAgentThoughtRecord, + MessageAnnotationHitRecord, + MessageAnnotationRecord, + MessageFileReference, + MessageInputValue, + MessageRating, + MessageRecord, + MessageSource, +) + + +def snapshot_message(*, message: Message, session: Session) -> MessageSource[MessageRecord]: + """Borrow the caller's session and return detached data plus raw file references.""" + return _message_source(record=_message_record(message=message, session=session), session=session) + + +def snapshot_console_message(*, message: Message, session: Session) -> MessageSource[ConsoleMessageRecord]: + """Materialize Console fields without restoring files or signing URLs.""" + return _message_source(record=_console_message_record(message=message, session=session), session=session) + + +def _message_source[RecordT: MessageRecord](*, record: RecordT, session: Session) -> MessageSource[RecordT]: + """Snapshot file references without restoring files or signing URLs.""" + files = session.scalars(select(MessageFile).where(MessageFile.message_id == record.id)) + return MessageSource( + record=record, + files=tuple( + MessageFileReference( + id=file.id, + type=file.type.value, + transfer_method=file.transfer_method.value, + url=file.url, + upload_file_id=file.upload_file_id, + belongs_to=file.belongs_to.value if file.belongs_to is not None else None, + ) + for file in files + ), + ) + + +def _message_record(*, message: Message, session: Session) -> MessageRecord: + # Snapshot stored JSON; restoration and URL signing belong to the query + # service's file adapter after this database session has closed. + inputs = deepcopy(cast(dict[str, MessageInputValue], message._inputs)) + feedback = message.user_feedback_with_session(session=session) + thoughts = [_thought_record(thought) for thought in message.agent_thoughts_with_session(session=session)] + return MessageRecord( + id=message.id, + conversation_id=message.conversation_id, + parent_message_id=message.parent_message_id, + inputs=inputs, + query=message.query, + answer=message.answer, + feedback=MessageRating(rating=feedback.rating.value) if feedback is not None else None, + retriever_resources=cast(list[dict[str, JsonValue]] | None, message.retriever_resources), + created_at=message.created_at, + agent_thoughts=thoughts, + message_files=[], + message_tokens=message.message_tokens, + answer_tokens=message.answer_tokens, + provider_response_latency=message.provider_response_latency, + total_price=message.total_price, + currency=message.currency, + status=message.status.value, + error=message.error, + metadata=cast(JsonValue, message.message_metadata_dict), + ) + + +def _thought_record(thought: MessageAgentThought) -> MessageAgentThoughtRecord: + return MessageAgentThoughtRecord( + id=thought.id, + message_id=thought.message_id, + message_chain_id=thought.message_chain_id, + position=thought.position, + thought=thought.thought, + answer=thought.answer, + tool=thought.tool, + tool_labels=cast(JsonValue, thought.tool_labels), + tool_input=thought.tool_input, + created_at=thought.created_at, + observation=thought.observation, + files=cast(list[str], thought.files), + ) + + +def _account_record(account: Account | None) -> MessageAccountRecord | None: + return MessageAccountRecord(id=account.id, name=account.name, email=account.email) if account else None + + +def _console_message_record(*, message: Message, session: Session) -> ConsoleMessageRecord: + record = _message_record(message=message, session=session) + annotation = message.annotation_with_session(session=session) + annotation_hit = message.annotation_hit_history_with_session(session=session) + return ConsoleMessageRecord( + id=record.id, + conversation_id=record.conversation_id, + parent_message_id=record.parent_message_id, + inputs=record.inputs, + query=record.query, + answer=record.answer, + feedback=record.feedback, + retriever_resources=record.retriever_resources or [], + created_at=record.created_at, + agent_thoughts=record.agent_thoughts, + message_files=record.message_files, + message_tokens=record.message_tokens, + answer_tokens=record.answer_tokens, + provider_response_latency=record.provider_response_latency, + total_price=record.total_price, + currency=record.currency, + status=record.status, + error=record.error, + metadata=record.metadata, + message=deepcopy(cast(JsonValue, message.message)), + from_source=message.from_source, + from_end_user_id=message.from_end_user_id, + from_account_id=message.from_account_id, + workflow_run_id=message.workflow_run_id, + feedbacks=[ + ConsoleMessageFeedbackRecord( + rating=feedback.rating.value, + content=feedback.content, + from_source=feedback.from_source.value, + from_end_user_id=feedback.from_end_user_id, + from_account=_account_record(feedback.from_account_with_session(session=session)), + ) + for feedback in message.feedbacks_with_session(session=session) + ], + annotation=MessageAnnotationRecord( + id=annotation.id, + question=annotation.question, + content=annotation.content, + account=_account_record(annotation.account_with_session(session=session)), + created_at=annotation.created_at, + ) + if annotation + else None, + annotation_hit_history=MessageAnnotationHitRecord( + id=annotation_hit.id, + annotation_create_account=_account_record( + annotation_hit.annotation_create_account_with_session(session=session) + ), + created_at=annotation_hit.created_at, + ) + if annotation_hit + else None, + ) diff --git a/api/repositories/message_repository.py b/api/repositories/message_repository.py index 1d72b2092bc..fca9c9b5ba9 100644 --- a/api/repositories/message_repository.py +++ b/api/repositories/message_repository.py @@ -21,21 +21,14 @@ from sqlalchemy.orm import Session, sessionmaker from core.memory.token_buffer_memory import PreparedHistory, TokenBufferMemory from models import Account, App, AppModelConfig, Conversation, EndUser, Message, MessageFile from models.enums import FeedbackFromSource, FeedbackRating -from models.model import AppMode, InstalledApp, MessageAgentThought, MessageFeedback, load_annotation_reply_config +from models.model import AppMode, InstalledApp, MessageFeedback, load_annotation_reply_config +from repositories.message_projection import snapshot_console_message, snapshot_message from services.app_definition_query_service import AppDefinitionUnavailableError from services.entities.message_entities import ( - ConsoleMessageFeedbackRecord, ConsoleMessageRecord, MessageAccount, - MessageAccountRecord, MessageActor, - MessageAgentThoughtRecord, - MessageAnnotationHitRecord, - MessageAnnotationRecord, - MessageFileReference, - MessageInputValue, MessagePage, - MessageRating, MessageRecord, MessageSource, ) @@ -99,8 +92,7 @@ class MessageRepository: limit=limit, has_more=has_more, data=tuple( - self._to_source(record=self._to_message_record(message=message, session=session), session=session) - for message in reversed(messages[:limit]) + snapshot_message(message=message, session=session) for message in reversed(messages[:limit]) ), ) @@ -146,10 +138,7 @@ class MessageRepository: limit=limit, has_more=has_more, data=tuple( - self._to_source( - record=self._to_console_message_record(message=message, session=session), session=session - ) - for message in reversed(messages) + snapshot_console_message(message=message, session=session) for message in reversed(messages) ), ) @@ -174,27 +163,7 @@ class MessageRepository: message = session.scalar(select(Message).where(Message.id == message_id, Message.app_id == app_id)) if message is None: raise MessageNotExistsError(f"Message {message_id} is unavailable for app {app_id}") - record = self._to_console_message_record(message=message, session=session) - return self._to_source(record=record, session=session) - - @staticmethod - def _to_source[RecordT: MessageRecord](*, record: RecordT, session: Session) -> MessageSource[RecordT]: - """Snapshot file references without restoring files or signing URLs.""" - files = session.scalars(select(MessageFile).where(MessageFile.message_id == record.id)) - return MessageSource( - record=record, - files=tuple( - MessageFileReference( - id=file.id, - type=file.type.value, - transfer_method=file.transfer_method.value, - url=file.url, - upload_file_id=file.upload_file_id, - belongs_to=file.belongs_to.value if file.belongs_to is not None else None, - ) - for file in files - ), - ) + return snapshot_console_message(message=message, session=session) def _require_page_app( self, @@ -657,115 +626,3 @@ class MessageRepository: Message.from_end_user_id == end_user_id, ) ) - - @staticmethod - def _to_message_record(*, message: Message, session: Session) -> MessageRecord: - # Snapshot stored JSON; restoration and URL signing belong to the query - # service's file adapter after this database session has closed. - inputs = deepcopy(cast(dict[str, MessageInputValue], message._inputs)) - feedback = message.user_feedback_with_session(session=session) - thoughts = [ - MessageRepository._thought_record(thought) - for thought in message.agent_thoughts_with_session(session=session) - ] - return MessageRecord( - id=message.id, - conversation_id=message.conversation_id, - parent_message_id=message.parent_message_id, - inputs=inputs, - query=message.query, - answer=message.answer, - feedback=MessageRating(rating=feedback.rating.value) if feedback is not None else None, - retriever_resources=cast(list[dict[str, JsonValue]] | None, message.retriever_resources), - created_at=message.created_at, - agent_thoughts=thoughts, - message_files=[], - message_tokens=message.message_tokens, - answer_tokens=message.answer_tokens, - provider_response_latency=message.provider_response_latency, - total_price=message.total_price, - currency=message.currency, - status=message.status.value, - error=message.error, - metadata=cast(JsonValue, message.message_metadata_dict), - ) - - @staticmethod - def _thought_record(thought: MessageAgentThought) -> MessageAgentThoughtRecord: - return MessageAgentThoughtRecord( - id=thought.id, - message_id=thought.message_id, - message_chain_id=thought.message_chain_id, - position=thought.position, - thought=thought.thought, - answer=thought.answer, - tool=thought.tool, - tool_labels=cast(JsonValue, thought.tool_labels), - tool_input=thought.tool_input, - created_at=thought.created_at, - observation=thought.observation, - files=cast(list[str], thought.files), - ) - - @staticmethod - def _account_record(account: Account | None) -> MessageAccountRecord | None: - return MessageAccountRecord(id=account.id, name=account.name, email=account.email) if account else None - - def _to_console_message_record(self, *, message: Message, session: Session) -> ConsoleMessageRecord: - record = self._to_message_record(message=message, session=session) - annotation = message.annotation_with_session(session=session) - annotation_hit = message.annotation_hit_history_with_session(session=session) - return ConsoleMessageRecord( - id=record.id, - conversation_id=record.conversation_id, - parent_message_id=record.parent_message_id, - inputs=record.inputs, - query=record.query, - answer=record.answer, - feedback=record.feedback, - retriever_resources=record.retriever_resources or [], - created_at=record.created_at, - agent_thoughts=record.agent_thoughts, - message_files=record.message_files, - message_tokens=record.message_tokens, - answer_tokens=record.answer_tokens, - provider_response_latency=record.provider_response_latency, - total_price=record.total_price, - currency=record.currency, - status=record.status, - error=record.error, - metadata=record.metadata, - message=deepcopy(cast(JsonValue, message.message)), - from_source=message.from_source, - from_end_user_id=message.from_end_user_id, - from_account_id=message.from_account_id, - workflow_run_id=message.workflow_run_id, - feedbacks=[ - ConsoleMessageFeedbackRecord( - rating=feedback.rating.value, - content=feedback.content, - from_source=feedback.from_source.value, - from_end_user_id=feedback.from_end_user_id, - from_account=self._account_record(feedback.from_account_with_session(session=session)), - ) - for feedback in message.feedbacks_with_session(session=session) - ], - annotation=MessageAnnotationRecord( - id=annotation.id, - question=annotation.question, - content=annotation.content, - account=self._account_record(annotation.account_with_session(session=session)), - created_at=annotation.created_at, - ) - if annotation - else None, - annotation_hit_history=MessageAnnotationHitRecord( - id=annotation_hit.id, - annotation_create_account=self._account_record( - annotation_hit.annotation_create_account_with_session(session=session) - ), - created_at=annotation_hit.created_at, - ) - if annotation_hit - else None, - ) diff --git a/api/services/conversation_command_service.py b/api/services/conversation_command_service.py index e3f8f935183..7128e1237d6 100644 --- a/api/services/conversation_command_service.py +++ b/api/services/conversation_command_service.py @@ -2,10 +2,21 @@ from typing import Protocol -from services.entities.conversation_entities import ConversationDeletion, ConversationRef +from machinery.context import RequestContext +from services.entities.conversation_entities import ConsoleConversationKind, ConversationDeletion, ConversationRef class ConversationCommandStore(Protocol): + def mark_console_read( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> None: ... + + def delete_console( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> ConversationDeletion: + """Delete the account's Console conversation within the admitted workspace.""" + ... + def delete(self, *, ref: ConversationRef) -> ConversationDeletion: """Commit soft deletion and workspace retirement before returning cleanup data.""" ... @@ -24,6 +35,22 @@ class ConversationCommandService: self._conversations: ConversationCommandStore = conversations self._enqueue_delete_cleanup: ConversationCleanup = enqueue_delete_cleanup + def mark_console_read( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> None: + """Record the legacy Console read receipt after detail rendering succeeds.""" + self._conversations.mark_console_read(context, app_id, conversation_id, kind=kind) + + def delete_console( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> None: + deleted = self._conversations.delete_console(context, app_id, conversation_id, kind=kind) + self._enqueue_delete_cleanup( + tenant_id=deleted.tenant_id, + conversation_id=deleted.conversation_id, + retired_workspace_ids=deleted.retired_workspace_ids, + ) + def delete(self, *, ref: ConversationRef) -> None: deleted = self._conversations.delete(ref=ref) # The retirement and soft-delete commit must precede physical cleanup. diff --git a/api/services/conversation_query_service.py b/api/services/conversation_query_service.py index d606804fb07..cf281b754e6 100644 --- a/api/services/conversation_query_service.py +++ b/api/services/conversation_query_service.py @@ -4,16 +4,38 @@ from collections.abc import Callable, Mapping from dataclasses import replace from typing import Protocol +from machinery.context import RequestContext from services.entities.conversation_entities import ( + ConsoleChatConversationDetail, + ConsoleCompletionConversationDetail, + ConsoleCompletionConversationRecord, + ConsoleConversationDetailSource, + ConsoleConversationFilters, + ConsoleConversationKind, + ConsoleConversationPage, + ConsoleConversationScope, ConversationInputValue, ConversationListSource, ConversationPage, ConversationScope, ConversationSort, ) +from services.message_query_service import MessageFiles class ConversationQueryStore(Protocol): + def get_console_detail( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleConversationDetailSource: ... + + def get_console_scope( + self, context: RequestContext, app_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleConversationScope: ... + + def get_console_page( + self, *, scope: ConsoleConversationScope, filters: ConsoleConversationFilters + ) -> ConsoleConversationPage: ... + def get_page( self, *, @@ -33,9 +55,57 @@ class ConversationInputRestorer(Protocol): class ConversationQueryService: - def __init__(self, *, conversations: ConversationQueryStore, restore_inputs: ConversationInputRestorer) -> None: + def __init__( + self, *, conversations: ConversationQueryStore, restore_inputs: ConversationInputRestorer, files: MessageFiles + ) -> None: self._conversations = conversations self._restore_inputs = restore_inputs + self._files = files + + def get_console_detail( + self, context: RequestContext, app_id: str, conversation_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleCompletionConversationDetail | ConsoleChatConversationDetail: + source = self._conversations.get_console_detail(context, app_id, conversation_id, kind=kind) + record = source.record + if isinstance(record, ConsoleCompletionConversationDetail) and record.first_message is not None: + message = record.first_message + files = self._files.resolve( + tenant_id=source.tenant_id, inputs=message.inputs, answer=message.answer, files=source.files + ) + record = replace( + record, + first_message=replace(message, inputs=files.inputs, answer=files.answer, message_files=files.files), + ) + return record + + def get_console_scope( + self, context: RequestContext, app_id: str, *, kind: ConsoleConversationKind + ) -> ConsoleConversationScope: + return self._conversations.get_console_scope(context, app_id, kind=kind) + + def get_console_page( + self, *, scope: ConsoleConversationScope, filters: ConsoleConversationFilters + ) -> ConsoleConversationPage: + page = self._conversations.get_console_page(scope=scope, filters=filters) + # Completion logs include the first message's file inputs. Restore them + # only after every database-backed response field has been materialized. + return replace( + page, + items=tuple( + replace( + record, + first_message=replace( + record.first_message, + inputs=self._restore_inputs( + inputs=record.first_message.inputs, tenant_resolver=lambda: page.tenant_id + ), + ), + ) + if isinstance(record, ConsoleCompletionConversationRecord) and record.first_message is not None + else record + for record in page.items + ), + ) def get_page( self, diff --git a/api/services/conversation_service.py b/api/services/conversation_service.py index 412a027cd74..890b7208113 100644 --- a/api/services/conversation_service.py +++ b/api/services/conversation_service.py @@ -5,7 +5,6 @@ from sqlalchemy.orm import Session from models import Account from models.model import App, Conversation, EndUser -from repositories.conversation_lifecycle import retire_conversation from services.errors.conversation import ConversationNotExistsError from tasks.collect_agent_resources_task import enqueue_agent_resource_collection from tasks.delete_conversation_task import delete_conversation_related_data @@ -36,31 +35,6 @@ class ConversationService: return conversation - @classmethod - def delete(cls, app_model: App, conversation_id: str, user: Account | EndUser | None, *, session: Session) -> None: - """ - Delete a conversation only if it belongs to the given user and app context. - - Conversation deletion is the product lifecycle boundary for its - Workspace. Physical collection happens only after the retire commit. - - Raises: - ConversationNotExistsError: When the conversation is not visible to the current user. - """ - conversation = cls.get_conversation(app_model, conversation_id, user, session=session) - tenant_id = app_model.tenant_id - try: - retired_workspace_ids = retire_conversation(app_model=app_model, conversation=conversation, session=session) - session.commit() - except Exception: - session.rollback() - raise - cls.enqueue_delete_cleanup( - tenant_id=tenant_id, - conversation_id=conversation_id, - retired_workspace_ids=retired_workspace_ids, - ) - @staticmethod def enqueue_delete_cleanup(*, tenant_id: str, conversation_id: str, retired_workspace_ids: tuple[str, ...]) -> None: """Dispatch resource collection only after the lifecycle transaction commits.""" diff --git a/api/services/entities/conversation_entities.py b/api/services/entities/conversation_entities.py index bf03ac43288..d36bb824203 100644 --- a/api/services/entities/conversation_entities.py +++ b/api/services/entities/conversation_entities.py @@ -7,6 +7,7 @@ from typing import Literal from pydantic import JsonValue from graphon.file import File +from services.entities.message_entities import ConsoleMessageRecord, MessageFileReference from services.installed_app_access_service import InstalledAppRef type ConversationInputValue = JsonValue | File | list[File] @@ -76,3 +77,126 @@ class ConversationDeletion: tenant_id: str conversation_id: str retired_workspace_ids: tuple[str, ...] + + +type ConsoleConversationKind = Literal["completion", "chat"] +type ConversationAnnotationStatus = Literal["annotated", "not_annotated", "all"] + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationScope: + app_id: str + tenant_id: str + app_mode: str + timezone: str + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationFilters: + keyword: str | None + start: datetime | None + end: datetime | None + annotation_status: ConversationAnnotationStatus + sort_by: ConversationSort + page: int + limit: int + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationAccount: + id: str + name: str + email: str + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationAnnotation: + id: str + question: str | None + content: str + account: ConsoleConversationAccount | None + created_at: datetime + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationMessage: + # File reconstruction belongs after the repository's read session closes. + inputs: dict[str, ConversationInputValue] + query: str + message: JsonValue + answer: str + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationRecord: + id: str + status: str + from_source: str + from_end_user_id: str | None + from_end_user_session_id: str | None + from_account_id: str | None + from_account_name: str | None + read_at: datetime | None + created_at: datetime | None + updated_at: datetime | None + model_config: dict[str, JsonValue] + user_feedback_stats: dict[str, int] + admin_feedback_stats: dict[str, int] + + +@dataclass(frozen=True, slots=True) +class ConsoleCompletionConversationRecord(ConsoleConversationRecord): + annotation: ConsoleConversationAnnotation | None + first_message: ConsoleConversationMessage | None + + +@dataclass(frozen=True, slots=True) +class ConsoleChatConversationRecord(ConsoleConversationRecord): + name: str + summary_or_query: str + annotated: bool + message_count: int + status_count: dict[str, int] | None + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationPage: + tenant_id: str + page: int + per_page: int + total: int + has_next: bool + items: tuple[ConsoleCompletionConversationRecord | ConsoleChatConversationRecord, ...] + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationDetail: + id: str + status: str + from_source: str + from_end_user_id: str | None + from_account_id: str | None + created_at: datetime | None + model_config: dict[str, JsonValue] + + +@dataclass(frozen=True, slots=True) +class ConsoleChatConversationDetail(ConsoleConversationDetail): + updated_at: datetime | None + annotated: bool + introduction: str | None + message_count: int + user_feedback_stats: dict[str, int] + admin_feedback_stats: dict[str, int] + + +@dataclass(frozen=True, slots=True) +class ConsoleCompletionConversationDetail(ConsoleConversationDetail): + first_message: ConsoleMessageRecord | None + + +@dataclass(frozen=True, slots=True) +class ConsoleConversationDetailSource: + tenant_id: str + record: ConsoleChatConversationDetail | ConsoleCompletionConversationDetail + files: tuple[MessageFileReference, ...] diff --git a/api/tests/e2e_tests/containers/controllers/console/app/test_conversation_read_timestamp.py b/api/tests/e2e_tests/containers/controllers/console/app/test_conversation_read_timestamp.py index 0559f14698f..7290a6f08a1 100644 --- a/api/tests/e2e_tests/containers/controllers/console/app/test_conversation_read_timestamp.py +++ b/api/tests/e2e_tests/containers/controllers/console/app/test_conversation_read_timestamp.py @@ -1,31 +1,34 @@ +"""Execute Console read-marker writes against PostgreSQL without replacing the clock.""" + from datetime import datetime -from unittest.mock import patch +from uuid import uuid4 import pytest -from sqlalchemy.orm import Session -from werkzeug.exceptions import NotFound +from sqlalchemy.orm import Session, sessionmaker -from controllers.console.app.conversation import _get_conversation -from models.enums import ConversationFromSource +from libs.datetime_utils import naive_utc_now +from machinery.context import RequestContext +from models.enums import ConversationFromSource, ConversationStatus from models.model import AppMode, Conversation +from repositories.conversation_repository import ConversationRepository +from services.errors.conversation import ConversationNotExistsError from tests.e2e_tests.containers.controllers.console.helpers import ( create_console_account_and_tenant, create_console_app, ) -def test_get_conversation_mark_read_keeps_updated_at_unchanged( +def test_console_detail_then_mark_preserves_updated_at_and_the_first_reader( db_session_with_containers: Session, -): +) -> None: account, tenant = create_console_account_and_tenant(db_session_with_containers) app = create_console_app(db_session_with_containers, tenant.id, account.id, AppMode.CHAT) - - original_updated_at = datetime(2026, 2, 8, 0, 0, 0) + original_updated_at = datetime(2020, 2, 8) conversation = Conversation( app_id=app.id, name="read timestamp test", _inputs={}, - status="normal", + status=ConversationStatus.NORMAL, mode=AppMode.CHAT, from_source=ConversationFromSource.CONSOLE, from_account_id=account.id, @@ -33,29 +36,70 @@ def test_get_conversation_mark_read_keeps_updated_at_unchanged( conversation.updated_at = original_updated_at db_session_with_containers.add(conversation) db_session_with_containers.commit() + context = RequestContext( + request_id="postgres-read", trace_id=None, account_id=account.id, active_workspace_id=tenant.id + ) + app_id, conversation_id = app.id, conversation.id + sessions = sessionmaker(bind=db_session_with_containers.get_bind(), expire_on_commit=False) + db_session_with_containers.rollback() + repository = ConversationRepository(session_factory=sessions) - read_at = datetime(2026, 2, 9, 0, 0, 0) + loaded = repository.get_console_detail(context, app_id, conversation_id, kind="chat") + assert loaded.record.id == conversation_id + with sessions() as session: + stored = session.get_one(Conversation, conversation_id) + assert stored.read_at is None + assert stored.read_account_id is None + assert stored.updated_at == original_updated_at - with patch( - "controllers.console.app.conversation.naive_utc_now", - return_value=read_at, - autospec=True, - ): - loaded = _get_conversation(db_session_with_containers, account, app, conversation.id) + before = naive_utc_now() + repository.mark_console_read(context, app_id, conversation_id, kind="chat") + after = naive_utc_now() + with sessions() as session: + stored = session.get_one(Conversation, conversation_id) + first_read_at = stored.read_at + assert first_read_at is not None + assert before <= first_read_at <= after + assert stored.read_account_id == context.account_id + assert stored.updated_at == original_updated_at - db_session_with_containers.refresh(conversation) - - assert loaded.id == conversation.id - assert conversation.read_at == read_at - assert conversation.read_account_id == account.id - assert conversation.updated_at == original_updated_at + second_reader = context._replace(account_id=str(uuid4())) + repository.mark_console_read(second_reader, app_id, conversation_id, kind="chat") + with sessions() as session: + stored = session.get_one(Conversation, conversation_id) + assert stored.read_at == first_read_at + assert stored.read_account_id == context.account_id + assert stored.updated_at == original_updated_at -def test_get_conversation_raises_not_found_for_missing_conversation( +def test_console_missing_detail_or_mark_does_not_mark_another_conversation( db_session_with_containers: Session, -): +) -> None: account, tenant = create_console_account_and_tenant(db_session_with_containers) app = create_console_app(db_session_with_containers, tenant.id, account.id, AppMode.CHAT) - - with pytest.raises(NotFound): - _get_conversation(db_session_with_containers, account, app, "00000000-0000-0000-0000-000000000000") + conversation = Conversation( + app_id=app.id, + name="Untouched conversation", + _inputs={}, + mode=AppMode.CHAT, + from_source=ConversationFromSource.CONSOLE, + from_account_id=account.id, + ) + db_session_with_containers.add(conversation) + db_session_with_containers.commit() + context = RequestContext( + request_id="postgres-read", trace_id=None, account_id=account.id, active_workspace_id=tenant.id + ) + app_id, conversation_id = app.id, conversation.id + sessions = sessionmaker(bind=db_session_with_containers.get_bind(), expire_on_commit=False) + db_session_with_containers.rollback() + repository = ConversationRepository(session_factory=sessions) + missing_id = str(uuid4()) + with pytest.raises(ConversationNotExistsError): + repository.get_console_detail(context, app_id, missing_id, kind="chat") + with pytest.raises(ConversationNotExistsError): + repository.mark_console_read(context, app_id, missing_id, kind="chat") + with sessions() as session: + stored = session.get_one(Conversation, conversation_id) + assert stored.read_at is None + assert stored.read_account_id is None diff --git a/api/tests/e2e_tests/containers/services/test_conversation_service.py b/api/tests/e2e_tests/containers/services/test_conversation_service.py index 1838195d656..a02a577d246 100644 --- a/api/tests/e2e_tests/containers/services/test_conversation_service.py +++ b/api/tests/e2e_tests/containers/services/test_conversation_service.py @@ -2,11 +2,9 @@ from __future__ import annotations from datetime import datetime from decimal import Decimal -from unittest.mock import patch from uuid import uuid4 import pytest -from sqlalchemy import select from sqlalchemy.orm import Session from core.app.entities.app_invoke_entities import InvokeFrom @@ -234,112 +232,3 @@ class TestConversationServiceRetrieval: # Assert assert result == conversation - - @patch("services.conversation_service.delete_conversation_related_data") - def test_delete_conversation(self, mock_delete_task, db_session_with_containers: Session): - """ - Test conversation deletion with async cleanup. - - Deletion is a two-step process: - 1. Immediately hide the conversation with a durable soft-delete marker - 2. Trigger async background task to clean up related data - (messages, annotations, vector embeddings, file uploads) - """ - # Arrange - Set up test data - app_model, user = ConversationServiceIntegrationTestDataFactory.create_app_and_account( - db_session_with_containers - ) - conversation = ConversationServiceIntegrationTestDataFactory.create_conversation( - db_session_with_containers, - app_model, - user, - ) - conversation_id = conversation.id - - # Act - Delete the conversation - ConversationService.delete( - app_model=app_model, conversation_id=conversation_id, user=user, session=db_session_with_containers - ) - - # Assert - Verify two-step deletion process - # Step 1: Immediate logical deletion - deleted = db_session_with_containers.scalar(select(Conversation).where(Conversation.id == conversation_id)) - assert deleted is not None - assert deleted.is_deleted is True - - # Step 2: Async cleanup task triggered - # The Celery task will handle cleanup of messages, annotations, etc. - mock_delete_task.delay.assert_called_once_with(conversation_id) - - @patch("services.conversation_service.delete_conversation_related_data") - def test_delete_conversation_not_owned_by_account( - self, - mock_delete_task, - db_session_with_containers: Session, - ): - """ - Test deletion is denied when conversation belongs to a different account. - """ - # Arrange - app_model, owner_account = ConversationServiceIntegrationTestDataFactory.create_app_and_account( - db_session_with_containers - ) - _, other_account = ConversationServiceIntegrationTestDataFactory.create_app_and_account( - db_session_with_containers - ) - conversation = ConversationServiceIntegrationTestDataFactory.create_conversation( - db_session_with_containers, - app_model, - owner_account, - ) - - # Act & Assert - with pytest.raises(ConversationNotExistsError): - ConversationService.delete( - app_model=app_model, - conversation_id=conversation.id, - user=other_account, - session=db_session_with_containers, - ) - - # Verify no deletion and no async cleanup trigger - not_deleted = db_session_with_containers.scalar(select(Conversation).where(Conversation.id == conversation.id)) - assert not_deleted is not None - mock_delete_task.delay.assert_not_called() - - @patch("services.conversation_service.delete_conversation_related_data") - def test_delete_handles_exception_and_rollback( - self, - mock_delete_task, - db_session_with_containers: Session, - ): - """ - Test that delete propagates exceptions and does not trigger the cleanup task. - - When a DB error occurs during deletion, the conversation row stays in place. - """ - # Arrange - app_model, user = ConversationServiceIntegrationTestDataFactory.create_app_and_account( - db_session_with_containers - ) - conversation = ConversationServiceIntegrationTestDataFactory.create_conversation( - db_session_with_containers, app_model, user - ) - conversation_id = conversation.id - - # Act — force an error during the soft-delete commit to exercise rollback - with patch.object(db_session_with_containers, "commit", side_effect=Exception("DB error")): - with pytest.raises(Exception, match="DB error"): - ConversationService.delete( - app_model=app_model, - conversation_id=conversation_id, - user=user, - session=db_session_with_containers, - ) - - # Assert — related-data deletion is not scheduled. - mock_delete_task.delay.assert_not_called() - - # Conversation is still present because the deletion was never committed - still_there = db_session_with_containers.scalar(select(Conversation).where(Conversation.id == conversation_id)) - assert still_there is not None diff --git a/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_delete.py b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_delete.py new file mode 100644 index 00000000000..0a7b5d8045a --- /dev/null +++ b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_delete.py @@ -0,0 +1,337 @@ +"""Console conversation deletion through real account admission and SQLite.""" + +from collections.abc import Callable, Iterator +from dataclasses import dataclass, field +from http import HTTPStatus +from typing import Literal +from uuid import uuid4 + +import pytest +from flask import Flask +from sqlalchemy import Connection, Engine, event, select, update +from sqlalchemy.orm import Session, SessionTransaction, sessionmaker +from werkzeug.test import TestResponse + +from constants import COOKIE_NAME_CSRF_TOKEN, HEADER_NAME_CSRF_TOKEN +from controllers.common.rbac import RBAC_CHECKS_ATTR, PlainApp, RBACCheck, RBACPermission +from controllers.console.app.conversation import ChatConversationDetailApi, CompletionConversationDetailApi +from enums import DeploymentEdition +from enums.account import TenantAccountRole +from extensions.ext_database import db +from extensions.ext_login import DifyLoginManager, unauthorized_handler +from libs.external_api import ExternalApi +from libs.token import generate_csrf_token +from models import Account, Tenant, TenantAccountJoin +from models.agent import AgentWorkingResourceStatus, AgentWorkspace, AgentWorkspaceOwnerType +from models.enums import ConversationFromSource +from models.model import App, AppMode, Conversation +from repositories.conversation_repository import ConversationRepository +from services.conversation_command_service import ConversationCommandService +from tests.helpers.unit.model_factories import make_account, make_app, make_conversation + +type _Kind = Literal["chat", "completion"] + + +@dataclass(frozen=True) +class _Services: + conversation_commands: ConversationCommandService + + +@dataclass +class _Harness: + app: Flask + factory: sessionmaker[Session] + account: Account + tenant: Tenant + target: App + conversation: Conversation + sessions: list[Session] = field(default_factory=list) + cleanups: list[tuple[str, str, tuple[str, ...]]] = field(default_factory=list) + + def delete( + self, + kind: _Kind, + *, + app_id: str | None = None, + conversation_id: str | None = None, + authenticated: bool = True, + csrf: bool = True, + ) -> TestResponse: + client = self.app.test_client() + headers: dict[str, str] = {} + if authenticated: + with client.session_transaction() as session: + session["_user_id"] = self.account.id + session["_fresh"] = True + if csrf: + token = generate_csrf_token(self.account.id) + client.set_cookie(COOKIE_NAME_CSRF_TOKEN, token) + headers[HEADER_NAME_CSRF_TOKEN] = token + response = client.delete( + f"/apps/{app_id or self.target.id}/{kind}-conversations/{conversation_id or self.conversation.id}", + headers=headers, + ) + self.assert_closed() + assert response.content_type == "application/json" + return response + + def assert_closed(self) -> None: + assert all(not session.in_transaction() and not session.identity_map for session in self.sessions) + + def mode(self, mode: AppMode) -> None: + with self.factory.begin() as session: + session.execute(update(App).where(App.id == self.target.id).values(mode=mode)) + self.target.mode = mode + + def assert_unchanged(self, *, previously_deleted: bool = False) -> None: + with self.factory() as session: + stored = session.get(Conversation, self.conversation.id) + assert stored is not None + assert stored.is_deleted is previously_deleted + assert stored.read_at is None + assert stored.read_account_id is None + assert not self.cleanups + + +@pytest.fixture +def harness( + sqlite_engine: Engine, sqlite_session_factory: sessionmaker[Session], config_overrides: Callable[..., None] +) -> Iterator[_Harness]: + config_overrides( + DEPLOYMENT_EDITION=DeploymentEdition.CLOUD, + LOGIN_DISABLED=False, + RBAC_ENABLED=False, + SECRET_KEY="conversation-http-tests-key-at-least-32-bytes", + CONSOLE_WEB_URL="http://localhost", + CONSOLE_API_URL="http://localhost", + COOKIE_DOMAIN="", + ) + tenant = Tenant(name="Console workspace") + account = make_account(account_id=str(uuid4()), timezone="UTC") + target = make_app(app_id=str(uuid4()), tenant_id=tenant.id) + conversation = make_conversation( + conversation_id=str(uuid4()), + app_id=target.id, + inputs={}, + from_source=ConversationFromSource.CONSOLE, + from_account_id=account.id, + is_deleted=False, + ) + with sqlite_session_factory.begin() as session: + session.add_all( + [ + tenant, + account, + target, + conversation, + TenantAccountJoin(tenant_id=tenant.id, account_id=account.id, role=TenantAccountRole.OWNER), + ] + ) + app = Flask(__name__) + app.config.update( + TESTING=True, + RESTX_ERROR_404_HELP=False, + SECRET_KEY="console-test-session", + SQLALCHEMY_DATABASE_URI=str(sqlite_engine.url), + ) + db.init_app(app) + login_manager = DifyLoginManager() + login_manager.init_app(app) + login_manager.unauthorized_handler(unauthorized_handler) + state = _Harness(app, sqlite_session_factory, account, tenant, target, conversation) + + @login_manager.user_loader + def load_account(account_id: str) -> Account | None: + with sqlite_session_factory() as session: + loaded = session.get(Account, account_id) + if loaded is not None: + loaded.set_tenant_id_with_session(tenant.id, session=session) + return loaded + + factory = sessionmaker(bind=sqlite_engine, expire_on_commit=False) + + @event.listens_for(factory, "after_begin") + def record_session(session: Session, _transaction: SessionTransaction, _connection: Connection) -> None: + state.assert_closed() + state.sessions.append(session) + + def enqueue_cleanup(*, tenant_id: str, conversation_id: str, retired_workspace_ids: tuple[str, ...]) -> None: + # Cleanup crosses an external queue boundary only after the real command + # transaction has closed and its committed deletion is visible. + state.assert_closed() + with sqlite_session_factory() as session: + stored = session.get(Conversation, conversation_id) + assert stored is not None + assert stored.is_deleted + for workspace_id in retired_workspace_ids: + workspace = session.get(AgentWorkspace, workspace_id) + assert workspace is not None + assert workspace.status == AgentWorkingResourceStatus.RETIRED + assert workspace.active_guard is None + assert workspace.retired_at is not None + state.cleanups.append((tenant_id, conversation_id, retired_workspace_ids)) + + app.extensions["application_services"] = _Services( + ConversationCommandService( + conversations=ConversationRepository(session_factory=factory), enqueue_delete_cleanup=enqueue_cleanup + ) + ) + api = ExternalApi(app) + api.add_resource(ChatConversationDetailApi, "/apps//chat-conversations/") + api.add_resource( + CompletionConversationDetailApi, "/apps//completion-conversations/" + ) + yield state + with app.app_context(): + db.session.remove() + db.engine.dispose() + + +@pytest.mark.parametrize( + ("kind", "mode"), + [ + ("completion", AppMode.COMPLETION), + ("chat", AppMode.CHAT), + ("chat", AppMode.AGENT_CHAT), + ("chat", AppMode.ADVANCED_CHAT), + ("chat", AppMode.AGENT), + ], +) +def test_delete_commits_retirement_before_cleanup_and_returns_empty_204( + harness: _Harness, kind: _Kind, mode: AppMode +) -> None: + harness.mode(mode) + with harness.factory.begin() as session: + workspace = AgentWorkspace( + id=str(uuid4()), + tenant_id=harness.target.tenant_id, + app_id=harness.target.id, + owner_type=AgentWorkspaceOwnerType.CONVERSATION, + owner_id=harness.conversation.id, + owner_scope_key="chatflow-node", + backend_workspace_ref="backend-workspace", + status=AgentWorkingResourceStatus.ACTIVE, + active_guard=1, + ) + session.add(workspace) + + response = harness.delete(kind) + + assert response.status_code == HTTPStatus.NO_CONTENT + assert response.data == b"" + assert "Content-Length" not in response.headers + assert harness.cleanups == [(harness.tenant.id, harness.conversation.id, (workspace.id,))] + assert harness.sessions + with harness.factory() as session: + stored = session.get(Conversation, harness.conversation.id) + assert stored is not None + assert stored.read_at is None + assert stored.read_account_id is None + + repeated = harness.delete(kind) + assert repeated.status_code == HTTPStatus.NOT_FOUND + assert repeated.get_json()["code"] == "conversation_not_found" + assert len(harness.cleanups) == 1 + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("field", ["app_id", "from_source", "from_account_id", "from_end_user_id", "is_deleted"]) +def test_delete_cannot_target_another_conversation_owner(harness: _Harness, kind: _Kind, field: str) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + value: str | bool + if field == "is_deleted": + value = True + elif field == "from_source": + value = ConversationFromSource.API + elif field == "from_end_user_id": + # Account and EndUser IDs remain different owner namespaces even if equal. + value = harness.account.id + else: + value = str(uuid4()) + with harness.factory.begin() as session: + session.execute(update(Conversation).where(Conversation.id == harness.conversation.id).values({field: value})) + response = harness.delete(kind) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "conversation_not_found" + harness.assert_unchanged(previously_deleted=field == "is_deleted") + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_missing_conversation_does_not_delete_existing_conversation(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + response = harness.delete(kind, conversation_id=str(uuid4())) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "conversation_not_found" + harness.assert_unchanged() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("unavailable", ["missing", "other_tenant"]) +def test_app_scope_is_checked_before_delete(harness: _Harness, kind: _Kind, unavailable: str) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with harness.factory.begin() as session: + target = session.get(App, harness.target.id) + assert target is not None + if unavailable == "missing": + session.delete(target) + else: + target.tenant_id = str(uuid4()) + response = harness.delete(kind) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "app_not_found" + harness.assert_unchanged() + + +@pytest.mark.parametrize( + ("kind", "mode"), + [("chat", AppMode.COMPLETION), ("chat", AppMode.WORKFLOW), ("completion", AppMode.CHAT)], +) +def test_unsupported_app_mode_cannot_delete_conversation(harness: _Harness, kind: _Kind, mode: AppMode) -> None: + harness.mode(mode) + response = harness.delete(kind) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "app_not_found" + harness.assert_unchanged() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize( + "role", + [ + TenantAccountRole.OWNER, + TenantAccountRole.ADMIN, + TenantAccountRole.EDITOR, + TenantAccountRole.NORMAL, + TenantAccountRole.DATASET_OPERATOR, + ], +) +def test_existing_edit_roles_are_preserved(harness: _Harness, kind: _Kind, role: TenantAccountRole) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with harness.factory.begin() as session: + membership = session.scalar(select(TenantAccountJoin).where(TenantAccountJoin.account_id == harness.account.id)) + assert membership is not None + membership.role = role + response = harness.delete(kind) + if TenantAccountRole.is_editing_role(role): + assert response.status_code == HTTPStatus.NO_CONTENT + assert len(harness.cleanups) == 1 + else: + assert response.status_code == HTTPStatus.FORBIDDEN + harness.assert_unchanged() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_login_and_csrf_remain_required(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + assert harness.delete(kind, authenticated=False).status_code == HTTPStatus.UNAUTHORIZED + assert harness.delete(kind, csrf=False).status_code == HTTPStatus.UNAUTHORIZED + harness.assert_unchanged() + + +@pytest.mark.parametrize("handler", [ChatConversationDetailApi.delete, CompletionConversationDetailApi.delete]) +def test_app_edit_rbac_check_is_declared(handler: Callable[..., object]) -> None: + checks: tuple[RBACCheck, ...] = vars(handler)[RBAC_CHECKS_ATTR] + assert len(checks) == 1 + assert checks[0].scene == RBACPermission.APP_EDIT + assert isinstance(checks[0].locator, PlainApp) + assert checks[0].locator.param == "app_id" diff --git a/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_detail.py b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_detail.py new file mode 100644 index 00000000000..5365f72d9f1 --- /dev/null +++ b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_detail.py @@ -0,0 +1,477 @@ +"""Console detail reads use real admission, queries, rendering and read receipts.""" + +import json +from collections.abc import Callable +from dataclasses import dataclass +from datetime import datetime, timedelta +from decimal import Decimal +from http import HTTPStatus +from typing import Literal, cast +from uuid import uuid4 + +import pytest +from sqlalchemy import Connection, event, select, text, update +from sqlalchemy.orm import Session, SessionTransaction, sessionmaker +from werkzeug.test import TestResponse + +from constants import COOKIE_NAME_CSRF_TOKEN, HEADER_NAME_CSRF_TOKEN +from controllers.common.rbac import RBAC_CHECKS_ATTR, PlainApp, RBACCheck, RBACPermission +from controllers.console.app.conversation import ChatConversationDetailApi, CompletionConversationDetailApi +from enums.account import TenantAccountRole +from fields.conversation_fields import ConversationDetail, ConversationMessageDetail, ConversationResponseSource +from libs.datetime_utils import naive_utc_now +from libs.helper import dump_response, to_timestamp +from libs.token import generate_csrf_token +from models import TenantAccountJoin +from models.agent import Agent, AgentScope, AgentSource, AgentStatus +from models.enums import ConversationFromSource, CreatorUserRole, FeedbackFromSource, FeedbackRating +from models.model import ( + App, + AppAnnotationHitHistory, + AppMode, + Conversation, + Message, + MessageAgentThought, + MessageAnnotation, + MessageFeedback, +) +from models.utils.file_input_compat import restore_input_files +from repositories.conversation_repository import ConversationRepository +from services.conversation_command_service import ConversationCommandService +from services.conversation_query_service import ConversationQueryService +from services.message_query_adapters import MessageFileResolver +from tests.e2e_tests.contracts.controllers.console.app import test_conversation_delete as delete_tests +from tests.e2e_tests.contracts.controllers.console.app.test_conversation_delete import _Harness as DeleteHarness +from tests.helpers.unit.model_factories import make_account, make_message + +type _Kind = Literal["chat", "completion"] +_UPDATED_AT = datetime(2024, 2, 3, 4, 5, 6) +delete_harness = delete_tests.harness + + +@dataclass(frozen=True) +class _Services: + conversation_commands: ConversationCommandService + conversation_queries: ConversationQueryService + + +@dataclass(frozen=True) +class _Harness: + source: DeleteHarness + + def get( + self, + kind: _Kind, + *, + account_id: str | None = None, + conversation_id: str | None = None, + authenticated: bool = True, + csrf: bool = True, + ) -> TestResponse: + source = self.source + client = source.app.test_client() + actor = account_id or source.account.id + headers: dict[str, str] = {} + if authenticated: + with client.session_transaction() as session: + session["_user_id"] = actor + session["_fresh"] = True + if csrf: + token = generate_csrf_token(actor) + client.set_cookie(COOKIE_NAME_CSRF_TOKEN, token) + headers[HEADER_NAME_CSRF_TOKEN] = token + response = client.get( + f"/apps/{source.target.id}/{kind}-conversations/{conversation_id or source.conversation.id}", + headers=headers, + ) + source.assert_closed() + assert response.content_type == "application/json" + assert int(response.headers["Content-Length"]) == len(response.data) + assert not source.cleanups + return response + + def assert_unread(self) -> None: + with self.source.factory() as session: + conversation = session.get_one(Conversation, self.source.conversation.id) + assert conversation.read_at is None + assert conversation.read_account_id is None + assert conversation.updated_at == _UPDATED_AT + + +@pytest.fixture +def harness(delete_harness: DeleteHarness) -> _Harness: + previous = cast(delete_tests._Services, delete_harness.app.extensions["application_services"]) + factory = sessionmaker(bind=delete_harness.factory.kw["bind"], expire_on_commit=False) + + @event.listens_for(factory, "after_begin") + def record_session(session: Session, _transaction: SessionTransaction, _connection: Connection) -> None: + delete_harness.assert_closed() + delete_harness.sessions.append(session) + + delete_harness.app.extensions["application_services"] = _Services( + conversation_commands=previous.conversation_commands, + conversation_queries=ConversationQueryService( + conversations=ConversationRepository(session_factory=factory), + restore_inputs=restore_input_files, + files=MessageFileResolver(), + ), + ) + with delete_harness.factory.begin() as session: + session.execute( + update(Conversation) + .where(Conversation.id == delete_harness.conversation.id) + .values({Conversation.updated_at: _UPDATED_AT, Conversation.introduction: "Welcome"}) + ) + return _Harness(delete_harness) + + +def _message(harness: _Harness, *, created_at: datetime = _UPDATED_AT) -> Message: + source = harness.source + message = make_message( + message_id=str(uuid4()), + app_id=source.target.id, + conversation_id=source.conversation.id, + inputs={"empty": "", "zero": 0, "false": False, "null": None, "list": []}, + query="The first question", + message=[{"role": "user", "text": "The prompt"}], + answer="The answer", + message_tokens=7, + answer_tokens=13, + provider_response_latency=1.25, + message_unit_price=Decimal(0), + answer_unit_price=Decimal(0), + total_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.API, + from_account_id=source.account.id, + created_at=created_at, + updated_at=created_at, + ) + message.message_metadata = '{"usage":{"total_tokens":20}}' + with source.factory.begin() as session: + session.add(message) + return message + + +@pytest.mark.parametrize( + ("kind", "mode"), + [ + ("completion", AppMode.COMPLETION), + ("chat", AppMode.CHAT), + ("chat", AppMode.AGENT_CHAT), + ("chat", AppMode.ADVANCED_CHAT), + ("chat", AppMode.AGENT), + ], +) +def test_empty_details_keep_response_and_mark_first_read_without_touching_updated_at( + harness: _Harness, kind: _Kind, mode: AppMode +) -> None: + source = harness.source + source.mode(mode) + with source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + schema = ConversationMessageDetail if kind == "completion" else ConversationDetail + expected = dump_response(schema, ConversationResponseSource(stored, session=session)) + before = naive_utc_now() + response = harness.get(kind) + after = naive_utc_now() + assert response.status_code == HTTPStatus.OK, response.get_json() + assert response.get_json() == expected + assert "read_at" not in response.get_json() + if kind == "completion": + assert response.get_json()["message"] is None + else: + assert response.get_json()["message_count"] == 0 + assert response.get_json()["updated_at"] == to_timestamp(_UPDATED_AT) + with source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + assert stored.read_at is not None + assert before <= stored.read_at <= after + assert stored.read_account_id == source.account.id + assert stored.updated_at == _UPDATED_AT + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("scope", ["other_account", "end_user", "soft_deleted"]) +def test_management_detail_preserves_app_wide_visibility(harness: _Harness, kind: _Kind, scope: str) -> None: + source = harness.source + source.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with source.factory.begin() as session: + conversation = session.get_one(Conversation, source.conversation.id) + if scope == "other_account": + conversation.from_account_id = str(uuid4()) + elif scope == "end_user": + conversation.from_source = ConversationFromSource.API + conversation.from_account_id = None + conversation.from_end_user_id = str(uuid4()) + else: + conversation.is_deleted = True + conversation.updated_at = _UPDATED_AT + response = harness.get(kind) + assert response.status_code == HTTPStatus.OK, response.get_json() + assert response.get_json()["id"] == source.conversation.id + with source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + assert stored.is_deleted is (scope == "soft_deleted") + assert stored.read_account_id == source.account.id + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_second_reader_does_not_replace_first_read_receipt(harness: _Harness, kind: _Kind) -> None: + source = harness.source + source.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + assert harness.get(kind).status_code == HTTPStatus.OK + with source.factory.begin() as session: + stored = session.get_one(Conversation, source.conversation.id) + first_read_at = stored.read_at + second = make_account(account_id=str(uuid4()), name="Second reader") + session.add_all( + [ + second, + TenantAccountJoin(tenant_id=source.tenant.id, account_id=second.id, role=TenantAccountRole.EDITOR), + ] + ) + assert harness.get(kind, account_id=second.id).status_code == HTTPStatus.OK + with source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + assert stored.read_at == first_read_at + assert stored.read_account_id == source.account.id + assert stored.updated_at == _UPDATED_AT + + +def test_completion_detail_preserves_nested_message_projection(harness: _Harness) -> None: + source = harness.source + source.mode(AppMode.COMPLETION) + first = _message(harness) + _message(harness, created_at=_UPDATED_AT + timedelta(seconds=1)) + with source.factory.begin() as session: + stored = session.get_one(Conversation, source.conversation.id) + stored.override_model_configs = json.dumps({"model": {"name": "fixture-model"}, "pre_prompt": "Be useful"}) + session.get_one(Message, first.id).message_metadata = json.dumps( + {"usage": {"total_tokens": 20}, "retriever_resources": []} + ) + stored.updated_at = _UPDATED_AT + annotation = MessageAnnotation( + app_id=source.target.id, + conversation_id=source.conversation.id, + message_id=first.id, + account_id=source.account.id, + question="Question", + content="Annotated answer", + ) + session.add_all( + [ + annotation, + MessageFeedback( + app_id=source.target.id, + conversation_id=source.conversation.id, + message_id=first.id, + rating=FeedbackRating.LIKE, + from_source=FeedbackFromSource.USER, + from_account_id=source.account.id, + content="Helpful", + ), + AppAnnotationHitHistory( + app_id=source.target.id, + annotation_id=annotation.id, + source="annotation", + question="Question", + account_id=source.account.id, + score=0.8, + message_id=first.id, + annotation_question="Question", + annotation_content="Annotated answer", + ), + MessageAgentThought( + message_id=first.id, + position=2, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=source.account.id, + thought="Second thought", + tool_labels_str="{}", + ), + MessageAgentThought( + message_id=first.id, + position=1, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=source.account.id, + thought="First thought", + tool="search", + tool_labels_str='{"search":"Search"}', + message_files='["file-1"]', + message_chain_id=str(uuid4()), + ), + ] + ) + with source.app.app_context(), source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + expected = dump_response(ConversationMessageDetail, ConversationResponseSource(stored, session=session)) + response = harness.get("completion") + assert response.status_code == HTTPStatus.OK, response.get_json() + actual = response.get_json() + assert actual == expected + message = actual["message"] + assert message["id"] == first.id + assert message["inputs"] == {"empty": "", "zero": 0, "false": False, "null": None, "list": []} + assert message["metadata"] == {"usage": {"total_tokens": 20}, "retriever_resources": []} + assert message["annotation_hit_history"]["annotation_id"] == annotation.id + assert message["feedbacks"][0]["from_account"]["id"] == source.account.id + assert [thought["thought"] for thought in message["agent_thoughts"]] == ["First thought", "Second thought"] + assert message["agent_thoughts"][0]["chain_id"] is not None + assert message["message_files"] == [] + + +def test_chat_detail_preserves_annotations_and_feedback_counts(harness: _Harness) -> None: + source = harness.source + first = _message(harness) + _message(harness, created_at=_UPDATED_AT + timedelta(seconds=1)) + with source.factory.begin() as session: + session.add_all( + [ + MessageAnnotation( + app_id=source.target.id, + conversation_id=source.conversation.id, + message_id=first.id, + account_id=source.account.id, + question="Question", + content="Annotated answer", + ), + *[ + MessageFeedback( + app_id=source.target.id, + conversation_id=source.conversation.id, + message_id=first.id, + rating=rating, + from_source=feedback_source, + ) + for feedback_source, rating in [ + (FeedbackFromSource.USER, FeedbackRating.LIKE), + (FeedbackFromSource.USER, FeedbackRating.DISLIKE), + (FeedbackFromSource.ADMIN, FeedbackRating.LIKE), + ] + ], + ] + ) + with source.factory() as session: + stored = session.get_one(Conversation, source.conversation.id) + expected = dump_response(ConversationDetail, ConversationResponseSource(stored, session=session)) + response = harness.get("chat") + assert response.status_code == HTTPStatus.OK + assert response.get_json() == expected + assert response.get_json()["annotated"] is True + assert response.get_json()["message_count"] == 2 + assert response.get_json()["user_feedback_stats"] == {"like": 1, "dislike": 1} + assert response.get_json()["admin_feedback_stats"] == {"like": 1, "dislike": 0} + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("gate", ["missing", "tenant", "status", "hidden_agent", "mode"]) +def test_unavailable_app_cannot_be_read_or_marked(harness: _Harness, kind: _Kind, gate: str) -> None: + source = harness.source + source.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with source.factory.begin() as session: + app = session.get_one(App, source.target.id) + if gate == "missing": + session.delete(app) + elif gate == "tenant": + app.tenant_id = str(uuid4()) + elif gate == "status": + session.execute(text("UPDATE apps SET status = 'disabled' WHERE id = :id"), {"id": app.id}) + elif gate == "mode": + app.mode = AppMode.WORKFLOW + else: + app.mode = AppMode.AGENT + session.add( + Agent( + tenant_id=app.tenant_id, + name="Hidden", + scope=AgentScope.WORKFLOW_ONLY, + source=AgentSource.WORKFLOW, + backing_app_id=app.id, + status=AgentStatus.ARCHIVED, + ) + ) + response = harness.get(kind) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "app_not_found" + harness.assert_unread() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("change", ["missing", "other_app"]) +def test_missing_or_other_app_conversation_does_not_mark_existing_row( + harness: _Harness, kind: _Kind, change: str +) -> None: + source = harness.source + source.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + conversation_id = str(uuid4()) if change == "missing" else source.conversation.id + if change == "other_app": + with source.factory.begin() as session: + session.execute( + update(Conversation) + .where(Conversation.id == conversation_id) + .values({Conversation.app_id: str(uuid4()), Conversation.updated_at: _UPDATED_AT}) + ) + response = harness.get(kind, conversation_id=conversation_id) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.get_json()["code"] == "conversation_not_found" + harness.assert_unread() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("role", list(TenantAccountRole)) +def test_detail_preserves_edit_roles(harness: _Harness, kind: _Kind, role: TenantAccountRole) -> None: + source = harness.source + source.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with source.factory.begin() as session: + membership = session.scalar(select(TenantAccountJoin).where(TenantAccountJoin.account_id == source.account.id)) + assert membership is not None + membership.role = role + response = harness.get(kind) + if TenantAccountRole.is_editing_role(role): + assert response.status_code == HTTPStatus.OK + else: + assert response.status_code == HTTPStatus.FORBIDDEN + harness.assert_unread() + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_detail_requires_login_and_csrf(harness: _Harness, kind: _Kind) -> None: + assert harness.get(kind, authenticated=False).status_code == HTTPStatus.UNAUTHORIZED + assert harness.get(kind, csrf=False).status_code == HTTPStatus.UNAUTHORIZED + harness.assert_unread() + + +@pytest.mark.parametrize("handler", [ChatConversationDetailApi.get, CompletionConversationDetailApi.get]) +def test_detail_declares_app_view_layout(handler: Callable[..., object]) -> None: + checks: tuple[RBACCheck, ...] = vars(handler)[RBAC_CHECKS_ATTR] + assert len(checks) == 1 + assert checks[0].scene == RBACPermission.APP_VIEW_LAYOUT + assert isinstance(checks[0].locator, PlainApp) + assert checks[0].locator.param == "app_id" + + +@pytest.mark.parametrize("failure", ["metadata", "thought_files"]) +def test_failed_message_rendering_does_not_mark_conversation_read(harness: _Harness, failure: str) -> None: + source = harness.source + source.mode(AppMode.COMPLETION) + message = _message(harness) + with source.factory.begin() as session: + if failure == "metadata": + stored = session.get_one(Message, message.id) + stored.message_metadata = "invalid-json" + else: + session.add( + MessageAgentThought( + message_id=message.id, + position=1, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=source.account.id, + message_files='[{"not":"a file id"}]', + tool_labels_str="{}", + ) + ) + response = harness.get("completion") + assert response.status_code == HTTPStatus.BAD_REQUEST + assert response.get_json()["code"] == "invalid_param" + harness.assert_unread() diff --git a/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_list.py b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_list.py new file mode 100644 index 00000000000..196ced754c8 --- /dev/null +++ b/api/tests/e2e_tests/contracts/controllers/console/app/test_conversation_list.py @@ -0,0 +1,457 @@ +"""Console conversation lists through real account admission, services and SQLite.""" + +from collections.abc import Callable, Iterator +from dataclasses import dataclass, field +from datetime import datetime, timedelta +from decimal import Decimal +from typing import Literal +from uuid import uuid4 + +import pytest +from flask import Flask +from sqlalchemy import Connection, Engine, event, select +from sqlalchemy.orm import Session, SessionTransaction, sessionmaker +from werkzeug.test import TestResponse + +from constants import COOKIE_NAME_CSRF_TOKEN, HEADER_NAME_CSRF_TOKEN +from controllers.common.rbac import RBAC_CHECKS_ATTR, PlainApp, RBACCheck, RBACPermission +from controllers.console.app.conversation import ChatConversationApi, CompletionConversationApi +from enums import DeploymentEdition +from enums.account import TenantAccountRole +from extensions.ext_database import db +from extensions.ext_login import DifyLoginManager, unauthorized_handler +from fields.conversation_fields import ( + ConversationPagination, + ConversationResponseSource, + ConversationWithSummaryPagination, +) +from libs.external_api import ExternalApi +from libs.helper import dump_response +from libs.token import generate_csrf_token +from models import Account, Tenant, TenantAccountJoin +from models.enums import ConversationFromSource, FeedbackFromSource, FeedbackRating +from models.model import App, AppMode, Conversation, Message, MessageAnnotation, MessageFeedback +from models.utils.file_input_compat import restore_input_files +from repositories.conversation_repository import ConversationRepository +from services.conversation_query_service import ConversationQueryService +from services.message_query_adapters import MessageFileResolver +from tests.helpers.unit.model_factories import make_account, make_app, make_conversation, make_message + +type _Kind = Literal["chat", "completion"] +_CREATED_AT = datetime(2024, 1, 15, 6, 30, 0) + + +@dataclass(frozen=True) +class _Services: + conversation_queries: ConversationQueryService + + +@dataclass +class _Harness: + app: Flask + factory: sessionmaker[Session] + account: Account + tenant: Tenant + target: App + sessions: list[Session] = field(default_factory=list) + after_account_load: Callable[[], None] | None = None + + def get( + self, + kind: _Kind, + *, + query: dict[str, str | int] | None = None, + app_id: str | None = None, + authenticated: bool = True, + csrf: bool = True, + ) -> TestResponse: + client = self.app.test_client() + headers: dict[str, str] = {} + if authenticated: + with client.session_transaction() as session: + session["_user_id"] = self.account.id + session["_fresh"] = True + if csrf: + token = generate_csrf_token(self.account.id) + client.set_cookie(COOKIE_NAME_CSRF_TOKEN, token) + headers[HEADER_NAME_CSRF_TOKEN] = token + response = client.get( + f"/apps/{app_id or self.target.id}/{kind}-conversations", query_string=query, headers=headers + ) + assert all(not session.in_transaction() and not session.identity_map for session in self.sessions) + assert response.content_type == "application/json" + assert int(response.headers["Content-Length"]) == len(response.data) + return response + + def mode(self, mode: AppMode) -> None: + with self.factory.begin() as session: + target = session.get(App, self.target.id) + assert target is not None + target.mode = mode + self.target.mode = mode + + +@pytest.fixture +def harness( + sqlite_engine: Engine, sqlite_session_factory: sessionmaker[Session], config_overrides: Callable[..., None] +) -> Iterator[_Harness]: + config_overrides( + DEPLOYMENT_EDITION=DeploymentEdition.CLOUD, + LOGIN_DISABLED=False, + RBAC_ENABLED=False, + SECRET_KEY="conversation-http-tests-key-at-least-32-bytes", + CONSOLE_WEB_URL="http://localhost", + CONSOLE_API_URL="http://localhost", + COOKIE_DOMAIN="", + ) + tenant = Tenant(name="Console workspace") + account = make_account(account_id=str(uuid4()), timezone="Asia/Shanghai") + target = make_app(app_id=str(uuid4()), tenant_id=tenant.id) + with sqlite_session_factory.begin() as session: + session.add_all( + [ + tenant, + account, + target, + TenantAccountJoin(tenant_id=tenant.id, account_id=account.id, role=TenantAccountRole.OWNER), + ] + ) + app = Flask(__name__) + app.config.update( + TESTING=True, + RESTX_ERROR_404_HELP=False, + SECRET_KEY="console-test-session", + SQLALCHEMY_DATABASE_URI=str(sqlite_engine.url), + ) + db.init_app(app) + login_manager = DifyLoginManager() + login_manager.init_app(app) + login_manager.unauthorized_handler(unauthorized_handler) + + @login_manager.user_loader + def load_account(account_id: str) -> Account | None: + with sqlite_session_factory() as session: + loaded = session.get(Account, account_id) + if loaded is not None: + loaded.set_tenant_id_with_session(tenant.id, session=session) + if state.after_account_load is not None: + state.after_account_load() + return loaded + + factory = sessionmaker(bind=sqlite_engine, expire_on_commit=False) + state = _Harness(app, sqlite_session_factory, account, tenant, target) + + @event.listens_for(factory, "after_begin") + def record_session(session: Session, _transaction: SessionTransaction, _connection: Connection) -> None: + assert all(not previous.in_transaction() and not previous.identity_map for previous in state.sessions) + state.sessions.append(session) + + app.extensions["application_services"] = _Services( + ConversationQueryService( + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=factory), + restore_inputs=restore_input_files, + ) + ) + api = ExternalApi(app) + api.add_resource(ChatConversationApi, "/apps//chat-conversations") + api.add_resource(CompletionConversationApi, "/apps//completion-conversations") + yield state + with app.app_context(): + db.session.remove() + db.engine.dispose() + + +def _conversation( + harness: _Harness, + *, + app_id: str | None = None, + name: str = "Conversation name", + query: str = "First question", + answer: str = "First answer", + created_at: datetime = _CREATED_AT, + updated_at: datetime = _CREATED_AT, + deleted: bool = False, + invoke_from: str = "web-app", + with_message: bool = True, +) -> Conversation: + conversation = make_conversation( + conversation_id=str(uuid4()), + app_id=app_id or harness.target.id, + mode=harness.target.mode, + name=name, + inputs={}, + status="normal", + from_source=ConversationFromSource.API, + from_account_id=harness.account.id, + introduction="Conversation introduction", + invoke_from=invoke_from, + is_deleted=deleted, + override_model_configs='{"pre_prompt":"Be useful","model":{"name":"example"}}', + ) + conversation.created_at = created_at + conversation.updated_at = updated_at + message = make_message( + message_id=str(uuid4()), + app_id=conversation.app_id, + conversation_id=conversation.id, + inputs={"text": "", "zero": 0, "boolean": False, "empty": [], "null": None}, + query=query, + answer=answer, + message=[{"text": "Prompt", "role": "user"}], + message_unit_price=Decimal(0), + answer_unit_price=Decimal(0), + total_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.API, + created_at=created_at, + updated_at=updated_at, + ) + with harness.factory.begin() as session: + session.add(conversation) + if with_message: + session.add(message) + return conversation + + +def _ids(response: TestResponse) -> list[str]: + assert response.status_code == 200, response.get_json() + return [row["id"] for row in response.get_json()["data"]] + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_empty_page_retains_wire_contract(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + response = harness.get(kind) + assert response.status_code == 200 + assert response.get_json() == {"page": 1, "limit": 20, "total": 0, "has_more": False, "data": []} + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_response_matches_legacy_projection_without_marking_read(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + conversation = _conversation(harness) + with harness.factory.begin() as session: + message = session.scalar(select(Message).where(Message.conversation_id == conversation.id)) + assert message is not None + session.add_all( + [ + MessageAnnotation( + app_id=harness.target.id, + conversation_id=conversation.id, + message_id=message.id, + account_id=harness.account.id, + question="Question", + content="Annotation answer", + ), + MessageFeedback( + app_id=harness.target.id, + conversation_id=conversation.id, + message_id=message.id, + rating=FeedbackRating.LIKE, + from_source=FeedbackFromSource.USER, + ), + ] + ) + schema = ConversationPagination if kind == "completion" else ConversationWithSummaryPagination + with harness.app.app_context(), harness.factory() as session: + stored = session.get(Conversation, conversation.id) + assert stored is not None + expected = dump_response( + schema, + { + "page": 1, + "per_page": 20, + "total": 1, + "has_next": False, + "items": [ConversationResponseSource(stored, session=session)], + }, + ) + response = harness.get(kind) + assert response.status_code == 200 + assert response.get_json() == expected + assert response.get_json()["data"][0]["model_config"] == {"model": {"name": "example"}, "pre_prompt": "Be useful"} + with harness.factory() as session: + stored = session.get(Conversation, conversation.id) + assert stored is not None + assert stored.read_at is None + assert stored.read_account_id is None + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_pagination_and_app_deletion_isolation(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + first = _conversation(harness, created_at=_CREATED_AT, updated_at=_CREATED_AT) + second = _conversation( + harness, created_at=_CREATED_AT + timedelta(hours=1), updated_at=_CREATED_AT + timedelta(hours=1) + ) + _conversation(harness, app_id=str(uuid4())) + _conversation(harness, deleted=True) + first_page = harness.get(kind, query={"limit": 1}) + assert _ids(first_page) == [second.id] + assert first_page.get_json()["total"] == 2 + assert first_page.get_json()["has_more"] is True + second_page = harness.get(kind, query={"limit": 1, "page": 2}) + assert _ids(second_page) == [first.id] + assert second_page.get_json()["has_more"] is False + assert _ids(harness.get(kind, query={"page": 3, "limit": 1})) == [] + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize( + "query", [{"limit": 0}, {"limit": 101}, {"page": 0}, {"page": 100000}, {"annotation_status": "bad"}] +) +def test_invalid_query_returns_422(harness: _Harness, kind: _Kind, query: dict[str, str | int]) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + response = harness.get(kind, query=query) + assert response.status_code == 422 + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +@pytest.mark.parametrize("query", [{"start": "bad"}, {"start": "2024-01-16 00:00", "end": "2024-01-15 00:00"}]) +def test_invalid_date_range_is_precise_400(harness: _Harness, kind: _Kind, query: dict[str, str | int]) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + response = harness.get(kind, query=query) + assert response.status_code == 400 + assert response.get_json()["code"] == "invalid_param" + assert response.get_json()["message"] + + +@pytest.mark.parametrize( + ("kind", "sort_by"), [("completion", "-created_at"), ("chat", "created_at"), ("chat", "-updated_at")] +) +def test_account_timezone_and_exact_end_second_are_inclusive(harness: _Harness, kind: _Kind, sort_by: str) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + boundary = _CREATED_AT.replace(second=59) + expected = _conversation(harness, created_at=boundary, updated_at=boundary) + _conversation( + harness, created_at=boundary + timedelta(microseconds=1), updated_at=boundary + timedelta(microseconds=1) + ) + _conversation(harness, created_at=_CREATED_AT - timedelta(seconds=1), updated_at=_CREATED_AT - timedelta(seconds=1)) + assert _ids( + harness.get(kind, query={"start": "2024-01-15 14:30", "end": "2024-01-15 14:30", "sort_by": sort_by}) + ) == [expected.id] + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_empty_dates_remain_unbounded(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + conversation = _conversation(harness) + assert _ids(harness.get(kind, query={"start": "", "end": ""})) == [conversation.id] + + +@pytest.mark.parametrize("field", ["query", "answer"]) +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_keyword_is_case_insensitive_and_escapes_wildcards(harness: _Harness, kind: _Kind, field: str) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + matching = _conversation( + harness, + query="Literal 50%_done" if field == "query" else "First question", + answer="Literal 50%_done" if field == "answer" else "First answer", + ) + _conversation(harness, query="50XYZdone", answer="50XYZdone") + assert _ids(harness.get(kind, query={"keyword": "50%_DONE"})) == [matching.id] + + +@pytest.mark.parametrize("filter_value", ["annotated", "not_annotated"]) +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_annotation_filter(harness: _Harness, kind: _Kind, filter_value: str) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + annotated = _conversation(harness) + plain = _conversation(harness) + with harness.factory.begin() as session: + session.add( + MessageAnnotation( + app_id=harness.target.id, + conversation_id=annotated.id, + account_id=harness.account.id, + content="Annotation", + question="Question", + ) + ) + assert _ids(harness.get(kind, query={"annotation_status": filter_value})) == [ + annotated.id if filter_value == "annotated" else plain.id + ] + + +@pytest.mark.parametrize("mode", [AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT]) +def test_chat_modes_and_debugger_visibility(harness: _Harness, mode: AppMode) -> None: + harness.mode(mode) + published = _conversation(harness) + debugger = _conversation(harness, invoke_from="debugger") + expected = {published.id} if mode == AppMode.ADVANCED_CHAT else {published.id, debugger.id} + assert set(_ids(harness.get("chat"))) == expected + + +@pytest.mark.parametrize( + ("kind", "mode"), [("chat", AppMode.COMPLETION), ("chat", AppMode.WORKFLOW), ("completion", AppMode.CHAT)] +) +def test_unavailable_app_is_rejected_before_query_validation(harness: _Harness, kind: _Kind, mode: AppMode) -> None: + harness.mode(mode) + response = harness.get(kind, query={"limit": 0}) + assert response.status_code == 404 + assert response.get_json()["code"] == "app_not_found" + assert "App mode is not in the supported list" in response.get_json()["message"] + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_another_tenants_app_is_not_visible(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + with harness.factory.begin() as session: + target = session.get(App, harness.target.id) + assert target is not None + target.tenant_id = str(uuid4()) + response = harness.get(kind) + assert response.status_code == 404 + assert response.get_json()["code"] == "app_not_found" + + +@pytest.mark.parametrize( + "role", + [ + TenantAccountRole.OWNER, + TenantAccountRole.ADMIN, + TenantAccountRole.EDITOR, + TenantAccountRole.NORMAL, + TenantAccountRole.DATASET_OPERATOR, + ], +) +def test_existing_edit_roles_are_preserved(harness: _Harness, role: TenantAccountRole) -> None: + with harness.factory.begin() as session: + membership = session.scalar(select(TenantAccountJoin).where(TenantAccountJoin.account_id == harness.account.id)) + assert membership is not None + membership.role = role + response = harness.get("chat") + assert response.status_code == (200 if TenantAccountRole.is_editing_role(role) else 403) + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_login_and_csrf_remain_required(harness: _Harness, kind: _Kind) -> None: + assert harness.get(kind, authenticated=False).status_code == 401 + assert harness.get(kind, csrf=False).status_code == 401 + + +@pytest.mark.parametrize("handler", [ChatConversationApi.get, CompletionConversationApi.get]) +def test_app_view_layout_rbac_check_is_declared(handler: Callable[..., object]) -> None: + checks: tuple[RBACCheck, ...] = vars(handler)[RBAC_CHECKS_ATTR] + assert len(checks) == 1 + assert checks[0].scene == RBACPermission.APP_VIEW_LAYOUT + assert isinstance(checks[0].locator, PlainApp) + assert checks[0].locator.param == "app_id" + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_account_removed_after_authentication_is_unauthorized(harness: _Harness, kind: _Kind) -> None: + harness.mode(AppMode.COMPLETION if kind == "completion" else AppMode.CHAT) + + def delete_account() -> None: + with harness.factory.begin() as session: + account = session.get(Account, harness.account.id) + assert account is not None + session.delete(account) + + harness.after_account_load = delete_account + response = harness.get(kind) + assert response.status_code == 401 + assert response.get_json()["code"] == "unauthorized" + assert response.get_json()["message"] diff --git a/api/tests/e2e_tests/contracts/controllers/service_api/app/test_conversation_list.py b/api/tests/e2e_tests/contracts/controllers/service_api/app/test_conversation_list.py index 9bd67138274..9ca03d60bc6 100644 --- a/api/tests/e2e_tests/contracts/controllers/service_api/app/test_conversation_list.py +++ b/api/tests/e2e_tests/contracts/controllers/service_api/app/test_conversation_list.py @@ -23,6 +23,7 @@ from repositories.conversation_repository import ConversationRepository from services.auth.resource_access_token_contracts import ResourceAccessTokenResource from services.conversation_query_service import ConversationQueryService from services.entities.conversation_entities import ConversationSort +from services.message_query_adapters import MessageFileResolver from tests.e2e_tests.contracts.controllers.service_api.app import test_conversation_rename as rename_tests from tests.e2e_tests.contracts.controllers.service_api.app.test_conversation_rename import _Harness as RenameHarness from tests.e2e_tests.contracts.controllers.service_api.app.test_conversation_rename import _Services as RenameServices @@ -67,6 +68,7 @@ def harness(rename_harness: RenameHarness) -> _Harness: conversation_names=original.conversation_names, resource_access_tokens=original.resource_access_tokens, conversation_queries=ConversationQueryService( + files=MessageFileResolver(), conversations=ConversationRepository(session_factory=rename_harness.repository_factory), restore_inputs=restore_input_files, ), diff --git a/api/tests/e2e_tests/contracts/controllers/web/test_conversation_list.py b/api/tests/e2e_tests/contracts/controllers/web/test_conversation_list.py index 8612830865d..38d79795040 100644 --- a/api/tests/e2e_tests/contracts/controllers/web/test_conversation_list.py +++ b/api/tests/e2e_tests/contracts/controllers/web/test_conversation_list.py @@ -22,6 +22,7 @@ from models.web import PinnedConversation from repositories.conversation_repository import ConversationRepository from services.conversation_query_service import ConversationQueryService from services.entities.conversation_entities import ConversationSort +from services.message_query_adapters import MessageFileResolver from tests.e2e_tests.contracts.controllers.web import test_conversation_rename as rename_tests from tests.e2e_tests.contracts.controllers.web.test_conversation_rename import _Harness as RenameHarness from tests.helpers.unit.model_factories import make_conversation @@ -64,6 +65,7 @@ def harness(rename_harness: RenameHarness) -> _Harness: rename_harness.app.extensions["application_services"] = _Services( conversation_queries=ConversationQueryService( + files=MessageFileResolver(), conversations=ConversationRepository(session_factory=rename_harness.repository_factory), restore_inputs=restore, ) diff --git a/api/tests/unit_tests/controllers/console/app/test_conversation.py b/api/tests/unit_tests/controllers/console/app/test_conversation.py index eaf406da334..2c62fc7f266 100644 --- a/api/tests/unit_tests/controllers/console/app/test_conversation.py +++ b/api/tests/unit_tests/controllers/console/app/test_conversation.py @@ -1,32 +1,24 @@ from __future__ import annotations -import datetime from decimal import Decimal -from inspect import unwrap -from unittest.mock import MagicMock, patch -import pytest -import sqlalchemy as sa -from flask import Flask from sqlalchemy.orm import Session -from werkzeug.exceptions import BadRequest, NotFound -from controllers.console.app import conversation as conversation_module -from core.app.entities.app_invoke_entities import InvokeFrom +from fields.conversation_fields import ConversationResponseSource from graphon.enums import WorkflowExecutionStatus -from models import Account, App, EndUser, Message, MessageAnnotation, MessageFeedback +from models import Account, EndUser, Message, MessageAnnotation, MessageFeedback from models.enums import ( ConversationFromSource, CreatorUserRole, EndUserType, FeedbackFromSource, FeedbackRating, + InvokeFrom, WorkflowRunTriggeredFrom, ) from models.model import AppMode, Conversation from models.workflow import WorkflowRun, WorkflowType -from services.errors.conversation import ConversationNotExistsError -from tests.helpers.unit.model_factories import make_app, make_conversation +from tests.helpers.unit.model_factories import make_conversation def _make_account() -> Account: @@ -35,10 +27,6 @@ def _make_account() -> Account: return account -def _app(*, mode: AppMode = AppMode.CHAT) -> App: - return make_app(name="Conversation app", mode=mode) - - def _conversation(*, conversation_id: str = "c1", app_id: str = "app-1") -> Conversation: return make_conversation( conversation_id=conversation_id, @@ -54,97 +42,6 @@ def _conversation(*, conversation_id: str = "c1", app_id: str = "app-1") -> Conv ) -def test_completion_conversation_list_returns_paginated_result( - app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - api = conversation_module.CompletionConversationApi() - method = unwrap(api.get) - account = _make_account() - monkeypatch.setattr(conversation_module, "parse_time_range", lambda *_args, **_kwargs: (None, None)) - paginate_result = MagicMock() - paginate_result.page = 1 - paginate_result.per_page = 20 - paginate_result.total = 0 - paginate_result.has_next = False - paginate_result.items = [] - monkeypatch.setattr(conversation_module, "paginate_query", lambda *_args, **_kwargs: paginate_result) - with app.test_request_context("/console/api/apps/app-1/completion-conversations", method="GET"): - response = method( - api, - conversation_module.CompletionConversationQuery(), - unbound_session, - account, - app_model=_app(mode=AppMode.COMPLETION), - ) - assert response == {"page": 1, "limit": 20, "total": 0, "has_more": False, "data": []} - - -def test_completion_conversation_list_invalid_time_range( - app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - api = conversation_module.CompletionConversationApi() - method = unwrap(api.get) - account = _make_account() - monkeypatch.setattr( - conversation_module, - "parse_time_range", - lambda *_args, **_kwargs: (_ for _ in ()).throw(ValueError("bad range")), - ) - with app.test_request_context( - "/console/api/apps/app-1/completion-conversations", method="GET", query_string={"start": "bad"} - ): - with pytest.raises(BadRequest): - method( - api, - conversation_module.CompletionConversationQuery(), - unbound_session, - account, - app_model=_app(mode=AppMode.COMPLETION), - ) - - -def test_chat_conversation_list_advanced_chat_calls_paginate( - app: Flask, monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - api = conversation_module.ChatConversationApi() - method = unwrap(api.get) - account = _make_account() - monkeypatch.setattr(conversation_module, "parse_time_range", lambda *_args, **_kwargs: (None, None)) - paginate_result = MagicMock() - paginate_result.page = 1 - paginate_result.per_page = 20 - paginate_result.total = 0 - paginate_result.has_next = False - paginate_result.items = [] - monkeypatch.setattr(conversation_module, "paginate_query", lambda *_args, **_kwargs: paginate_result) - with app.test_request_context("/console/api/apps/app-1/chat-conversations", method="GET"): - response = method( - api, - conversation_module.ChatConversationQuery(), - unbound_session, - account, - app_model=_app(mode=AppMode.ADVANCED_CHAT), - ) - assert response == {"page": 1, "limit": 20, "total": 0, "has_more": False, "data": []} - - -def test_get_conversation_updates_read_at(sqlite_session: Session) -> None: - conversation = _conversation() - sqlite_session.add(conversation) - sqlite_session.flush() - session = sqlite_session - result = conversation_module._get_conversation(session, _make_account(), _app(), "c1") - assert result is conversation - assert conversation.read_at is not None - assert conversation.read_account_id == "u1" - - -def test_get_conversation_missing_raises_not_found(sqlite_session: Session) -> None: - session = sqlite_session - with pytest.raises(NotFound): - conversation_module._get_conversation(session, _make_account(), _app(), "missing") - - def test_conversation_response_source_uses_caller_session(sqlite_session: Session) -> None: account = _make_account() end_user = EndUser( @@ -240,7 +137,7 @@ def test_conversation_response_source_uses_caller_session(sqlite_session: Sessio sqlite_session.add_all([account, end_user, conversation, message, annotation, workflow_run, *feedbacks]) sqlite_session.commit() - source = conversation_module.ConversationResponseSource(conversation, session=sqlite_session) + source = ConversationResponseSource(conversation, session=sqlite_session) assert source.inputs == {"topic": "support"} assert source.model_config == {"model_id": None, "provider": None} @@ -256,124 +153,3 @@ def test_conversation_response_source_uses_caller_session(sqlite_session: Sessio assert source.first_message is not None assert source.from_end_user_session_id == "end-user-session" assert source.from_account_name == "Account" - - -def test_completion_conversation_delete_maps_not_found( - monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - api = conversation_module.CompletionConversationDetailApi() - method = unwrap(api.delete) - monkeypatch.setattr( - conversation_module.ConversationService, - "delete", - lambda *_args, **_kwargs: (_ for _ in ()).throw(ConversationNotExistsError()), - ) - session = unbound_session - with pytest.raises(NotFound): - method(api, session, _make_account(), app_model=_app(), conversation_id="c1") - - -# ── Regression tests for the < vs <= end-time boundary bug ────────────────── - - -def _captured_where_clauses( - monkeypatch: pytest.MonkeyPatch, - app: Flask, - endpoint_class, - query_instance, - app_mode, -) -> list[str]: - """ - Run the endpoint's GET handler with a fixed end_datetime and capture the - SQLAlchemy WHERE clause strings that were applied so we can assert on the - comparison operator used for the end-time boundary. - - ``query_instance`` is passed directly to the unwrapped handler, so callers - can pre-configure it (e.g. ``ChatConversationQuery(sort_by="created_at")``). - """ - # 2024-01-15 14:30:00 UTC — parse_time_range returns second=0; the handler - # then calls .replace(second=59) giving 14:30:59. - boundary_dt = datetime.datetime(2024, 1, 15, 14, 30, 0) - - monkeypatch.setattr( - conversation_module, - "parse_time_range", - lambda *_args, **_kwargs: (None, boundary_dt), - ) - - captured: list[str] = [] - original_where = sa.sql.selectable.Select.where - - def spy_where(self, *criteria, **kwargs): - for c in criteria: - captured.append(str(c.compile(compile_kwargs={"literal_binds": True}))) - return original_where(self, *criteria, **kwargs) - - paginate_result = MagicMock() - paginate_result.page = 1 - paginate_result.per_page = 20 - paginate_result.total = 0 - paginate_result.has_next = False - paginate_result.items = [] - monkeypatch.setattr(conversation_module, "paginate_query", lambda *_args, **_kwargs: paginate_result) - - api = endpoint_class() - method = unwrap(api.get) - account = _make_account() - - with Session() as session, app.test_request_context("/", method="GET"): - with patch.object(sa.sql.selectable.Select, "where", spy_where): - method( - api, - query_instance, - session, - account, - app_model=_app(mode=app_mode), - ) - - return captured - - -def test_completion_conversation_end_time_filter_is_inclusive(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: - """CompletionConversationApi must use <= (not <) for the end-time boundary. - - Regression test for the bug where `created_at < end_datetime_utc` was used - instead of `created_at <= end_datetime_utc`, silently dropping conversations - created at exactly the boundary second (HH:MM:59). - """ - clauses = _captured_where_clauses( - monkeypatch, - app, - conversation_module.CompletionConversationApi, - conversation_module.CompletionConversationQuery(), - AppMode.COMPLETION, - ) - # Filter by the boundary timestamp value — field name not assumed. - end_clauses = [c for c in clauses if "14:30:59" in c] - assert end_clauses, "No end-time WHERE clause found for CompletionConversationApi" - for clause in end_clauses: - assert "<=" in clause, f"CompletionConversationApi end-time filter must use <= (inclusive), got: {clause!r}" - assert clause.count("<") == 1, f"Expected exactly one <= operator (no stray < without =), got: {clause!r}" - - -def test_chat_conversation_end_time_filter_is_inclusive(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: - """ChatConversationApi must use <= for the end-time boundary (control test). - - ChatConversationQuery defaults to sort_by='-updated_at', which routes the - end-time filter through the updated_at branch. We explicitly request - sort_by='created_at' to exercise the created_at branch and confirm it also - uses <=, mirroring what CompletionConversationApi does after the fix. - """ - clauses = _captured_where_clauses( - monkeypatch, - app, - conversation_module.ChatConversationApi, - # Use created_at sort so the WHERE clause hits created_at, not updated_at. - conversation_module.ChatConversationQuery(sort_by="created_at"), - AppMode.ADVANCED_CHAT, - ) - # Filter by the boundary timestamp value — field name not assumed. - end_clauses = [c for c in clauses if "14:30:59" in c] - assert end_clauses, "No end-time WHERE clause found for ChatConversationApi" - for clause in end_clauses: - assert "<=" in clause, f"ChatConversationApi end-time filter must use <= (inclusive), got: {clause!r}" diff --git a/api/tests/unit_tests/controllers/console/explore/test_conversation.py b/api/tests/unit_tests/controllers/console/explore/test_conversation.py index 1a1236ad04a..1568d89b53b 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_conversation.py +++ b/api/tests/unit_tests/controllers/console/explore/test_conversation.py @@ -24,6 +24,7 @@ from services.conversation_command_service import ConversationCommandService from services.conversation_name_service import ConversationNameService from services.conversation_query_service import ConversationQueryService from services.installed_app_conversation_service import InstalledAppConversationService +from services.message_query_adapters import MessageFileResolver from tests.unit_tests.controllers.console.explore.test_installed_app_admission import ( _assert_json_response, _Harness, @@ -122,7 +123,9 @@ def conversations( installed_apps=_InstalledAppServices( conversations=InstalledAppConversationService( queries=ConversationQueryService( - conversations=ConversationRepository(session_factory=factory), restore_inputs=restore_input_files + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=factory), + restore_inputs=restore_input_files, ), names=ConversationNameService( conversations=ConversationRepository(session_factory=factory), generate_name=state.generate_name diff --git a/api/tests/unit_tests/controllers/service_api/app/test_conversation.py b/api/tests/unit_tests/controllers/service_api/app/test_conversation.py index b944792319b..191e934f9ff 100644 --- a/api/tests/unit_tests/controllers/service_api/app/test_conversation.py +++ b/api/tests/unit_tests/controllers/service_api/app/test_conversation.py @@ -21,7 +21,6 @@ from fields._value_type_serializer import serialize_value_type from graphon.variables import StringSegment from graphon.variables.types import SegmentType from models.model import App, AppMode -from services.conversation_service import ConversationService """ Unit tests for Service API Conversation controllers. @@ -426,14 +425,6 @@ class TestConversationCases: error = services.errors.conversation.ConversationVariableTypeMismatchError("Type mismatch") assert isinstance(error, services.errors.conversation.ConversationVariableTypeMismatchError) - class TestConversationService: - """Test ConversationService integration patterns.""" - - def test_delete_method_exists(self): - """Test that ConversationService.delete exists.""" - assert hasattr(ConversationService, "delete") - assert callable(ConversationService.delete) - class TestConversationPayloadsController: def test_rename_requires_name(self) -> None: with pytest.raises( diff --git a/api/tests/unit_tests/extensions/test_ext_application_services.py b/api/tests/unit_tests/extensions/test_ext_application_services.py index 14264b4cec2..ab18ed81c9c 100644 --- a/api/tests/unit_tests/extensions/test_ext_application_services.py +++ b/api/tests/unit_tests/extensions/test_ext_application_services.py @@ -329,6 +329,7 @@ def test_build_application_services_preserves_composed_boundaries( assert services.dataset_api_keys._access is services.knowledge.datasets._dataset_access assert isinstance(services.oauth_device, OAuthDeviceApplicationService) assert register_script.call_count == 3 + assert services.conversation_queries._files is services.message_queries._files assert isinstance(services.installed_apps.generation, InstalledAppGenerationService) installed_apps = services.installed_apps.access._installed_apps diff --git a/api/tests/unit_tests/repositories/test_conversation_repository.py b/api/tests/unit_tests/repositories/test_conversation_repository.py index 150874d6c16..f19c8d74e05 100644 --- a/api/tests/unit_tests/repositories/test_conversation_repository.py +++ b/api/tests/unit_tests/repositories/test_conversation_repository.py @@ -1,21 +1,70 @@ -from dataclasses import replace +import json +from dataclasses import dataclass, replace from datetime import datetime, timedelta from decimal import Decimal from typing import Literal from uuid import uuid4 import pytest -from sqlalchemy import Engine, event, inspect, select, update +from sqlalchemy import Engine, event, inspect, select, text, update +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session, sessionmaker -from core.app.entities.app_invoke_entities import InvokeFrom -from models.agent import AgentWorkingResourceStatus, AgentWorkspace, AgentWorkspaceOwnerType -from models.enums import ConversationFromSource, CreatorUserRole, EndUserType +from core.agent.workspace import AgentWorkspaceNotFoundError +from fields.conversation_fields import Conversation as CompletionResponse +from fields.conversation_fields import ConversationResponseSource, ConversationWithSummary +from graphon.enums import WorkflowExecutionStatus +from graphon.file import FILE_MODEL_IDENTITY, FileTransferMethod, FileType +from libs.datetime_utils import naive_utc_now +from machinery.context import RequestContext +from models.account import Account +from models.agent import ( + Agent, + AgentConfigVersionKind, + AgentScope, + AgentSource, + AgentStatus, + AgentWorkingResourceStatus, + AgentWorkspace, + AgentWorkspaceBinding, + AgentWorkspaceOwnerType, +) +from models.enums import ( + ConversationFromSource, + ConversationStatus, + CreatorUserRole, + EndUserType, + FeedbackFromSource, + FeedbackRating, + InvokeFrom, + MessageFileBelongsTo, + WorkflowRunTriggeredFrom, +) from models.enums import InvokeFrom as ModelInvokeFrom -from models.model import App, AppMode, Conversation, EndUser, InstalledApp, Message +from models.model import ( + App, + AppMode, + Conversation, + EndUser, + InstalledApp, + Message, + MessageAnnotation, + MessageFeedback, + MessageFile, +) from models.web import PinnedConversation +from models.workflow import WorkflowRun, WorkflowType from repositories.conversation_repository import ConversationRepository +from services.app.console_service import ConsoleAppNotFoundError from services.entities.conversation_entities import ( + ConsoleChatConversationRecord, + ConsoleCompletionConversationDetail, + ConsoleCompletionConversationRecord, + ConsoleConversationFilters, + ConsoleConversationKind, + ConsoleConversationPage, + ConsoleConversationScope, + ConversationAnnotationStatus, ConversationListSource, ConversationPage, ConversationSort, @@ -29,7 +78,11 @@ from services.errors.message import MessageNotExistsError from services.installed_app_access_service import InstalledAppNotFoundError, InstalledAppRef _ACCOUNT_ID = "11111111-1111-4111-8111-111111111111" + + _OWNER_TENANT_ID = "22222222-2222-4222-8222-222222222222" + + _TIME = datetime(2026, 9, 8, 12, 30) @@ -922,3 +975,1063 @@ def test_end_user_empty_page_keeps_missing_cursor_and_empty_pin_semantics( pinned=False, sort_by="-updated_at", ) + + +_CONSOLE_LIST_TIME = datetime(2026, 10, 13, 12, 30) + + +_FILTERS = ConsoleConversationFilters( + keyword=None, start=None, end=None, annotation_status="all", sort_by="-updated_at", page=1, limit=20 +) + + +@dataclass(frozen=True) +class ConsoleData: + context: RequestContext + app_id: str + end_user_id: str + + +@pytest.fixture +def console_data(sqlite_session_factory: sessionmaker[Session]) -> ConsoleData: + with sqlite_session_factory.begin() as session: + account = Account(name="Maintainer", email="maintainer@example.com", timezone="Asia/Shanghai") + app = App(tenant_id=str(uuid4()), name="Chat app", mode=AppMode.CHAT, enable_api=True, enable_site=True) + session.add_all([account, app]) + session.flush() + end_user = EndUser( + tenant_id=app.tenant_id, app_id=app.id, type=EndUserType.BROWSER, session_id="visitor-needle" + ) + session.add(end_user) + session.flush() + return ConsoleData( + context=RequestContext( + request_id="test", trace_id=None, account_id=account.id, active_workspace_id=app.tenant_id + ), + app_id=app.id, + end_user_id=end_user.id, + ) + + +def _console_conversation(session: Session, data: ConsoleData, *, mode: AppMode = AppMode.CHAT) -> Conversation: + conversation = Conversation( + app_id=data.app_id, + mode=mode, + name="Conversation", + _inputs={}, + from_source=ConversationFromSource.API, + from_end_user_id=data.end_user_id, + from_account_id=None, + status=ConversationStatus.NORMAL, + invoke_from=InvokeFrom.SERVICE_API, + is_deleted=False, + ) + conversation.created_at = _CONSOLE_LIST_TIME + conversation.updated_at = _CONSOLE_LIST_TIME + session.add(conversation) + session.flush() + return conversation + + +def _console_message(session: Session, conversation: Conversation, *, query: str = "First query") -> Message: + message = Message( + app_id=conversation.app_id, + conversation_id=conversation.id, + _inputs={"zero": 0, "empty": "", "none": None}, + query=query, + message=[{"text": "Prompt text"}], + message_unit_price=Decimal(0), + answer="Answer", + answer_unit_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.API, + from_end_user_id=conversation.from_end_user_id, + ) + message.created_at = _CONSOLE_LIST_TIME + session.add(message) + session.flush() + return message + + +def _scope( + factory: sessionmaker[Session], data: ConsoleData, *, kind: ConsoleConversationKind = "chat" +) -> ConsoleConversationScope: + return ConversationRepository(session_factory=factory).get_console_scope(data.context, data.app_id, kind=kind) + + +def _page( + factory: sessionmaker[Session], data: ConsoleData, *, filters: ConsoleConversationFilters = _FILTERS +) -> ConsoleConversationPage: + return ConversationRepository(session_factory=factory).get_console_page( + scope=_scope(factory, data), filters=filters + ) + + +@pytest.mark.parametrize("mode", [AppMode.CHAT, AppMode.AGENT_CHAT, AppMode.ADVANCED_CHAT, AppMode.AGENT]) +def test_chat_scope_allows_supported_modes_and_returns_account_timezone( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, mode: AppMode +) -> None: + with sqlite_session_factory.begin() as session: + app = session.get_one(App, console_data.app_id) + app.mode = mode + scope = _scope(sqlite_session_factory, console_data) + assert scope == ConsoleConversationScope( + app_id=console_data.app_id, + tenant_id=console_data.context.active_workspace_id, + app_mode=mode, + timezone="Asia/Shanghai", + ) + + +@pytest.mark.parametrize("change", ["missing", "tenant", "status", "mode", "hidden_agent"]) +def test_scope_rejects_unavailable_or_hidden_app( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, change: str +) -> None: + with sqlite_session_factory.begin() as session: + app = session.get_one(App, console_data.app_id) + if change == "missing": + session.delete(app) + elif change == "tenant": + app.tenant_id = str(uuid4()) + elif change == "status": + session.execute( + text("UPDATE apps SET status = :status WHERE id = :id"), {"status": "disabled", "id": app.id} + ) + elif change == "mode": + app.mode = AppMode.COMPLETION + else: + app.mode = AppMode.AGENT + session.add( + Agent( + tenant_id=app.tenant_id, + name="Hidden", + scope=AgentScope.WORKFLOW_ONLY, + source=AgentSource.WORKFLOW, + backing_app_id=app.id, + status=AgentStatus.ARCHIVED, + ) + ) + with pytest.raises(ConsoleAppNotFoundError) as error: + _scope(sqlite_session_factory, console_data) + assert str(error.value) == ( + "App mode is not in the supported list: ['advanced-chat', 'agent', 'agent-chat', 'chat']" + if change == "mode" + else "App not found." + ) + + +@pytest.mark.parametrize("change", ["missing", "tenant", "status", "mode", "hidden_agent"]) +def test_page_rechecks_app_after_scope_resolution( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, change: str +) -> None: + if change == "hidden_agent": + with sqlite_session_factory.begin() as session: + session.get_one(App, console_data.app_id).mode = AppMode.AGENT + scope = _scope(sqlite_session_factory, console_data) + with sqlite_session_factory.begin() as session: + app = session.get_one(App, console_data.app_id) + if change == "missing": + session.delete(app) + elif change == "tenant": + app.tenant_id = str(uuid4()) + elif change == "status": + session.execute( + text("UPDATE apps SET status = :status WHERE id = :id"), {"status": "disabled", "id": app.id} + ) + elif change == "mode": + app.mode = AppMode.ADVANCED_CHAT + else: + session.add( + Agent( + tenant_id=app.tenant_id, + name="Hidden", + scope=AgentScope.WORKFLOW_ONLY, + source=AgentSource.WORKFLOW, + backing_app_id=app.id, + ) + ) + with pytest.raises(ConsoleAppNotFoundError) as error: + ConversationRepository(session_factory=sqlite_session_factory).get_console_page(scope=scope, filters=_FILTERS) + assert str(error.value) == ( + "App mode changed from 'chat' to 'advanced-chat' after admission." if change == "mode" else "App not found." + ) + + +def test_console_page_spans_actors_but_excludes_deleted_and_other_apps( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData +) -> None: + with sqlite_session_factory.begin() as session: + first = _console_conversation(session, console_data) + other_user = _console_conversation(session, console_data) + other_user.from_end_user_id = str(uuid4()) + account = _console_conversation(session, console_data) + account.from_source = ConversationFromSource.CONSOLE + account.from_end_user_id = None + account.from_account_id = console_data.context.account_id + deleted = _console_conversation(session, console_data) + deleted.is_deleted = True + other_app = _console_conversation(session, console_data) + other_app.app_id = str(uuid4()) + page = _page(sqlite_session_factory, console_data) + assert {item.id for item in page.items} == {first.id, other_user.id, account.id} + assert page.total == 3 + assert page.has_next is False + assert page.tenant_id == console_data.context.active_workspace_id + + +@pytest.mark.parametrize("field", ["query", "answer", "name", "introduction", "end_user"]) +def test_chat_keyword_matches_each_legacy_field_case_insensitively_without_duplicates( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, field: str +) -> None: + with sqlite_session_factory.begin() as session: + row = _console_conversation(session, console_data) + message = _console_message(session, row, query="Other") + _console_message(session, row, query="Other") + if field == "query": + message.query = "Some NEEDLE phrase" + elif field == "answer": + message.answer = "Some NEEDLE phrase" + elif field == "name": + row.name = "Some NEEDLE phrase" + elif field == "introduction": + row.introduction = "Some NEEDLE phrase" + no_messages = _console_conversation(session, console_data) + no_messages.name = "NEEDLE" + page = _page(sqlite_session_factory, console_data, filters=replace(_FILTERS, keyword="needle")) + assert [item.id for item in page.items] == [row.id] + assert page.total == 1 + + +@pytest.mark.parametrize("keyword", ["%", "_", "\\"]) +def test_keyword_treats_wildcards_as_literal_characters( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, keyword: str +) -> None: + with sqlite_session_factory.begin() as session: + literal = _console_conversation(session, console_data) + _console_message(session, literal, query=f"Literal {keyword}") + ordinary = _console_conversation(session, console_data) + _console_message(session, ordinary, query="Ordinary text") + page = _page(sqlite_session_factory, console_data, filters=replace(_FILTERS, keyword=keyword)) + assert [item.id for item in page.items] == [literal.id] + + +def test_completion_keeps_message_join_duplicates_and_only_completion_rows( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData +) -> None: + with sqlite_session_factory.begin() as session: + session.get_one(App, console_data.app_id).mode = AppMode.COMPLETION + row = _console_conversation(session, console_data, mode=AppMode.COMPLETION) + _console_message(session, row, query="Needle 1") + _console_message(session, row, query="Needle 2") + wrong_mode = _console_conversation(session, console_data) + _console_message(session, wrong_mode, query="Needle") + page = ConversationRepository(session_factory=sqlite_session_factory).get_console_page( + scope=_scope(sqlite_session_factory, console_data, kind="completion"), + filters=replace(_FILTERS, keyword="needle", limit=1), + ) + assert [item.id for item in page.items] == [row.id] + assert page.total == 2 + assert page.has_next is True + + +@pytest.mark.parametrize("annotation_status", ["all", "annotated", "not_annotated"]) +def test_annotation_filter_and_pagination( + sqlite_session_factory: sessionmaker[Session], + console_data: ConsoleData, + annotation_status: ConversationAnnotationStatus, +) -> None: + with sqlite_session_factory.begin() as session: + annotated = _console_conversation(session, console_data) + plain = _console_conversation(session, console_data) + session.add_all( + [ + MessageAnnotation( + app_id=console_data.app_id, + conversation_id=annotated.id, + question="Question", + content="Answer", + account_id=console_data.context.account_id, + ), + MessageAnnotation( + app_id=console_data.app_id, + conversation_id=annotated.id, + question="Question 2", + content="Answer 2", + account_id=console_data.context.account_id, + ), + ] + ) + page = _page(sqlite_session_factory, console_data, filters=replace(_FILTERS, annotation_status=annotation_status)) + expected = ( + {annotated.id, plain.id} + if annotation_status == "all" + else {annotated.id if annotation_status == "annotated" else plain.id} + ) + assert {item.id for item in page.items} == expected + assert page.total == len(expected) + + +@pytest.mark.parametrize("sort_by", ["created_at", "-created_at", "updated_at", "-updated_at"]) +def test_time_filter_and_sort_use_the_same_column_and_include_end_minute( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, sort_by: ConversationSort +) -> None: + with sqlite_session_factory.begin() as session: + before = _console_conversation(session, console_data) + before.created_at = _CONSOLE_LIST_TIME - timedelta(seconds=1) + before.updated_at = _CONSOLE_LIST_TIME - timedelta(seconds=1) + start = _console_conversation(session, console_data) + end = _console_conversation(session, console_data) + end.created_at = _CONSOLE_LIST_TIME.replace(second=59) + end.updated_at = _CONSOLE_LIST_TIME.replace(second=59) + after = _console_conversation(session, console_data) + after.created_at = _CONSOLE_LIST_TIME + timedelta(minutes=1) + after.updated_at = _CONSOLE_LIST_TIME + timedelta(minutes=1) + other_column = _console_conversation(session, console_data) + if "updated" in sort_by: + other_column.updated_at = _CONSOLE_LIST_TIME + timedelta(minutes=2) + else: + other_column.created_at = _CONSOLE_LIST_TIME + timedelta(minutes=2) + page = _page( + sqlite_session_factory, + console_data, + filters=replace( + _FILTERS, start=_CONSOLE_LIST_TIME, end=_CONSOLE_LIST_TIME.replace(second=59), sort_by=sort_by, limit=1 + ), + ) + assert page.total == 2 + assert page.has_next is True + assert [item.id for item in page.items] == [end.id if sort_by.startswith("-") else start.id] + next_page = _page( + sqlite_session_factory, + console_data, + filters=replace( + _FILTERS, + start=_CONSOLE_LIST_TIME, + end=_CONSOLE_LIST_TIME.replace(second=59), + sort_by=sort_by, + limit=1, + page=2, + ), + ) + assert next_page.has_next is False + assert [item.id for item in next_page.items] == [start.id if sort_by.startswith("-") else end.id] + + +def test_advanced_chat_hides_debugger_and_null_invoke_source( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData +) -> None: + with sqlite_session_factory.begin() as session: + session.get_one(App, console_data.app_id).mode = AppMode.ADVANCED_CHAT + visible = _console_conversation(session, console_data, mode=AppMode.ADVANCED_CHAT) + debugger = _console_conversation(session, console_data, mode=AppMode.ADVANCED_CHAT) + debugger.invoke_from = InvokeFrom.DEBUGGER + legacy = _console_conversation(session, console_data, mode=AppMode.ADVANCED_CHAT) + legacy.invoke_from = None + assert [item.id for item in _page(sqlite_session_factory, console_data).items] == [visible.id] + + +@pytest.mark.parametrize("completion", [False, True]) +def test_snapshots_match_existing_response_fields_and_close_all_read_sessions( + sqlite_session_factory: sessionmaker[Session], sqlite_engine: Engine, console_data: ConsoleData, completion: bool +) -> None: + with sqlite_session_factory.begin() as session: + mode = AppMode.COMPLETION if completion else AppMode.CHAT + session.get_one(App, console_data.app_id).mode = mode + row = _console_conversation(session, console_data, mode=mode) + row.override_model_configs = json.dumps( + {"model": {"name": "selected-model", "provider": "provider"}, "pre_prompt": "Prefix"} + ) + first = _console_message(session, row) + session.add( + MessageAnnotation( + app_id=console_data.app_id, + conversation_id=row.id, + question="Reviewed", + content="Annotation", + account_id=console_data.context.account_id, + ) + ) + session.add( + MessageFeedback( + app_id=console_data.app_id, + conversation_id=row.id, + message_id=first.id, + rating=FeedbackRating.LIKE, + from_source=FeedbackFromSource.USER, + ) + ) + session.add( + MessageFeedback( + app_id=console_data.app_id, + conversation_id=row.id, + message_id=first.id, + rating=FeedbackRating.DISLIKE, + from_source=FeedbackFromSource.ADMIN, + ) + ) + response_type = CompletionResponse if completion else ConversationWithSummary + with sqlite_session_factory() as session: + row = session.get_one(Conversation, row.id) + expected = response_type.model_validate(ConversationResponseSource(row, session=session)).model_dump( + mode="json" + ) + checkouts: list[int] = [] + checkins: list[int] = [] + + def checked_out(_connection: object, _record: object, _proxy: object) -> None: + checkouts.append(1) + + def checked_in(_connection: object, _record: object) -> None: + checkins.append(1) + + event.listen(sqlite_engine, "checkout", checked_out) + event.listen(sqlite_engine, "checkin", checked_in) + try: + scope = _scope(sqlite_session_factory, console_data, kind="completion" if completion else "chat") + page = ConversationRepository(session_factory=sqlite_session_factory).get_console_page( + scope=scope, filters=_FILTERS + ) + assert len(checkouts) == len(checkins) == 2 + item = page.items[0] + assert response_type.model_validate(item).model_dump(mode="json") == expected + assert len(checkouts) == 2 # Detached serialization cannot execute SQL. + assert item.model_config["model"] == expected["model_config"]["model"] + if completion: + assert isinstance(item, ConsoleCompletionConversationRecord) + assert item.first_message is not None + assert item.first_message.inputs == {"zero": 0, "empty": "", "none": None} + assert item.annotation is not None + assert item.annotation.account is not None + assert item.annotation.account.email == "maintainer@example.com" + else: + assert isinstance(item, ConsoleChatConversationRecord) + assert item.summary_or_query == "First query" + assert item.message_count == 1 + assert item.annotated is True + finally: + event.remove(sqlite_engine, "checkout", checked_out) + event.remove(sqlite_engine, "checkin", checked_in) + + +def test_empty_page_preserves_requested_page_and_limit( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData +) -> None: + assert _page( + sqlite_session_factory, console_data, filters=replace(_FILTERS, page=8, limit=5) + ) == ConsoleConversationPage( + tenant_id=console_data.context.active_workspace_id, page=8, per_page=5, total=0, has_next=False, items=() + ) + + +def test_completion_returns_raw_file_inputs_without_restoration( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData +) -> None: + # An invalid/missing upload would fail file restoration. Persistence must only + # snapshot it; the query service owns restoration after this session closes. + raw_file = { + "dify_model_identity": "__dify__file__", + "transfer_method": "local_file", + "upload_file_id": str(uuid4()), + } + with sqlite_session_factory.begin() as session: + session.get_one(App, console_data.app_id).mode = AppMode.COMPLETION + row = _console_conversation(session, console_data, mode=AppMode.COMPLETION) + message = _console_message(session, row) + message.inputs = {"attachment": raw_file} + result = ( + ConversationRepository(session_factory=sqlite_session_factory) + .get_console_page(scope=_scope(sqlite_session_factory, console_data, kind="completion"), filters=_FILTERS) + .items[0] + ) + assert isinstance(result, ConsoleCompletionConversationRecord) + assert result.first_message is not None + assert result.first_message.inputs == {"attachment": raw_file} + + +@pytest.mark.parametrize("completion", [False, True]) +def test_empty_conversation_snapshot_preserves_nullable_and_zero_fields( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, completion: bool +) -> None: + with sqlite_session_factory.begin() as session: + mode = AppMode.COMPLETION if completion else AppMode.CHAT + session.get_one(App, console_data.app_id).mode = mode + _console_conversation(session, console_data, mode=mode) + record = ( + ConversationRepository(session_factory=sqlite_session_factory) + .get_console_page( + scope=_scope(sqlite_session_factory, console_data, kind="completion" if completion else "chat"), + filters=_FILTERS, + ) + .items[0] + ) + assert record.read_at is None + assert record.user_feedback_stats == record.admin_feedback_stats == {"like": 0, "dislike": 0} + if completion: + assert isinstance(record, ConsoleCompletionConversationRecord) + assert record.first_message is None + assert record.annotation is None + else: + assert isinstance(record, ConsoleChatConversationRecord) + assert record.summary_or_query == "" + assert record.message_count == 0 + assert record.annotated is False + assert record.status_count is None + + +@pytest.mark.parametrize("foreign_app", [False, True]) +def test_chat_status_counts_preserve_duplicate_messages_and_app_scope( + sqlite_session_factory: sessionmaker[Session], console_data: ConsoleData, foreign_app: bool +) -> None: + with sqlite_session_factory.begin() as session: + row = _console_conversation(session, console_data) + run = WorkflowRun( + tenant_id=console_data.context.active_workspace_id, + app_id=str(uuid4()) if foreign_app else console_data.app_id, + workflow_id=str(uuid4()), + type=WorkflowType.CHAT, + triggered_from=WorkflowRunTriggeredFrom.APP_RUN, + version="1", + status=WorkflowExecutionStatus.SUCCEEDED, + created_by_role=CreatorUserRole.END_USER, + created_by=console_data.end_user_id, + ) + session.add(run) + session.flush() + first = _console_message(session, row) + first.workflow_run_id = run.id + second = _console_message(session, row) + second.workflow_run_id = run.id + missing = _console_message(session, row) + missing.workflow_run_id = str(uuid4()) + record = _page(sqlite_session_factory, console_data).items[0] + assert isinstance(record, ConsoleChatConversationRecord) + assert record.message_count == 3 + assert record.status_count == {"success": 0 if foreign_app else 2, "failed": 0, "partial_success": 0, "paused": 0} + + +_CONSOLE_DETAIL_TIME = datetime(2020, 1, 2, 3, 4, 5) + + +@dataclass(frozen=True) +class DetailData: + context: RequestContext + app_id: str + conversation_id: str + end_user_id: str + + +@pytest.fixture +def detail_data(sqlite_session_factory: sessionmaker[Session]) -> DetailData: + with sqlite_session_factory.begin() as session: + account = Account(name="Maintainer", email="detail-reader@example.com", timezone="UTC") + app = App(tenant_id=str(uuid4()), name="Console detail", mode=AppMode.CHAT, enable_api=True, enable_site=True) + session.add_all([account, app]) + session.flush() + end_user = EndUser(tenant_id=app.tenant_id, app_id=app.id, type=EndUserType.BROWSER, session_id="detail-user") + session.add(end_user) + session.flush() + conversation = Conversation( + app_id=app.id, + mode=AppMode.CHAT, + name="Conversation", + _inputs={}, + from_source=ConversationFromSource.API, + from_end_user_id=end_user.id, + from_account_id=None, + introduction="Introduction", + is_deleted=False, + ) + conversation.created_at = _CONSOLE_DETAIL_TIME + conversation.updated_at = _CONSOLE_DETAIL_TIME + session.add(conversation) + session.flush() + return DetailData( + context=RequestContext( + request_id="detail", trace_id=None, account_id=account.id, active_workspace_id=app.tenant_id + ), + app_id=app.id, + conversation_id=conversation.id, + end_user_id=end_user.id, + ) + + +def _detail_message(session: Session, data: DetailData) -> Message: + message = Message( + app_id=data.app_id, + conversation_id=data.conversation_id, + _inputs={"zero": 0, "none": None, "empty": []}, + query="Question", + message=[{"text": "Raw prompt"}], + message_unit_price=Decimal(0), + answer="Raw answer", + answer_unit_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.API, + from_end_user_id=data.end_user_id, + ) + message.created_at = _CONSOLE_DETAIL_TIME + session.add(message) + session.flush() + return message + + +@pytest.mark.parametrize("kind", ["chat", "completion"]) +def test_detail_materializes_after_session_closes_without_marking_read( + sqlite_session_factory: sessionmaker[Session], + sqlite_engine: Engine, + detail_data: DetailData, + kind: ConsoleConversationKind, +) -> None: + data = detail_data + with sqlite_session_factory.begin() as session: + mode = AppMode.CHAT if kind == "chat" else AppMode.COMPLETION + session.get_one(App, data.app_id).mode = mode + session.execute( + update(Conversation) + .where(Conversation.id == data.conversation_id) + .values({Conversation.mode: mode, Conversation.updated_at: _CONSOLE_DETAIL_TIME}) + ) + _detail_message(session, data) + events: list[str] = [] + + @event.listens_for(sqlite_engine, "checkout") + def checkout(_connection: object, _record: object, _proxy: object) -> None: + events.append("checkout") + + @event.listens_for(sqlite_engine, "checkin") + def checkin(_connection: object, _record: object) -> None: + events.append("checkin") + + @event.listens_for(sqlite_engine, "commit") + def commit(_connection: object) -> None: + events.append("commit") + + source = ConversationRepository(session_factory=sqlite_session_factory).get_console_detail( + data.context, data.app_id, data.conversation_id, kind=kind + ) + assert events == ["checkout", "checkin"] + assert source.tenant_id == data.context.active_workspace_id + assert source.record.id == data.conversation_id + assert source.record.created_at == _CONSOLE_DETAIL_TIME + assert source.record.from_end_user_id == data.end_user_id + assert source.files == () + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + assert stored.read_at is None + assert stored.read_account_id is None + assert stored.updated_at == _CONSOLE_DETAIL_TIME + + +@pytest.mark.parametrize("actor", ["current_account", "other_account", "end_user"]) +@pytest.mark.parametrize("soft_deleted", [False, True]) +def test_detail_and_mark_are_app_wide_even_for_other_actors_and_soft_deleted_rows( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData, actor: str, soft_deleted: bool +) -> None: + data = detail_data + with sqlite_session_factory.begin() as session: + conversation = session.get_one(Conversation, data.conversation_id) + conversation.is_deleted = soft_deleted + if actor != "end_user": + conversation.from_source = ConversationFromSource.CONSOLE + conversation.from_end_user_id = None + conversation.from_account_id = data.context.account_id if actor == "current_account" else str(uuid4()) + repository = ConversationRepository(session_factory=sqlite_session_factory) + source = repository.get_console_detail(data.context, data.app_id, data.conversation_id, kind="chat") + assert source.record.id == data.conversation_id + repository.mark_console_read(data.context, data.app_id, data.conversation_id, kind="chat") + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + assert stored.read_account_id == data.context.account_id + assert stored.read_at is not None + assert stored.is_deleted is soft_deleted + + +@pytest.mark.parametrize("operation", ["detail", "mark"]) +@pytest.mark.parametrize("change", ["missing", "tenant", "status", "mode", "hidden_agent"]) +def test_detail_and_mark_reject_unavailable_app_without_writing( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData, operation: str, change: str +) -> None: + data = detail_data + with sqlite_session_factory.begin() as session: + app = session.get_one(App, data.app_id) + if change == "missing": + session.delete(app) + elif change == "tenant": + app.tenant_id = str(uuid4()) + elif change == "status": + session.execute(text("UPDATE apps SET status = 'disabled' WHERE id = :id"), {"id": app.id}) + elif change == "mode": + app.mode = AppMode.COMPLETION + else: + app.mode = AppMode.AGENT + session.add( + Agent( + tenant_id=app.tenant_id, + name="Hidden workflow agent", + scope=AgentScope.WORKFLOW_ONLY, + source=AgentSource.WORKFLOW, + backing_app_id=app.id, + ) + ) + repository = ConversationRepository(session_factory=sqlite_session_factory) + query = repository.get_console_detail if operation == "detail" else repository.mark_console_read + with pytest.raises(ConsoleAppNotFoundError): + query(data.context, data.app_id, data.conversation_id, kind="chat") + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + assert stored.read_at is None + assert stored.read_account_id is None + assert stored.updated_at == _CONSOLE_DETAIL_TIME + + +@pytest.mark.parametrize("operation", ["detail", "mark"]) +@pytest.mark.parametrize("change", ["missing", "other_app"]) +def test_detail_and_mark_scope_conversation_id_to_app( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData, operation: str, change: str +) -> None: + data = detail_data + conversation_id = str(uuid4()) if change == "missing" else data.conversation_id + if change == "other_app": + with sqlite_session_factory.begin() as session: + session.get_one(Conversation, data.conversation_id).app_id = str(uuid4()) + repository = ConversationRepository(session_factory=sqlite_session_factory) + query = repository.get_console_detail if operation == "detail" else repository.mark_console_read + with pytest.raises(ConversationNotExistsError): + query(data.context, data.app_id, conversation_id, kind="chat") + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + assert stored.read_at is None + assert stored.read_account_id is None + + +def test_mark_commits_first_reader_without_changing_update_time_or_overwriting_on_repeat( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData +) -> None: + data = detail_data + repository = ConversationRepository(session_factory=sqlite_session_factory) + before = naive_utc_now() + repository.mark_console_read(data.context, data.app_id, data.conversation_id, kind="chat") + after = naive_utc_now() + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + first_read = stored.read_at + assert first_read is not None + assert before <= first_read <= after + assert stored.read_account_id == data.context.account_id + assert stored.updated_at == _CONSOLE_DETAIL_TIME + other_context = data.context._replace(account_id=str(uuid4())) + repository.mark_console_read(other_context, data.app_id, data.conversation_id, kind="chat") + with sqlite_session_factory() as session: + stored = session.get_one(Conversation, data.conversation_id) + assert stored.read_at == first_read + assert stored.read_account_id == data.context.account_id + assert stored.updated_at == _CONSOLE_DETAIL_TIME + + +def test_completion_detail_keeps_raw_attachment_references_and_answer_without_external_io( + sqlite_session_factory: sessionmaker[Session], sqlite_engine: Engine, detail_data: DetailData +) -> None: + data = detail_data + raw_file = { + "dify_model_identity": FILE_MODEL_IDENTITY, + "transfer_method": "remote_url", + "type": "image", + "url": "https://example.invalid/unresolved-image.png", + } + raw_answer = "![image](https://example.invalid/files/tools/example.png?timestamp=1&nonce=n&sign=s)" + with sqlite_session_factory.begin() as session: + session.get_one(App, data.app_id).mode = AppMode.COMPLETION + session.get_one(Conversation, data.conversation_id).mode = AppMode.COMPLETION + message = _detail_message(session, data) + message.inputs = {"file": raw_file, "files": [raw_file], "zero": 0, "none": None} + message.answer = raw_answer + remote = MessageFile( + message_id=message.id, + type=FileType.IMAGE, + transfer_method=FileTransferMethod.REMOTE_URL, + created_by_role=CreatorUserRole.END_USER, + created_by=data.end_user_id, + url=raw_file["url"], + belongs_to=MessageFileBelongsTo.USER, + ) + session.add(remote) + commits: list[object] = [] + + @event.listens_for(sqlite_engine, "commit") + def record_commit(connection: object) -> None: + commits.append(connection) + + source = ConversationRepository(session_factory=sqlite_session_factory).get_console_detail( + data.context, data.app_id, data.conversation_id, kind="completion" + ) + assert isinstance(source.record, ConsoleCompletionConversationDetail) + first = source.record.first_message + assert first is not None + assert first.id == message.id + assert first.inputs == {"file": raw_file, "files": [raw_file], "zero": 0, "none": None} + assert first.answer == raw_answer + assert first.message_files == [] + assert len(source.files) == 1 + reference = source.files[0] + assert reference.id == remote.id + assert reference.type == "image" + assert reference.transfer_method == "remote_url" + assert reference.url == raw_file["url"] + assert reference.upload_file_id is None + assert reference.belongs_to == "user" + assert commits == [] + + +def test_completion_detail_with_no_messages_returns_no_message_or_files( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData +) -> None: + data = detail_data + with sqlite_session_factory.begin() as session: + session.get_one(App, data.app_id).mode = AppMode.COMPLETION + session.get_one(Conversation, data.conversation_id).mode = AppMode.COMPLETION + source = ConversationRepository(session_factory=sqlite_session_factory).get_console_detail( + data.context, data.app_id, data.conversation_id, kind="completion" + ) + assert isinstance(source.record, ConsoleCompletionConversationDetail) + assert source.record.first_message is None + assert source.files == () + + +@dataclass(frozen=True) +class ConsoleDeletionData: + context: RequestContext + app_id: str + conversation_id: str + workspace_ids: tuple[str, ...] + binding_id: str + + +@pytest.fixture +def deletion_data(sqlite_session_factory: sessionmaker[Session]) -> ConsoleDeletionData: + with sqlite_session_factory.begin() as session: + account = Account(name="Maintainer", email="maintainer@example.com", timezone="UTC") + app = App(tenant_id=str(uuid4()), name="Chat", mode=AppMode.CHAT, enable_api=True, enable_site=True) + session.add_all([account, app]) + session.flush() + conversation = Conversation( + app_id=app.id, + mode=AppMode.CHAT, + name="Owned conversation", + _inputs={}, + from_source=ConversationFromSource.CONSOLE, + from_account_id=account.id, + from_end_user_id=None, + is_deleted=False, + ) + session.add(conversation) + session.flush() + workspaces = [ + AgentWorkspace( + tenant_id=app.tenant_id, + app_id=app.id, + owner_type=AgentWorkspaceOwnerType.CONVERSATION, + owner_id=conversation.id, + owner_scope_key=scope, + backend_workspace_ref=f"workspace-{scope}", + status=AgentWorkingResourceStatus.ACTIVE, + active_guard=1, + ) + for scope in ("root", "node:workflow-binding") + ] + session.add_all(workspaces) + session.flush() + binding = AgentWorkspaceBinding( + tenant_id=app.tenant_id, + app_id=app.id, + workspace_id=workspaces[0].id, + agent_id=str(uuid4()), + agent_config_version_id=str(uuid4()), + agent_config_version_kind=AgentConfigVersionKind.SNAPSHOT, + backend_binding_ref="binding", + status=AgentWorkingResourceStatus.ACTIVE, + ) + session.add(binding) + session.flush() + conversation.agent_workspace_binding_id = binding.id + return ConsoleDeletionData( + context=RequestContext( + request_id="test", trace_id=None, account_id=account.id, active_workspace_id=app.tenant_id + ), + app_id=app.id, + conversation_id=conversation.id, + workspace_ids=tuple(workspace.id for workspace in workspaces), + binding_id=binding.id, + ) + + +def _assert_active(factory: sessionmaker[Session], data: ConsoleDeletionData) -> None: + with factory() as session: + assert session.get_one(Conversation, data.conversation_id).is_deleted is False + for workspace_id in data.workspace_ids: + workspace = session.get_one(AgentWorkspace, workspace_id) + assert workspace.status == AgentWorkingResourceStatus.ACTIVE + assert workspace.active_guard == 1 + assert workspace.retired_at is None + binding = session.get_one(AgentWorkspaceBinding, data.binding_id) + assert binding.status == AgentWorkingResourceStatus.ACTIVE + assert binding.retired_at is None + + +@pytest.mark.parametrize( + ("kind", "mode"), + [ + ("completion", AppMode.COMPLETION), + ("chat", AppMode.CHAT), + ("chat", AppMode.AGENT_CHAT), + ("chat", AppMode.ADVANCED_CHAT), + ("chat", AppMode.AGENT), + ], +) +def test_delete_commits_retirement_and_returns_cleanup_references( + sqlite_session_factory: sessionmaker[Session], + deletion_data: ConsoleDeletionData, + kind: ConsoleConversationKind, + mode: AppMode, +) -> None: + with sqlite_session_factory.begin() as session: + session.get_one(App, deletion_data.app_id).mode = mode + repository = ConversationRepository(session_factory=sqlite_session_factory) + + deleted = repository.delete_console( + deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind=kind + ) + + assert deleted.tenant_id == deletion_data.context.active_workspace_id + assert deleted.conversation_id == deletion_data.conversation_id + assert set(deleted.retired_workspace_ids) == set(deletion_data.workspace_ids) + with sqlite_session_factory() as session: + assert session.get_one(Conversation, deletion_data.conversation_id).is_deleted is True + for workspace_id in deletion_data.workspace_ids: + workspace = session.get_one(AgentWorkspace, workspace_id) + assert workspace.status == AgentWorkingResourceStatus.RETIRED + assert workspace.active_guard is None + assert workspace.retired_at is not None + binding = session.get_one(AgentWorkspaceBinding, deletion_data.binding_id) + assert binding.status == AgentWorkingResourceStatus.RETIRED + assert binding.retired_at is not None + with pytest.raises(ConversationNotExistsError): + repository.delete_console(deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind=kind) + + +def test_delete_conversation_without_agent_workspaces( + sqlite_session_factory: sessionmaker[Session], deletion_data: ConsoleDeletionData +) -> None: + with sqlite_session_factory.begin() as session: + session.get_one(Conversation, deletion_data.conversation_id).agent_workspace_binding_id = None + session.delete(session.get_one(AgentWorkspaceBinding, deletion_data.binding_id)) + for workspace_id in deletion_data.workspace_ids: + session.delete(session.get_one(AgentWorkspace, workspace_id)) + + deleted = ConversationRepository(session_factory=sqlite_session_factory).delete_console( + deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind="chat" + ) + + assert deleted.retired_workspace_ids == () + with sqlite_session_factory() as session: + assert session.get_one(Conversation, deletion_data.conversation_id).is_deleted is True + + +@pytest.mark.parametrize("change", ["missing", "tenant", "status", "mode", "hidden_agent"]) +@pytest.mark.parametrize("kind", ["completion", "chat"]) +def test_delete_rejects_unavailable_or_hidden_app_without_retiring_conversation( + sqlite_session_factory: sessionmaker[Session], + deletion_data: ConsoleDeletionData, + change: str, + kind: ConsoleConversationKind, +) -> None: + with sqlite_session_factory.begin() as session: + app = session.get_one(App, deletion_data.app_id) + app.mode = AppMode.COMPLETION if kind == "completion" else AppMode.CHAT + if change == "missing": + session.delete(app) + elif change == "tenant": + app.tenant_id = str(uuid4()) + elif change == "status": + session.execute(text("UPDATE apps SET status = 'disabled' WHERE id = :id"), {"id": app.id}) + elif change == "mode": + app.mode = AppMode.CHAT if kind == "completion" else AppMode.COMPLETION + else: + app.mode = AppMode.AGENT + session.add( + Agent( + tenant_id=app.tenant_id, + name="Hidden", + scope=AgentScope.WORKFLOW_ONLY, + source=AgentSource.WORKFLOW, + backing_app_id=app.id, + status=AgentStatus.ARCHIVED, + ) + ) + with pytest.raises(ConsoleAppNotFoundError): + ConversationRepository(session_factory=sqlite_session_factory).delete_console( + deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind=kind + ) + _assert_active(sqlite_session_factory, deletion_data) + + +@pytest.mark.parametrize("change", ["app", "account", "source", "end_user", "deleted", "missing"]) +def test_delete_requires_exact_console_conversation_owner( + sqlite_session_factory: sessionmaker[Session], deletion_data: ConsoleDeletionData, change: str +) -> None: + conversation_id = deletion_data.conversation_id + with sqlite_session_factory.begin() as session: + conversation = session.get_one(Conversation, conversation_id) + if change == "app": + conversation.app_id = str(uuid4()) + elif change == "account": + conversation.from_account_id = str(uuid4()) + elif change == "source": + conversation.from_source = ConversationFromSource.API + elif change == "end_user": + conversation.from_end_user_id = str(uuid4()) + elif change == "deleted": + conversation.is_deleted = True + else: + conversation_id = str(uuid4()) + with pytest.raises(ConversationNotExistsError): + ConversationRepository(session_factory=sqlite_session_factory).delete_console( + deletion_data.context, deletion_data.app_id, conversation_id, kind="chat" + ) + with sqlite_session_factory() as session: + conversation = session.get_one(Conversation, deletion_data.conversation_id) + assert conversation.is_deleted is (change == "deleted") + for workspace_id in deletion_data.workspace_ids: + assert session.get_one(AgentWorkspace, workspace_id).status == AgentWorkingResourceStatus.ACTIVE + + +def test_wrong_binding_owner_prevents_all_lifecycle_changes( + sqlite_session_factory: sessionmaker[Session], deletion_data: ConsoleDeletionData +) -> None: + with sqlite_session_factory.begin() as session: + session.get_one(AgentWorkspace, deletion_data.workspace_ids[0]).owner_id = str(uuid4()) + with pytest.raises(AgentWorkspaceNotFoundError, match="Binding is unavailable"): + ConversationRepository(session_factory=sqlite_session_factory).delete_console( + deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind="chat" + ) + _assert_active(sqlite_session_factory, deletion_data) + + +def test_failed_database_write_rolls_back_workspace_and_binding_retirement( + sqlite_session_factory: sessionmaker[Session], deletion_data: ConsoleDeletionData +) -> None: + with sqlite_session_factory.begin() as session: + session.execute( + text( + "CREATE TRIGGER reject_conversation_deletion BEFORE UPDATE OF is_deleted ON conversations " + "WHEN NEW.is_deleted = 1 BEGIN SELECT RAISE(ABORT, 'conversation deletion rejected'); END" + ) + ) + try: + with pytest.raises(IntegrityError, match="conversation deletion rejected"): + ConversationRepository(session_factory=sqlite_session_factory).delete_console( + deletion_data.context, deletion_data.app_id, deletion_data.conversation_id, kind="chat" + ) + _assert_active(sqlite_session_factory, deletion_data) + finally: + with sqlite_session_factory.begin() as session: + session.execute(text("DROP TRIGGER reject_conversation_deletion")) diff --git a/api/tests/unit_tests/services/test_conversation_command_service.py b/api/tests/unit_tests/services/test_conversation_command_service.py index 0c0bd3cf393..fdefc2a55ae 100644 --- a/api/tests/unit_tests/services/test_conversation_command_service.py +++ b/api/tests/unit_tests/services/test_conversation_command_service.py @@ -1,13 +1,21 @@ """Conversation commands exercise persistence, ownership and cleanup ordering.""" -from dataclasses import replace +from dataclasses import dataclass, replace from uuid import uuid4 import pytest from sqlalchemy import Engine, delete, event, select, update from sqlalchemy.orm import Session, sessionmaker -from models.agent import AgentWorkingResourceStatus, AgentWorkspace, AgentWorkspaceOwnerType +from machinery.context import RequestContext +from models.account import Account +from models.agent import ( + AgentConfigVersionKind, + AgentWorkingResourceStatus, + AgentWorkspace, + AgentWorkspaceBinding, + AgentWorkspaceOwnerType, +) from models.enums import ConversationFromSource, CreatorUserRole, EndUserType from models.model import App, AppMode, Conversation, EndUser from models.web import PinnedConversation @@ -285,3 +293,172 @@ def test_end_user_existing_pin_and_unpin_do_not_require_live_conversation( service.set_pinned(ref=end_user_ref, is_pinned=False) with sqlite_session_factory() as session: assert session.scalars(select(PinnedConversation)).all() == [] + + +@dataclass(frozen=True) +class ConsoleDeletionData: + context: RequestContext + app_id: str + conversation_id: str + workspace_ids: tuple[str, ...] + binding_ids: tuple[str, ...] + + +@pytest.fixture +def console_deletion(sqlite_session_factory: sessionmaker[Session]) -> ConsoleDeletionData: + context = RequestContext( + request_id="console-delete", trace_id=None, account_id=str(uuid4()), active_workspace_id=str(uuid4()) + ) + with sqlite_session_factory.begin() as session: + app = App( + tenant_id=context.active_workspace_id, name="Chat", mode=AppMode.CHAT, enable_api=True, enable_site=True + ) + account = Account(name="Owner", email="owner@example.com") + account.id = context.account_id + session.add_all([app, account]) + session.flush() + conversation = Conversation( + app_id=app.id, + mode=AppMode.CHAT, + name="Console conversation", + _inputs={}, + from_source=ConversationFromSource.CONSOLE, + from_account_id=context.account_id, + from_end_user_id=None, + is_deleted=False, + ) + session.add(conversation) + session.flush() + workspaces: list[AgentWorkspace] = [] + bindings: list[AgentWorkspaceBinding] = [] + for owner_scope_key in ("root", "node-1:workflow-binding"): + workspace = AgentWorkspace( + id=str(uuid4()), + tenant_id=app.tenant_id, + app_id=app.id, + owner_type=AgentWorkspaceOwnerType.CONVERSATION, + owner_id=conversation.id, + owner_scope_key=owner_scope_key, + backend_workspace_ref=owner_scope_key, + status=AgentWorkingResourceStatus.ACTIVE, + active_guard=1, + ) + binding = AgentWorkspaceBinding( + id=str(uuid4()), + tenant_id=app.tenant_id, + app_id=app.id, + workspace_id=workspace.id, + agent_id=str(uuid4()), + agent_config_version_id=str(uuid4()), + agent_config_version_kind=AgentConfigVersionKind.SNAPSHOT, + backend_binding_ref=owner_scope_key, + status=AgentWorkingResourceStatus.ACTIVE, + ) + session.add_all([workspace, binding]) + workspaces.append(workspace) + bindings.append(binding) + conversation.agent_workspace_binding_id = bindings[0].id + return ConsoleDeletionData( + context=context, + app_id=app.id, + conversation_id=conversation.id, + workspace_ids=tuple(workspace.id for workspace in workspaces), + binding_ids=tuple(binding.id for binding in bindings), + ) + + +@pytest.mark.parametrize("has_root_binding", [False, True]) +def test_console_delete_retires_all_workspaces_and_returns_connection_before_cleanup( + sqlite_session_factory: sessionmaker[Session], + sqlite_engine: Engine, + console_deletion: ConsoleDeletionData, + has_root_binding: bool, +) -> None: + data = console_deletion + if not has_root_binding: + with sqlite_session_factory.begin() as session: + session.execute( + update(Conversation) + .where(Conversation.id == data.conversation_id) + .values(agent_workspace_binding_id=None) + ) + events: list[str] = [] + + @event.listens_for(sqlite_engine, "checkout") + def checkout(_connection: object, _record: object, _proxy: object) -> None: + events.append("checkout") + + @event.listens_for(sqlite_engine, "checkin") + def checkin(_connection: object, _record: object) -> None: + events.append("checkin") + + def cleanup(*, tenant_id: str, conversation_id: str, retired_workspace_ids: tuple[str, ...]) -> None: + assert events == ["checkout", "checkin"] + assert (tenant_id, conversation_id) == (data.context.active_workspace_id, data.conversation_id) + assert set(retired_workspace_ids) == set(data.workspace_ids) + events.append("cleanup") + with sqlite_session_factory() as session: + assert session.get_one(Conversation, conversation_id).is_deleted is True + for workspace_id in data.workspace_ids: + workspace = session.get_one(AgentWorkspace, workspace_id) + assert workspace.status == AgentWorkingResourceStatus.RETIRED + assert workspace.active_guard is None + assert workspace.retired_at is not None + for binding_id in data.binding_ids: + binding = session.get_one(AgentWorkspaceBinding, binding_id) + assert binding.status == AgentWorkingResourceStatus.RETIRED + assert binding.retired_at is not None + + service = _service(sqlite_session_factory, cleanup=cleanup) + service.delete_console(data.context, data.app_id, data.conversation_id, kind="chat") + assert events == ["checkout", "checkin", "cleanup", "checkout", "checkin"] + with pytest.raises(ConversationNotExistsError): + service.delete_console(data.context, data.app_id, data.conversation_id, kind="chat") + assert events.count("cleanup") == 1 + + +def test_console_delete_failed_commit_rolls_back_root_and_node_resources_without_cleanup( + sqlite_session_factory: sessionmaker[Session], sqlite_engine: Engine, console_deletion: ConsoleDeletionData +) -> None: + data = console_deletion + factory = sessionmaker(bind=sqlite_engine) + + @event.listens_for(factory, "before_commit") + def fail_commit(session: Session) -> None: + # Flush actual writes before the failure to exercise transaction rollback. + session.flush() + raise RuntimeError("Database commit failed") + + with pytest.raises(RuntimeError, match="Database commit failed"): + _service(factory).delete_console(data.context, data.app_id, data.conversation_id, kind="chat") + with sqlite_session_factory() as session: + assert session.get_one(Conversation, data.conversation_id).is_deleted is False + for workspace_id in data.workspace_ids: + workspace = session.get_one(AgentWorkspace, workspace_id) + assert workspace.status == AgentWorkingResourceStatus.ACTIVE + assert workspace.active_guard == 1 + assert workspace.retired_at is None + for binding_id in data.binding_ids: + binding = session.get_one(AgentWorkspaceBinding, binding_id) + assert binding.status == AgentWorkingResourceStatus.ACTIVE + assert binding.retired_at is None + + +def test_console_cleanup_failure_cannot_rollback_committed_deletion( + sqlite_session_factory: sessionmaker[Session], console_deletion: ConsoleDeletionData +) -> None: + data = console_deletion + + def cleanup(*, tenant_id: str, conversation_id: str, retired_workspace_ids: tuple[str, ...]) -> None: + assert (tenant_id, conversation_id) == (data.context.active_workspace_id, data.conversation_id) + assert set(retired_workspace_ids) == set(data.workspace_ids) + raise RuntimeError("Cleanup transport unavailable") + + with pytest.raises(RuntimeError, match="Cleanup transport unavailable"): + _service(sqlite_session_factory, cleanup=cleanup).delete_console( + data.context, data.app_id, data.conversation_id, kind="chat" + ) + with sqlite_session_factory() as session: + assert session.get_one(Conversation, data.conversation_id).is_deleted is True + for workspace_id in data.workspace_ids: + assert session.get_one(AgentWorkspace, workspace_id).status == AgentWorkingResourceStatus.RETIRED diff --git a/api/tests/unit_tests/services/test_conversation_query_service.py b/api/tests/unit_tests/services/test_conversation_query_service.py index 6b97739774c..2e5e9b2a49c 100644 --- a/api/tests/unit_tests/services/test_conversation_query_service.py +++ b/api/tests/unit_tests/services/test_conversation_query_service.py @@ -1,22 +1,41 @@ """Conversation listing materializes data before restoring file inputs.""" -from collections.abc import Callable, Mapping +from collections.abc import Callable, Generator, Iterator, Mapping +from contextlib import contextmanager from copy import deepcopy -from dataclasses import dataclass +from dataclasses import dataclass, field +from datetime import datetime +from decimal import Decimal +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from threading import Thread +from typing import override +from urllib.parse import parse_qs, urlsplit from uuid import uuid4 import pytest from pydantic import JsonValue -from sqlalchemy import Engine, event, select -from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy import Connection, Engine, event, select +from sqlalchemy.orm import Session, SessionTransaction, sessionmaker -from graphon.file import FILE_MODEL_IDENTITY, File -from models.enums import ConversationFromSource, EndUserType, InvokeFrom -from models.model import App, AppMode, Conversation, EndUser, InstalledApp +from core.helper.http_client_pooling import close_all_pooled_clients +from core.tools.signature import verify_tool_file_signature +from core.workflow.file_reference import build_file_reference, resolve_file_record_id +from extensions.storage.storage_type import StorageType +from fields.conversation_fields import ConversationMessageDetail +from fields.conversation_fields import MessageFile as MessageFileResponse +from graphon.file import FILE_MODEL_IDENTITY, File, FileTransferMethod, FileType +from machinery.context import RequestContext +from models import MessageFile, ToolFile, UploadFile +from models.enums import ConversationFromSource, CreatorUserRole, EndUserType, InvokeFrom, MessageFileBelongsTo +from models.model import App, AppMode, Conversation, EndUser, InstalledApp, Message from models.utils.file_input_compat import restore_input_files from repositories.conversation_repository import ConversationRepository from services.conversation_query_service import ConversationQueryService from services.entities.conversation_entities import ( + ConsoleCompletionConversationDetail, + ConsoleCompletionConversationRecord, + ConsoleConversationFilters, + ConsoleConversationScope, ConversationInputValue, ConversationListSource, ConversationScope, @@ -24,6 +43,8 @@ from services.entities.conversation_entities import ( InstalledAppConversationScope, ) from services.installed_app_access_service import InstalledAppRef +from services.message_query_adapters import MessageFileResolver +from tests.helpers.unit.model_factories import make_account, make_app, make_conversation, make_message @dataclass(frozen=True) @@ -145,7 +166,9 @@ def test_query_restores_real_file_inputs_after_returning_database_connection( return restore_input_files(inputs=inputs, tenant_resolver=tenant_resolver) service = ConversationQueryService( - conversations=ConversationRepository(session_factory=sqlite_session_factory), restore_inputs=restore + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=sqlite_session_factory), + restore_inputs=restore, ) result = service.get_page( scope=query_context.scope, @@ -226,7 +249,9 @@ def test_invalid_file_input_error_propagates_after_repository_connection_is_clos conversation.inputs = {"file": {"dify_model_identity": FILE_MODEL_IDENTITY, "transfer_method": "invalid"}} connection_events = _record_connections(sqlite_engine) service = ConversationQueryService( - conversations=ConversationRepository(session_factory=sqlite_session_factory), restore_inputs=restore_input_files + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=sqlite_session_factory), + restore_inputs=restore_input_files, ) with pytest.raises(ValueError, match="'invalid' is not a valid FileTransferMethod"): service.get_page( @@ -242,3 +267,424 @@ def test_invalid_file_input_error_propagates_after_repository_connection_is_clos assert ( session.scalar(select(Conversation.id).where(Conversation.id == query_context.conversation_id)) is not None ) + + +@pytest.fixture +def console_scope(sqlite_session_factory: sessionmaker[Session]) -> ConsoleConversationScope: + with sqlite_session_factory.begin() as session: + session.add(make_account(timezone="Asia/Shanghai")) + session.add(make_app(mode=AppMode.COMPLETION)) + session.add(make_conversation(mode=AppMode.COMPLETION, inputs={}, from_source=ConversationFromSource.API)) + session.add( + make_message( + inputs={}, + query="Question", + message=[{"text": "Prompt"}], + answer="Answer", + message_unit_price=Decimal(0), + answer_unit_price=Decimal(0), + total_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.API, + ) + ) + return ConversationRepository(session_factory=sqlite_session_factory).get_console_scope( + RequestContext(request_id="test", trace_id=None, account_id="account-1", active_workspace_id="tenant-1"), + "app-1", + kind="completion", + ) + + +def _console_filters() -> ConsoleConversationFilters: + return ConsoleConversationFilters( + keyword=None, start=None, end=None, annotation_status="all", sort_by="-created_at", page=1, limit=20 + ) + + +def test_console_message_file_inputs_restore_after_all_database_projections_complete( + sqlite_session_factory: sessionmaker[Session], sqlite_engine: Engine, console_scope: ConsoleConversationScope +) -> None: + file_mapping: dict[str, JsonValue] = { + "dify_model_identity": FILE_MODEL_IDENTITY, + "transfer_method": "remote_url", + "type": "document", + "url": "https://example.com/report.pdf", + "filename": "report.pdf", + "extension": ".pdf", + "mime_type": "application/pdf", + "size": 1, + } + inputs: dict[str, ConversationInputValue] = {"file": file_mapping, "zero": 0, "none": None} + with sqlite_session_factory.begin() as session: + message = session.get_one(Message, "message-1") + message.inputs = deepcopy(inputs) + connection_events = _record_connections(sqlite_engine) + commits: list[bool] = [] + + @event.listens_for(sqlite_session_factory, "after_commit") + def committed(_session: Session) -> None: + commits.append(True) + + def restore( + *, inputs: Mapping[str, ConversationInputValue], tenant_resolver: Callable[[], str] + ) -> dict[str, ConversationInputValue]: + assert connection_events == ["checkout", "checkin"] + assert tenant_resolver() == console_scope.tenant_id + connection_events.append("restore") + return restore_input_files(inputs=inputs, tenant_resolver=tenant_resolver) + + service = ConversationQueryService( + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=sqlite_session_factory), + restore_inputs=restore, + ) + page = service.get_console_page(scope=console_scope, filters=_console_filters()) + + assert console_scope.timezone == "Asia/Shanghai" + assert connection_events == ["checkout", "checkin", "restore"] + assert commits == [] + record = page.items[0] + assert isinstance(record, ConsoleCompletionConversationRecord) + assert record.first_message is not None + file = record.first_message.inputs["file"] + assert isinstance(file, File) + assert file.remote_url == "https://example.com/report.pdf" + assert record.first_message.inputs["zero"] == 0 + assert record.first_message.inputs["none"] is None + with sqlite_session_factory() as session: + message = session.get_one(Message, "message-1") + assert message._inputs == inputs + + +def test_console_invalid_file_metadata_fails_after_connection_release( + sqlite_session_factory: sessionmaker[Session], sqlite_engine: Engine, console_scope: ConsoleConversationScope +) -> None: + with sqlite_session_factory.begin() as session: + message = session.get_one(Message, "message-1") + message.inputs = {"file": {"dify_model_identity": FILE_MODEL_IDENTITY, "transfer_method": "invalid"}} + connection_events = _record_connections(sqlite_engine) + service = ConversationQueryService( + files=MessageFileResolver(), + conversations=ConversationRepository(session_factory=sqlite_session_factory), + restore_inputs=restore_input_files, + ) + + with pytest.raises(ValueError, match="'invalid' is not a valid FileTransferMethod"): + service.get_console_page(scope=console_scope, filters=_console_filters()) + + assert connection_events == ["checkout", "checkin"] + + +@pytest.fixture +def _file_configuration(config_overrides: Callable[..., None]) -> Iterator[None]: + config_overrides( + FILES_URL="https://files.example.test", + SECRET_KEY="conversation-detail-file-test", + SSRF_PROXY_HTTP_URL="", + SSRF_PROXY_HTTPS_URL="", + SSRF_PROXY_ALL_URL="", + ) + close_all_pooled_clients() + yield + close_all_pooled_clients() + + +@dataclass(frozen=True) +class DetailData: + context: RequestContext + app_id: str + conversation_id: str + message_id: str + upload_id: str + tool_id: str + legacy_file_id: str + inputs: dict[str, JsonValue] + + +def _file_input(record_id: str, method: str) -> dict[str, JsonValue]: + return { + "dify_model_identity": FILE_MODEL_IDENTITY, + "transfer_method": method, + "reference": build_file_reference(record_id=record_id), + "type": "document", + "filename": "stale.json", + "mime_type": "application/json", + "size": 999, + } + + +@pytest.fixture +def detail_data(sqlite_session_factory: sessionmaker[Session], _file_configuration: None) -> DetailData: + tenant_id, account_id, app_id = str(uuid4()), str(uuid4()), str(uuid4()) + conversation_id, message_id = str(uuid4()), str(uuid4()) + with sqlite_session_factory.begin() as session: + upload = UploadFile( + tenant_id=tenant_id, + storage_type=StorageType.LOCAL, + key="uploads/canonical.txt", + name="canonical.txt", + size=17, + extension="txt", + mime_type="text/plain", + created_by_role=CreatorUserRole.ACCOUNT, + created_by=account_id, + created_at=datetime(2026, 10, 13), + used=True, + ) + tool = ToolFile( + user_id=account_id, + tenant_id=tenant_id, + conversation_id=conversation_id, + file_key="tools/result.pdf", + mimetype="application/pdf", + name="result.pdf", + size=23, + ) + session.add_all([upload, tool]) + session.flush() + mapping = _file_input(upload.id, "local_file") + inputs: dict[str, JsonValue] = { + "upload": mapping, + "tools": [_file_input(tool.id, "tool_file")], + "mixed": [mapping, "leave unchanged"], + "empty": [], + "zero": 0, + "false": False, + "none": None, + } + session.add_all( + [ + make_account(account_id=account_id), + make_app(app_id=app_id, tenant_id=tenant_id, mode=AppMode.COMPLETION), + make_conversation( + conversation_id=conversation_id, + app_id=app_id, + mode=AppMode.COMPLETION, + inputs={}, + from_source=ConversationFromSource.CONSOLE, + from_account_id=account_id, + ), + make_message( + message_id=message_id, + app_id=app_id, + conversation_id=conversation_id, + inputs=inputs, + query="Files", + message={"text": "Prompt"}, + answer="Answer", + message_unit_price=Decimal(0), + answer_unit_price=Decimal(0), + currency="USD", + from_source=ConversationFromSource.CONSOLE, + from_account_id=account_id, + ), + ] + ) + legacy = MessageFile( + message_id=message_id, + type=FileType.DOCUMENT, + transfer_method=FileTransferMethod.TOOL_FILE, + url=f"https://old.example/files/tools/{tool.id}.pdf", + upload_file_id=None, + belongs_to=MessageFileBelongsTo.ASSISTANT, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=account_id, + ) + session.add_all( + [ + legacy, + MessageFile( + message_id=message_id, + type=FileType.DOCUMENT, + transfer_method=FileTransferMethod.LOCAL_FILE, + upload_file_id=upload.id, + belongs_to=MessageFileBelongsTo.USER, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=account_id, + ), + ] + ) + session.flush() + return DetailData( + context=RequestContext( + request_id="detail-test", trace_id=None, account_id=account_id, active_workspace_id=tenant_id + ), + app_id=app_id, + conversation_id=conversation_id, + message_id=message_id, + upload_id=upload.id, + tool_id=tool.id, + legacy_file_id=legacy.id, + inputs=inputs, + ) + + +def _detail(factory: sessionmaker[Session], data: DetailData) -> ConsoleCompletionConversationDetail: + result = ConversationQueryService( + conversations=ConversationRepository(session_factory=factory), + restore_inputs=restore_input_files, + files=MessageFileResolver(), + ).get_console_detail(data.context, data.app_id, data.conversation_id, kind="completion") + assert isinstance(result, ConsoleCompletionConversationDetail) + return result + + +@dataclass +class DatabaseReads: + sessions: list[Session] = field(default_factory=list) + active_counts: list[int] = field(default_factory=list) + commits: list[Session] = field(default_factory=list) + + +@contextmanager +def _observe_reads(factory: sessionmaker[Session]) -> Generator[DatabaseReads]: + observed = DatabaseReads() + + def begin(session: Session, _transaction: SessionTransaction, _connection: Connection) -> None: + observed.sessions.append(session) + observed.active_counts.append(sum(item.in_transaction() for item in observed.sessions)) + + def committed(session: Session) -> None: + observed.commits.append(session) + + event.listen(factory, "after_begin", begin) + event.listen(factory, "after_commit", committed) + try: + yield observed + finally: + event.remove(factory, "after_begin", begin) + event.remove(factory, "after_commit", committed) + + +def test_detail_restores_files_without_overlapping_sessions_or_persisting_legacy_fallback( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData +) -> None: + with _observe_reads(sqlite_session_factory) as reads: + detail = _detail(sqlite_session_factory, detail_data) + response = ConversationMessageDetail.model_validate(detail) + assert len(reads.sessions) > 1 + assert set(reads.active_counts) == {1} + assert reads.commits == [] + assert all(not session.in_transaction() and not session.identity_map for session in reads.sessions) + message = detail.first_message + assert message is not None + upload = message.inputs["upload"] + assert isinstance(upload, File) + assert (upload.filename, upload.mime_type, upload.size) == ("canonical.txt", "text/plain", 17) + assert resolve_file_record_id(upload.reference) == detail_data.upload_id + tools = message.inputs["tools"] + assert isinstance(tools, list) + assert isinstance(tools[0], File) + assert tools[0].filename == "result.pdf" + assert message.inputs["mixed"] == detail_data.inputs["mixed"] + assert message.inputs["empty"] == [] + assert message.inputs["zero"] == 0 + assert message.inputs["false"] is False + assert message.inputs["none"] is None + attachments = {attachment.filename: attachment for attachment in message.message_files} + assert attachments["canonical.txt"].belongs_to == "user" + assert attachments["canonical.txt"].size == 17 + assert attachments["result.pdf"].belongs_to == "assistant" + assert attachments["result.pdf"].upload_file_id == detail_data.tool_id + assert response.message is not None + assert response.message.inputs["upload"]["filename"] == "canonical.txt" + with sqlite_session_factory() as session: + assert session.get_one(MessageFile, detail_data.legacy_file_id).upload_file_id is None + assert session.get_one(Message, detail_data.message_id)._inputs == detail_data.inputs + assert session.get_one(Conversation, detail_data.conversation_id).read_at is None + + +def test_remote_head_occurs_after_conversation_and_file_lookup_transactions_close( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData +) -> None: + requests: list[tuple[str, str, bool]] = [] + with _observe_reads(sqlite_session_factory) as reads: + + class Handler(BaseHTTPRequestHandler): + @override + def log_message(self, format: str, *args: object) -> None: + pass + + def do_HEAD(self) -> None: + requests.append((self.command, self.path, any(session.in_transaction() for session in reads.sessions))) + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Length", "29") + self.send_header("Content-Disposition", "attachment; filename*=UTF-8''server%20report.txt") + self.end_headers() + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + with sqlite_session_factory.begin() as session: + session.add( + MessageFile( + message_id=detail_data.message_id, + type=FileType.DOCUMENT, + transfer_method=FileTransferMethod.REMOTE_URL, + url=f"http://127.0.0.1:{server.server_port}/download.bin", + belongs_to=MessageFileBelongsTo.USER, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=detail_data.context.account_id, + ) + ) + reads.sessions.clear() + reads.active_counts.clear() + reads.commits.clear() + detail = _detail(sqlite_session_factory, detail_data) + assert requests == [("HEAD", "/download.bin", False)] + assert reads.sessions + assert set(reads.active_counts) == {1} + assert reads.commits == [] + assert all(not session.in_transaction() and not session.identity_map for session in reads.sessions) + assert detail.first_message is not None + remote = next(file for file in detail.first_message.message_files if file.filename == "server report.txt") + assert (remote.mime_type, remote.size, remote.belongs_to) == ("text/plain", 29, "user") + finally: + server.shutdown() + server.server_close() + thread.join() + + +def test_detail_attachment_fields_match_the_legacy_model_projection( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData +) -> None: + detail = _detail(sqlite_session_factory, detail_data) + assert detail.first_message is not None + actual = { + file.id: MessageFileResponse.model_validate(file).model_dump(exclude={"url"}) + for file in detail.first_message.message_files + } + with sqlite_session_factory() as session: + message = session.get_one(Message, detail_data.message_id) + expected = { + file["id"]: MessageFileResponse.model_validate(file).model_dump(exclude={"url"}) + for file in message.message_files_with_session(session=session) + } + assert actual == expected + + +@pytest.mark.parametrize("shape", ["{}", "`{}`", "[download]({})"]) +@pytest.mark.parametrize("kind", ["tool", "file-preview", "image-preview"]) +def test_detail_signs_answer_urls_without_rewriting_stored_answer( + sqlite_session_factory: sessionmaker[Session], detail_data: DetailData, shape: str, kind: str +) -> None: + file_id = detail_data.tool_id if kind == "tool" else detail_data.upload_id + path = f"/files/tools/{file_id}.pdf" if kind == "tool" else f"/files/{file_id}/{kind}" + original = shape.format(f"http://internal.example{path}?timestamp=1&nonce=old&sign=old&as_attachment=true") + with sqlite_session_factory.begin() as session: + session.get_one(Message, detail_data.message_id).answer = original + detail = _detail(sqlite_session_factory, detail_data) + assert detail.first_message is not None + signed = detail.first_message.answer.removeprefix("[download](").removesuffix(")").strip("`") + parsed = urlsplit(signed) + query = parse_qs(parsed.query) + assert parsed.netloc == "files.example.test" + assert parsed.path == (f"/files/tools/{file_id}.pdf" if kind == "tool" else f"/files/{file_id}/file-preview") + assert query["as_attachment"] == ["true"] + assert query["nonce"] != ["old"] + assert query["timestamp"] != ["1"] + assert verify_tool_file_signature(file_id, query["timestamp"][0], query["nonce"][0], query["sign"][0]) + with sqlite_session_factory() as session: + assert session.get_one(Message, detail_data.message_id).answer == original diff --git a/api/tests/unit_tests/services/test_conversation_service.py b/api/tests/unit_tests/services/test_conversation_service.py index b8111ed95f0..21a1742b83e 100644 --- a/api/tests/unit_tests/services/test_conversation_service.py +++ b/api/tests/unit_tests/services/test_conversation_service.py @@ -11,7 +11,6 @@ from dataclasses import replace from unittest.mock import MagicMock import pytest -from sqlalchemy import event from sqlalchemy.orm import Session from libs.datetime_utils import naive_utc_now @@ -164,93 +163,6 @@ def conversation_workspace(sqlite_session: Session) -> tuple[AgentWorkspace, Age return workspace, binding -@pytest.mark.parametrize("has_root_binding", [False, True]) -def test_delete_retires_then_commits_before_enqueue( - monkeypatch: pytest.MonkeyPatch, - sqlite_session: Session, - conversation_workspace: tuple[AgentWorkspace, AgentWorkspaceBinding], - has_root_binding: bool, -) -> None: - workspace, binding = conversation_workspace - app = ConversationServiceTestDataFactory.create_app() - account = ConversationServiceTestDataFactory.create_account() - conversation = ConversationServiceTestDataFactory.create_conversation() - conversation.agent_workspace_binding_id = binding.id if has_root_binding else None - node_workspace = AgentWorkspace( - id="workspace-node", - tenant_id=TENANT_ID, - app_id=APP_ID, - owner_type=AgentWorkspaceOwnerType.CONVERSATION, - owner_id=conversation.id, - owner_scope_key="node-1:workflow-binding-1", - backend_workspace_ref="backend-workspace-node", - status=AgentWorkingResourceStatus.ACTIVE, - active_guard=1, - ) - sqlite_session.add_all([conversation, node_workspace]) - sqlite_session.commit() - events: list[str] = [] - event.listen(sqlite_session, "after_commit", lambda _session: events.append("commit")) - - def enqueue(*, tenant_id: str, workspace_ids: tuple[str, ...]) -> None: - assert tenant_id == TENANT_ID - assert set(workspace_ids) == {workspace.id, node_workspace.id} - assert not sqlite_session.in_transaction() - assert conversation.is_deleted is True - assert binding.status == workspace.status == AgentWorkingResourceStatus.RETIRED - assert binding.retired_at is not None - assert workspace.retired_at is not None - assert workspace.active_guard is None - assert node_workspace.status == AgentWorkingResourceStatus.RETIRED - assert node_workspace.retired_at is not None - assert node_workspace.active_guard is None - events.append("agent cleanup") - - monkeypatch.setattr(conversation_service, "enqueue_agent_resource_collection", enqueue) - delete_related = MagicMock(side_effect=lambda _conversation_id: events.append("conversation cleanup")) - monkeypatch.setattr(conversation_service.delete_conversation_related_data, "delay", delete_related) - - ConversationService.delete(app, conversation.id, account, session=sqlite_session) - - assert events == ["commit", "agent cleanup", "conversation cleanup"] - delete_related.assert_called_once_with(conversation.id) - - -def test_delete_commit_failure_rolls_back_all_lifecycle_changes_without_enqueue( - monkeypatch: pytest.MonkeyPatch, - sqlite_session: Session, - conversation_workspace: tuple[AgentWorkspace, AgentWorkspaceBinding], -) -> None: - workspace, binding = conversation_workspace - app = ConversationServiceTestDataFactory.create_app() - account = ConversationServiceTestDataFactory.create_account() - conversation = ConversationServiceTestDataFactory.create_conversation() - conversation.agent_workspace_binding_id = binding.id - sqlite_session.add(conversation) - sqlite_session.commit() - - def fail_commit(session: Session) -> None: - session.flush() - raise RuntimeError("commit failed") - - event.listen(sqlite_session, "before_commit", fail_commit, once=True) - enqueue_collection = MagicMock() - delete_related = MagicMock() - monkeypatch.setattr(conversation_service, "enqueue_agent_resource_collection", enqueue_collection) - monkeypatch.setattr(conversation_service.delete_conversation_related_data, "delay", delete_related) - - with pytest.raises(RuntimeError, match="commit failed"): - ConversationService.delete(app, conversation.id, account, session=sqlite_session) - - assert conversation.is_deleted is False - assert binding.status == workspace.status == AgentWorkingResourceStatus.ACTIVE - assert binding.retired_at is None - assert workspace.retired_at is None - assert workspace.active_guard == 1 - enqueue_collection.assert_not_called() - delete_related.assert_not_called() - - def test_retire_leaves_commit_and_cleanup_to_the_caller( monkeypatch: pytest.MonkeyPatch, sqlite_session: Session, @@ -325,27 +237,6 @@ def test_retire_rejects_participants_owned_by_another_scope( assert binding.status == workspace.status == AgentWorkingResourceStatus.ACTIVE -def test_delete_keeps_soft_deleted_marker_when_dispatch_fails( - monkeypatch: pytest.MonkeyPatch, sqlite_session: Session -) -> None: - app = ConversationServiceTestDataFactory.create_app() - account = ConversationServiceTestDataFactory.create_account() - conversation = ConversationServiceTestDataFactory.create_conversation() - sqlite_session.add(conversation) - sqlite_session.flush() - monkeypatch.setattr( - conversation_service.delete_conversation_related_data, - "delay", - MagicMock(side_effect=RuntimeError("broker unavailable")), - ) - - ConversationService.delete(app, conversation.id, account, session=sqlite_session) - - persisted = sqlite_session.get(Conversation, conversation.id) - assert persisted is not None - assert persisted.is_deleted is True - - def test_cleanup_propagates_agent_enqueue_failure_before_conversation_enqueue(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( conversation_service, "enqueue_agent_resource_collection", MagicMock(side_effect=RuntimeError("unavailable")) @@ -359,3 +250,24 @@ def test_cleanup_propagates_agent_enqueue_failure_before_conversation_enqueue(mo ) delete_related.assert_not_called() + + +def test_cleanup_keeps_soft_deleted_marker_when_dispatch_fails( + monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + conversation = ConversationServiceTestDataFactory.create_conversation(is_deleted=True) + sqlite_session.add(conversation) + sqlite_session.commit() + monkeypatch.setattr( + conversation_service.delete_conversation_related_data, + "delay", + MagicMock(side_effect=RuntimeError("broker unavailable")), + ) + + ConversationService.enqueue_delete_cleanup( + tenant_id=TENANT_ID, conversation_id=conversation.id, retired_workspace_ids=() + ) + + persisted = sqlite_session.get(Conversation, conversation.id) + assert persisted is not None + assert persisted.is_deleted is True diff --git a/api/tests/unit_tests/services/test_installed_app_conversation_service.py b/api/tests/unit_tests/services/test_installed_app_conversation_service.py index e6a3faadeaf..2369b84e8ff 100644 --- a/api/tests/unit_tests/services/test_installed_app_conversation_service.py +++ b/api/tests/unit_tests/services/test_installed_app_conversation_service.py @@ -27,6 +27,7 @@ from services.installed_app_conversation_service import ( ConversationNotChatAppError, InstalledAppConversationService, ) +from services.message_query_adapters import MessageFileResolver _TIME = datetime(2024, 1, 1) @@ -97,7 +98,9 @@ def _service( ) -> InstalledAppConversationService: repository = ConversationRepository(session_factory=factory) return InstalledAppConversationService( - queries=ConversationQueryService(conversations=repository, restore_inputs=restore_input_files), + queries=ConversationQueryService( + files=MessageFileResolver(), conversations=repository, restore_inputs=restore_input_files + ), names=ConversationNameService(conversations=repository, generate_name=generate_name), commands=ConversationCommandService(conversations=repository, enqueue_delete_cleanup=cleanup), ) diff --git a/packages/contracts/generated/api/console/apps/errors.gen.ts b/packages/contracts/generated/api/console/apps/errors.gen.ts index 6d18141dc35..0fecc13004f 100644 --- a/packages/contracts/generated/api/console/apps/errors.gen.ts +++ b/packages/contracts/generated/api/console/apps/errors.gen.ts @@ -202,6 +202,14 @@ const zUnsupportedAudioTypeError = /* @__PURE__ */ (() => }), ))() +const zConversationNotFoundError = /* @__PURE__ */ (() => + zErrorResponse.and( + z.object({ + code: z.literal('conversation_not_found'), + status: z.literal(404), + }), + ))() + const zAppSuggestedQuestionsAfterAnswerDisabledError = /* @__PURE__ */ (() => zErrorResponse.and( z.object({ @@ -637,9 +645,15 @@ export const post18Errors = { unsupported_audio_type: { status: 415, data: zUnsupportedAudioTypeError }, } -export const delete5Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } +export const delete5Errors = { + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, +} -export const get24Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } +export const get24Errors = { + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, +} export const get25Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } @@ -664,9 +678,15 @@ export const get27Errors = { app_unavailable: { status: 400, data: zAppUnavailableError }, } -export const delete6Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } +export const delete6Errors = { + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, +} -export const get28Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } +export const get28Errors = { + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, +} export const get29Errors = { app_not_found: { status: 404, data: zAppNotFoundError } } diff --git a/packages/contracts/generated/api/console/apps/orpc.gen.ts b/packages/contracts/generated/api/console/apps/orpc.gen.ts index 821b923b863..0e97d13e33c 100644 --- a/packages/contracts/generated/api/console/apps/orpc.gen.ts +++ b/packages/contracts/generated/api/console/apps/orpc.gen.ts @@ -19,6 +19,7 @@ import { zBadRequestError, zBindingFileDownloadUnavailableError, zCompletionRequestError, + zConversationNotFoundError, zDraftWorkflowNotExistError, zDraftWorkflowNotSyncError, zForbiddenError, @@ -1735,7 +1736,10 @@ export const delete5 = oc }) .input(z.object({ params: zDeleteAppsByAppIdChatConversationsByConversationIdPath })) .output(zDeleteAppsByAppIdChatConversationsByConversationIdResponse) - .errors({ app_not_found: { status: 404, data: zAppNotFoundError } }) + .errors({ + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, + }) /** * Get chat conversation details @@ -1751,7 +1755,10 @@ export const get24 = oc }) .input(z.object({ params: zGetAppsByAppIdChatConversationsByConversationIdPath })) .output(zGetAppsByAppIdChatConversationsByConversationIdResponse) - .errors({ app_not_found: { status: 404, data: zAppNotFoundError } }) + .errors({ + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, + }) export const byConversationId = { delete: delete5, @@ -1759,11 +1766,11 @@ export const byConversationId = { } /** - * Get chat conversations with pagination, filtering and summary + * Get chat conversations with pagination and filtering */ export const get25 = oc .route({ - description: 'Get chat conversations with pagination, filtering and summary', + description: 'Get chat conversations with pagination and filtering', inputStructure: 'detailed', method: 'GET', operationId: 'getAppsByAppIdChatConversations', @@ -1886,7 +1893,10 @@ export const delete6 = oc }) .input(z.object({ params: zDeleteAppsByAppIdCompletionConversationsByConversationIdPath })) .output(zDeleteAppsByAppIdCompletionConversationsByConversationIdResponse) - .errors({ app_not_found: { status: 404, data: zAppNotFoundError } }) + .errors({ + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, + }) /** * Get completion conversation details with messages @@ -1902,7 +1912,10 @@ export const get28 = oc }) .input(z.object({ params: zGetAppsByAppIdCompletionConversationsByConversationIdPath })) .output(zGetAppsByAppIdCompletionConversationsByConversationIdResponse) - .errors({ app_not_found: { status: 404, data: zAppNotFoundError } }) + .errors({ + app_not_found: { status: 404, data: zAppNotFoundError }, + conversation_not_found: { status: 404, data: zConversationNotFoundError }, + }) export const byConversationId2 = { delete: delete6, diff --git a/packages/contracts/generated/api/console/apps/types.gen.ts b/packages/contracts/generated/api/console/apps/types.gen.ts index e82dc598866..b92527ac984 100644 --- a/packages/contracts/generated/api/console/apps/types.gen.ts +++ b/packages/contracts/generated/api/console/apps/types.gen.ts @@ -527,6 +527,11 @@ export type ConversationWithSummaryPagination = { total: number } +export type ConversationNotFoundError = ErrorResponse & { + code: 'conversation_not_found' + status: 404 +} + export type ConversationDetail = { admin_feedback_stats?: FeedbackStat | null annotated: boolean @@ -6112,8 +6117,11 @@ export type GetAppsByAppIdChatConversationsData = { } export type GetAppsByAppIdChatConversationsErrors = { + 400: ErrorResponse + 401: ErrorResponse 403: ErrorResponse 404: AppNotFoundError | ErrorResponse + 422: ErrorResponse default: ErrorResponse } @@ -6138,8 +6146,9 @@ export type DeleteAppsByAppIdChatConversationsByConversationIdData = { } export type DeleteAppsByAppIdChatConversationsByConversationIdErrors = { + 401: ErrorResponse 403: ErrorResponse - 404: AppNotFoundError | ErrorResponse + 404: AppNotFoundError | ConversationNotFoundError | ErrorResponse default: ErrorResponse } @@ -6164,8 +6173,9 @@ export type GetAppsByAppIdChatConversationsByConversationIdData = { } export type GetAppsByAppIdChatConversationsByConversationIdErrors = { + 401: ErrorResponse 403: ErrorResponse - 404: AppNotFoundError | ErrorResponse + 404: AppNotFoundError | ConversationNotFoundError | ErrorResponse default: ErrorResponse } @@ -6285,8 +6295,11 @@ export type GetAppsByAppIdCompletionConversationsData = { } export type GetAppsByAppIdCompletionConversationsErrors = { + 400: ErrorResponse + 401: ErrorResponse 403: ErrorResponse 404: AppNotFoundError | ErrorResponse + 422: ErrorResponse default: ErrorResponse } @@ -6311,8 +6324,9 @@ export type DeleteAppsByAppIdCompletionConversationsByConversationIdData = { } export type DeleteAppsByAppIdCompletionConversationsByConversationIdErrors = { + 401: ErrorResponse 403: ErrorResponse - 404: AppNotFoundError | ErrorResponse + 404: AppNotFoundError | ConversationNotFoundError | ErrorResponse default: ErrorResponse } @@ -6337,8 +6351,9 @@ export type GetAppsByAppIdCompletionConversationsByConversationIdData = { } export type GetAppsByAppIdCompletionConversationsByConversationIdErrors = { + 401: ErrorResponse 403: ErrorResponse - 404: AppNotFoundError | ErrorResponse + 404: AppNotFoundError | ConversationNotFoundError | ErrorResponse default: ErrorResponse } diff --git a/packages/contracts/generated/api/console/apps/zod.gen.ts b/packages/contracts/generated/api/console/apps/zod.gen.ts index cf1895a13c9..718d41b78a8 100644 --- a/packages/contracts/generated/api/console/apps/zod.gen.ts +++ b/packages/contracts/generated/api/console/apps/zod.gen.ts @@ -554,6 +554,18 @@ export const zUnsupportedAudioTypeError = zErrorResponse.and( }), ) +/** + * ConversationNotFoundError + * + * The conversation was not found for this user and app. + */ +export const zConversationNotFoundError = zErrorResponse.and( + z.object({ + code: z.literal('conversation_not_found'), + status: z.literal(404), + }), +) + /** * SuggestedQuestionsResponse */ diff --git a/packages/contracts/generated/api/console/installed-apps/zod.gen.ts b/packages/contracts/generated/api/console/installed-apps/zod.gen.ts index 476148aa096..62b8992aa14 100644 --- a/packages/contracts/generated/api/console/installed-apps/zod.gen.ts +++ b/packages/contracts/generated/api/console/installed-apps/zod.gen.ts @@ -313,7 +313,7 @@ export const zConversationCursorNotFoundError = zErrorResponse.and( /** * ConversationNotFoundError * - * The conversation was not found for this account and app. + * The conversation was not found for this user and app. */ export const zConversationNotFoundError = zErrorResponse.and( z.object({