From e85c042eb643fc59cbb254f659a2a981c80bb52c Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Wed, 15 Apr 2026 20:13:39 +0700 Subject: [PATCH] fix(backend/copilot): add route tests, mutual-exclusive cursor validation, newestSequence guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - routes_test.py: 4 new tests for get_session — forward_paginated=True for completed sessions, False for active, after_sequence wiring, 400 for conflicting cursors - routes.py: reject 400 when both before_sequence and after_sequence are sent - useLoadMoreMessages.ts: guard newestSequence in else-branch so a parent refetch never reverts a cursor already advanced by paging --- .../backend/api/features/chat/routes.py | 6 + .../backend/api/features/chat/routes_test.py | 107 ++++++++++++++++++ .../(platform)/copilot/useLoadMoreMessages.ts | 8 +- 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/autogpt_platform/backend/backend/api/features/chat/routes.py b/autogpt_platform/backend/backend/api/features/chat/routes.py index 6f442a7591..beb85df717 100644 --- a/autogpt_platform/backend/backend/api/features/chat/routes.py +++ b/autogpt_platform/backend/backend/api/features/chat/routes.py @@ -479,6 +479,12 @@ async def get_session( SessionDetailResponse: Details for the requested session, including active_stream info and pagination metadata. """ + if before_sequence is not None and after_sequence is not None: + raise HTTPException( + status_code=400, + detail="before_sequence and after_sequence are mutually exclusive", + ) + is_initial_load = before_sequence is None and after_sequence is None # Check active stream before the DB query on initial loads so we can diff --git a/autogpt_platform/backend/backend/api/features/chat/routes_test.py b/autogpt_platform/backend/backend/api/features/chat/routes_test.py index 74259b3463..4504edbb74 100644 --- a/autogpt_platform/backend/backend/api/features/chat/routes_test.py +++ b/autogpt_platform/backend/backend/api/features/chat/routes_test.py @@ -722,3 +722,110 @@ def test_disconnect_stream_returns_404_when_session_missing( assert response.status_code == 404 mock_disconnect.assert_not_awaited() + + +# ─── GET /sessions/{session_id} — forward/backward pagination ────────────────── + + +def _make_paginated_messages( + mocker: pytest_mock.MockerFixture, *, has_more: bool = False +): + """Return a mock PaginatedMessages and configure the DB patch.""" + from datetime import UTC, datetime + + from backend.copilot.db import PaginatedMessages + from backend.copilot.model import ChatMessage, ChatSessionInfo, ChatSessionMetadata + + now = datetime.now(UTC) + session_info = ChatSessionInfo( + session_id="sess-1", + user_id=TEST_USER_ID, + usage=[], + started_at=now, + updated_at=now, + metadata=ChatSessionMetadata(), + ) + page = PaginatedMessages( + messages=[ChatMessage(role="user", content="hello", sequence=0)], + has_more=has_more, + oldest_sequence=0, + newest_sequence=0, + session=session_info, + ) + mock_paginate = mocker.patch( + "backend.api.features.chat.routes.get_chat_messages_paginated", + new_callable=AsyncMock, + return_value=page, + ) + return page, mock_paginate + + +def test_get_session_completed_returns_forward_paginated( + mocker: pytest_mock.MockerFixture, + test_user_id: str, +) -> None: + """Completed sessions (no active stream) return forward_paginated=True.""" + _make_paginated_messages(mocker) + mocker.patch( + "backend.api.features.chat.routes.stream_registry.get_active_session", + new_callable=AsyncMock, + return_value=(None, None), + ) + + response = client.get("/sessions/sess-1") + + assert response.status_code == 200 + data = response.json() + assert data["forward_paginated"] is True + assert data["newest_sequence"] == 0 + + +def test_get_session_active_returns_backward_paginated( + mocker: pytest_mock.MockerFixture, + test_user_id: str, +) -> None: + """Active sessions (with running stream) return forward_paginated=False.""" + from backend.copilot.stream_registry import ActiveSession + + _make_paginated_messages(mocker) + active = MagicMock(spec=ActiveSession) + active.turn_id = "turn-1" + mocker.patch( + "backend.api.features.chat.routes.stream_registry.get_active_session", + new_callable=AsyncMock, + return_value=(active, "msg-1"), + ) + + response = client.get("/sessions/sess-1") + + assert response.status_code == 200 + data = response.json() + assert data["forward_paginated"] is False + assert data["active_stream"] is not None + assert data["active_stream"]["turn_id"] == "turn-1" + + +def test_get_session_after_sequence_returns_forward_paginated( + mocker: pytest_mock.MockerFixture, + test_user_id: str, +) -> None: + """after_sequence param returns forward_paginated=True; no stream check needed.""" + _, mock_paginate = _make_paginated_messages(mocker) + + response = client.get("/sessions/sess-1?after_sequence=10") + + assert response.status_code == 200 + data = response.json() + assert data["forward_paginated"] is True + call_kwargs = mock_paginate.call_args + assert call_kwargs.kwargs.get("after_sequence") == 10 + assert call_kwargs.kwargs.get("before_sequence") is None + + +def test_get_session_both_cursors_returns_400( + test_user_id: str, +) -> None: + """Sending both before_sequence and after_sequence returns 400.""" + response = client.get("/sessions/sess-1?before_sequence=5&after_sequence=10") + + assert response.status_code == 400 diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts b/autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts index 2b001cea3d..1082da2b93 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts @@ -83,7 +83,13 @@ export function useLoadMoreMessages({ // Update from parent when initial data changes (e.g. refetch) prevInitialOldestRef.current = initialOldestSequence; setOldestSequence(initialOldestSequence); - setNewestSequence(initialNewestSequence); + // Only regress the forward cursor if we haven't paged ahead yet — + // otherwise a parent refetch would reset a cursor we already advanced. + setNewestSequence((prev) => + prev !== null && prev > (initialNewestSequence ?? -1) + ? prev + : initialNewestSequence, + ); setHasMore(initialHasMore); } }, [sessionId, initialOldestSequence, initialNewestSequence, initialHasMore]);