diff --git a/application/api/answer/routes/answer.py b/application/api/answer/routes/answer.py index 1e869c47..3fab4de2 100644 --- a/application/api/answer/routes/answer.py +++ b/application/api/answer/routes/answer.py @@ -52,8 +52,18 @@ class AnswerResource(Resource, BaseAnswerResource): ), "save_conversation": fields.Boolean( required=False, - default=True, - description="Whether to save the conversation", + description=( + "Deprecated, no effect: conversations always persist. " + "Use `visibility` to control sidebar listing." + ), + ), + "visibility": fields.String( + required=False, + default="hidden", + description=( + "'listed' shows the conversation in the owner's sidebar; " + "any other value (or omitting it) persists it hidden." + ), ), "model_id": fields.String( required=False, @@ -119,9 +129,7 @@ class AnswerResource(Resource, BaseAnswerResource): return error should_persist, visibility = resolve_persistence( - display_flag=data.get("save_conversation"), - api_key=data.get("api_key"), - is_shared_usage=processor.is_shared_usage, + visibility_flag=data.get("visibility"), persist_flag=data.get("persist"), ) stream = self.complete_stream( diff --git a/application/api/answer/routes/base.py b/application/api/answer/routes/base.py index 5afa1ead..c5654afa 100644 --- a/application/api/answer/routes/base.py +++ b/application/api/answer/routes/base.py @@ -184,7 +184,7 @@ class BaseAnswerResource: isNoneDoc: bool = False, index: Optional[int] = None, should_persist: bool = True, - visibility: str = "listed", + visibility: str = "hidden", attachment_ids: Optional[List[str]] = None, agent_id: Optional[str] = None, is_shared_usage: bool = False, @@ -207,7 +207,9 @@ class BaseAnswerResource: isNoneDoc: Flag for document-less responses index: Index of message to update should_persist: Whether to persist the conversation - visibility: ``listed`` (sidebar) or ``hidden`` for a new conversation + visibility: ``listed`` (sidebar) or ``hidden`` for a new + conversation; defaults to ``hidden`` so only callers that + explicitly opt in (the first-party UI) list rows attachment_ids: List of attachment IDs agent_id: ID of agent used is_shared_usage: Flag for shared agent usage diff --git a/application/api/answer/routes/stream.py b/application/api/answer/routes/stream.py index 18abb158..080687bf 100644 --- a/application/api/answer/routes/stream.py +++ b/application/api/answer/routes/stream.py @@ -55,8 +55,18 @@ class StreamResource(Resource, BaseAnswerResource): ), "save_conversation": fields.Boolean( required=False, - default=True, - description="Whether to save the conversation", + description=( + "Deprecated, no effect: conversations always persist. " + "Use `visibility` to control sidebar listing." + ), + ), + "visibility": fields.String( + required=False, + default="hidden", + description=( + "'listed' shows the conversation in the owner's sidebar; " + "any other value (or omitting it) persists it hidden." + ), ), "model_id": fields.String( required=False, @@ -137,9 +147,7 @@ class StreamResource(Resource, BaseAnswerResource): if error := self.check_usage(processor.agent_config): return error should_persist, visibility = resolve_persistence( - display_flag=data.get("save_conversation"), - api_key=data.get("api_key"), - is_shared_usage=processor.is_shared_usage, + visibility_flag=data.get("visibility"), persist_flag=data.get("persist"), ) return Response( diff --git a/application/api/answer/services/conversation_service.py b/application/api/answer/services/conversation_service.py index cedb94cc..897400c6 100644 --- a/application/api/answer/services/conversation_service.py +++ b/application/api/answer/services/conversation_service.py @@ -78,7 +78,7 @@ class ConversationService: shared_token: Optional[str] = None, attachment_ids: Optional[List[str]] = None, metadata: Optional[Dict[str, Any]] = None, - visibility: str = "listed", + visibility: str = "hidden", ) -> str: """Save or update a conversation in Postgres. @@ -204,7 +204,7 @@ class ConversationService: shared_token: Optional[str] = None, model_id: Optional[str] = None, request_id: Optional[str] = None, - visibility: str = "listed", + visibility: str = "hidden", status: str = "pending", index: Optional[int] = None, ) -> Dict[str, str]: diff --git a/application/api/answer/services/persistence_policy.py b/application/api/answer/services/persistence_policy.py index c943a2ac..b08bec6f 100644 --- a/application/api/answer/services/persistence_policy.py +++ b/application/api/answer/services/persistence_policy.py @@ -1,13 +1,16 @@ """Resolve whether an answer is persisted and whether it lists in the sidebar. Persistence (is a row written at all?) and visibility (does it show in the -owner's sidebar?) used to be one decision driven by the request-level -``save_conversation`` flag plus an identity heuristic. They are now separate: -conversations persist by default everywhere, while ``save_conversation`` only -controls sidebar visibility. +owner's sidebar?) are separate decisions. Conversations persist by default +everywhere, and visibility defaults to ``hidden`` for every caller: only an +explicit request-level ``visibility: "listed"`` — which the first-party UI +sends on normal chats — puts a conversation in the owner's sidebar. The +legacy ``save_conversation`` flag no longer affects either decision, so +API/OpenAI-compatible clients that still send it (its old meaning was +"persist this conversation") can't list rows into the agent owner's sidebar. """ -from typing import Optional, Tuple +from typing import Any, Optional, Tuple VISIBILITY_LISTED = "listed" VISIBILITY_HIDDEN = "hidden" @@ -15,20 +18,15 @@ VISIBILITY_HIDDEN = "hidden" def resolve_persistence( *, - display_flag: Optional[bool], - api_key: Optional[str] = None, - is_shared_usage: bool = False, + visibility_flag: Optional[Any] = None, persist_flag: Optional[bool] = None, ) -> Tuple[bool, str]: """Resolve ``(should_persist, visibility)`` for an answer request. Args: - display_flag: Request-level ``save_conversation`` (``None`` if the - caller omitted it). Forces visibility on/off when provided. - api_key: Present for programmatic/agent callers (including the - OpenAI-compatible ``/v1`` endpoint), absent for first-party - session users. - is_shared_usage: True for shared-agent/widget traffic. + visibility_flag: Request-level ``visibility`` value. Only the exact + string ``"listed"`` opts the conversation into the owner's + sidebar; anything else (including ``None``) stays hidden. persist_flag: Explicit persistence opt-out (``False`` to skip writing a row, e.g. stateless tool rounds that would orphan one). ``None`` keeps the always-persist default. @@ -38,8 +36,9 @@ def resolve_persistence( ``"listed"`` or ``"hidden"``. """ should_persist = True if persist_flag is None else bool(persist_flag) - # Only genuine first-party interactive turns default to the sidebar. - default_display = api_key is None and not is_shared_usage - display = default_display if display_flag is None else bool(display_flag) - visibility = VISIBILITY_LISTED if display else VISIBILITY_HIDDEN + visibility = ( + VISIBILITY_LISTED + if visibility_flag == VISIBILITY_LISTED + else VISIBILITY_HIDDEN + ) return should_persist, visibility diff --git a/application/api/v1/routes.py b/application/api/v1/routes.py index e40afddf..ca293439 100644 --- a/application/api/v1/routes.py +++ b/application/api/v1/routes.py @@ -188,11 +188,9 @@ def chat_completions(): return usage_error # v1 always persists (unless the translator opted out for a stateless - # tool round); the agent owner's sidebar only lists it on explicit - # ``docsgpt.save_conversation: true``. + # tool round) and never lists in the agent owner's sidebar — only the + # first-party UI opts a conversation into ``visibility: "listed"``. should_persist, visibility = resolve_persistence( - display_flag=internal_data.get("save_conversation"), - api_key=internal_data.get("api_key"), persist_flag=internal_data.get("persist"), ) # Only strip leaked reasoning from content for structured requests -- the diff --git a/application/api/v1/translator.py b/application/api/v1/translator.py index 349e1883..7ffc9325 100644 --- a/application/api/v1/translator.py +++ b/application/api/v1/translator.py @@ -267,12 +267,10 @@ def translate_request( # persist the final turn; stateless ones (no conversation_id, e.g. # opencode) skip it, else every tool round writes an orphan conversation # with an empty question. ``docsgpt.persist`` overrides. Visibility is - # separate — hidden unless ``docsgpt.save_conversation`` opts in. + # not request-controllable on v1 — rows always persist hidden, so the + # legacy ``docsgpt.save_conversation`` flag is ignored. docsgpt_ext = data.get("docsgpt", {}) result["persist"] = bool(docsgpt_ext.get("persist", bool(conversation_id))) - result["save_conversation"] = bool( - docsgpt_ext.get("save_conversation", False) - ) # Carry tools forward for next iteration if data.get("tools"): result["client_tools"] = data["tools"] @@ -305,9 +303,9 @@ def translate_request( "question": question, "api_key": api_key, "history": json.dumps(history), - # v1 conversations always persist; ``save_conversation`` only controls - # whether they list in the agent owner's sidebar (default hidden). - "save_conversation": bool(docsgpt.get("save_conversation", False)), + # v1 conversations always persist and stay hidden from the agent + # owner's sidebar; the legacy ``docsgpt.save_conversation`` flag + # (old meaning: "persist this conversation") is ignored. } if system_prompt_override is not None: @@ -586,7 +584,7 @@ def translate_stream_event( elif event_type == "id": # Skip the "None" placeholder conversation_id emitted when the call is - # not persisted (save_conversation=false) — nothing useful to surface. + # not persisted (persist=false tool rounds) — nothing useful to surface. conv_id = event_data.get("id", "") if conv_id and conv_id != "None": chunks.append( diff --git a/docs/content/Agents/api.mdx b/docs/content/Agents/api.mdx index ad77a69a..b843f986 100644 --- a/docs/content/Agents/api.mdx +++ b/docs/content/Agents/api.mdx @@ -59,7 +59,8 @@ Common request body fields: | `conversation_id` | `string` | No | `/api/answer`, `/stream` | Continue an existing conversation. | | `history` | `string` (JSON-encoded array) | No | `/api/answer`, `/stream` | Used for new conversations. Format: `[{\"prompt\":\"...\",\"response\":\"...\"}]`. | | `model_id` | `string` | No | `/api/answer`, `/stream` | Override model for this request. | -| `save_conversation` | `boolean` | No | `/api/answer`, `/stream` | Default `true`. If `false`, no conversation is persisted. | +| `save_conversation` | `boolean` | No | `/api/answer`, `/stream` | Deprecated, no effect. Conversations are always persisted; use `visibility` to control sidebar listing. | +| `visibility` | `string` | No | `/api/answer`, `/stream` | `"listed"` shows the conversation in the owner's sidebar. Any other value (or omitting it) persists it hidden. Default `hidden`. | | `passthrough` | `object` | No | `/api/answer`, `/stream` | Dynamic values injected into prompt templates. | | `prompt_id` | `string` | No | `/api/answer`, `/stream` | Ignored when `api_key` already defines prompt. | | `active_docs` | `string` or `string[]` | No | `/api/answer`, `/stream` | Overrides active docs when not using key-owned source config. | diff --git a/docs/content/Agents/openai-compatible.mdx b/docs/content/Agents/openai-compatible.mdx index 0994ec59..8cfb18b4 100644 --- a/docs/content/Agents/openai-compatible.mdx +++ b/docs/content/Agents/openai-compatible.mdx @@ -80,14 +80,14 @@ When an override is active, the agent's prompt template is replaced wholesale ## Conversation Persistence -Conversations are **not persisted by default** (stateless, like most OpenAI clients expect). Opt in per request: +Conversations are **always persisted** server-side, and the response includes +`docsgpt.conversation_id`. They never appear in the agent owner's sidebar — +`/v1` traffic is stored hidden, so external clients can't clutter the owner's +conversation list. -```json -{ "docsgpt": { "save_conversation": true } } -``` - -The response will include `docsgpt.conversation_id`. +The legacy `{ "docsgpt": { "save_conversation": true } }` opt-in from older +releases (when `/v1` was stateless by default) is deprecated and ignored. ## When to Use Native Endpoints Instead -Use [`/api/answer` or `/stream`](/Agents/api) if you need server-side attachments, `passthrough` template variables, explicit `conversation_id` reuse, or persistence by default. +Use [`/api/answer` or `/stream`](/Agents/api) if you need server-side attachments, `passthrough` template variables, explicit `conversation_id` reuse, or sidebar visibility control via `visibility`. diff --git a/frontend/src/agents/workflow/workflowPreviewSlice.ts b/frontend/src/agents/workflow/workflowPreviewSlice.ts index 86f48593..6b233aaf 100644 --- a/frontend/src/agents/workflow/workflowPreviewSlice.ts +++ b/frontend/src/agents/workflow/workflowPreviewSlice.ts @@ -80,6 +80,7 @@ export const fetchWorkflowPreviewAnswer = createAsyncThunk< question, workflow: workflowData, save_conversation: false, + visibility: 'hidden', }; await new Promise((resolve, reject) => { diff --git a/frontend/src/conversation/conversationHandlers.ts b/frontend/src/conversation/conversationHandlers.ts index 0ca0110c..4578c8d2 100644 --- a/frontend/src/conversation/conversationHandlers.ts +++ b/frontend/src/conversation/conversationHandlers.ts @@ -192,6 +192,10 @@ export function handleFetchAnswer( isNoneDoc: selectedDocs.length === 0, agent_id: agentId, save_conversation: save_conversation, + // The backend persists every conversation hidden unless the request + // explicitly opts into the sidebar; save_conversation alone no longer + // lists it. + visibility: save_conversation ? 'listed' : 'hidden', }; if (modelId) { @@ -265,6 +269,10 @@ export function handleFetchAnswerSteaming( index: indx, agent_id: agentId, save_conversation: save_conversation, + // The backend persists every conversation hidden unless the request + // explicitly opts into the sidebar; save_conversation alone no longer + // lists it. + visibility: save_conversation ? 'listed' : 'hidden', }; if (modelId) { @@ -909,6 +917,7 @@ export function handleFetchSharedAnswerStreaming( history: JSON.stringify(history), api_key: apiKey, save_conversation: false, + visibility: 'hidden', attachments: attachments.length > 0 ? attachments : undefined, }; conversationService @@ -984,6 +993,7 @@ export function handleFetchSharedAnswer( const payload = { question: question, api_key: apiKey, + visibility: 'hidden', attachments: attachments && attachments.length > 0 ? attachments : undefined, }; diff --git a/frontend/src/conversation/conversationModels.ts b/frontend/src/conversation/conversationModels.ts index a84a3b74..26b6bb9c 100644 --- a/frontend/src/conversation/conversationModels.ts +++ b/frontend/src/conversation/conversationModels.ts @@ -88,5 +88,6 @@ export interface RetrievalPayload { agent_id?: string; attachments?: string[]; save_conversation?: boolean; + visibility?: 'listed' | 'hidden'; model_id?: string; } diff --git a/tests/api/answer/services/test_persistence_policy.py b/tests/api/answer/services/test_persistence_policy.py index d3bde387..ef395074 100644 --- a/tests/api/answer/services/test_persistence_policy.py +++ b/tests/api/answer/services/test_persistence_policy.py @@ -11,34 +11,34 @@ from application.api.answer.services.persistence_policy import ( @pytest.mark.unit class TestResolvePersistence: - def test_first_party_default_lists(self): - persist, visibility = resolve_persistence(display_flag=None) - assert persist is True - assert visibility == VISIBILITY_LISTED - - def test_api_key_caller_defaults_hidden(self): - persist, visibility = resolve_persistence(display_flag=None, api_key="k-1") + def test_default_is_hidden(self): + persist, visibility = resolve_persistence() assert persist is True assert visibility == VISIBILITY_HIDDEN - def test_shared_usage_defaults_hidden(self): - _, visibility = resolve_persistence(display_flag=None, is_shared_usage=True) - assert visibility == VISIBILITY_HIDDEN - - def test_explicit_display_true_lists_api_key_caller(self): - _, visibility = resolve_persistence(display_flag=True, api_key="k-1") + def test_explicit_listed_lists(self): + persist, visibility = resolve_persistence(visibility_flag=VISIBILITY_LISTED) + assert persist is True assert visibility == VISIBILITY_LISTED - def test_explicit_display_false_hides_first_party(self): - persist, visibility = resolve_persistence(display_flag=False) - # Still persisted, just not shown in the sidebar. - assert persist is True + def test_explicit_hidden_stays_hidden(self): + _, visibility = resolve_persistence(visibility_flag=VISIBILITY_HIDDEN) + assert visibility == VISIBILITY_HIDDEN + + def test_unknown_visibility_value_falls_back_to_hidden(self): + _, visibility = resolve_persistence(visibility_flag="sidebar-please") + assert visibility == VISIBILITY_HIDDEN + + def test_non_string_visibility_value_falls_back_to_hidden(self): + # Clients sending booleans (e.g. a stray ``visibility: true``) must + # not accidentally list — only the exact string "listed" opts in. + _, visibility = resolve_persistence(visibility_flag=True) assert visibility == VISIBILITY_HIDDEN def test_persist_opt_out(self): - persist, _ = resolve_persistence(display_flag=None, persist_flag=False) + persist, _ = resolve_persistence(persist_flag=False) assert persist is False - def test_persist_defaults_true_for_hidden_api_caller(self): - persist, _ = resolve_persistence(display_flag=False, api_key="k-1") + def test_persist_defaults_true_when_listed(self): + persist, _ = resolve_persistence(visibility_flag=VISIBILITY_LISTED) assert persist is True diff --git a/tests/e2e/specs/tier-a/chat-turn.spec.ts b/tests/e2e/specs/tier-a/chat-turn.spec.ts index f2871304..c40ba893 100644 --- a/tests/e2e/specs/tier-a/chat-turn.spec.ts +++ b/tests/e2e/specs/tier-a/chat-turn.spec.ts @@ -266,8 +266,9 @@ test.describe('tier-a · chat turn save', () => { isNoneDoc: true, }); expect(result.status).toBe(200); - // save_conversation now controls sidebar visibility, not persistence: - // the turn persists, so a real conversation UUID is emitted. + // Conversations always persist and default to hidden; + // save_conversation is deprecated and has no effect. The turn + // persists, so a real conversation UUID is emitted. expect(result.conversationId).toMatch( /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, ); diff --git a/tests/e2e/specs/tier-a/conversation-crud.spec.ts b/tests/e2e/specs/tier-a/conversation-crud.spec.ts index 67800b97..200da2e7 100644 --- a/tests/e2e/specs/tier-a/conversation-crud.spec.ts +++ b/tests/e2e/specs/tier-a/conversation-crud.spec.ts @@ -88,6 +88,7 @@ test.describe('tier-a · conversations CRUD', () => { const id = await streamOnce(api, { question: `${STREAM_QUESTION} (${i})`, save_conversation: true, + visibility: 'listed', }); expect(id).toBeTruthy(); ids.push(id); @@ -130,6 +131,7 @@ test.describe('tier-a · conversations CRUD', () => { const convId = await streamOnce(api, { question: STREAM_QUESTION, save_conversation: true, + visibility: 'listed', }); page = await context.newPage(); @@ -198,10 +200,12 @@ test.describe('tier-a · conversations CRUD', () => { const keepId = await streamOnce(api, { question: STREAM_QUESTION + ' keep', save_conversation: true, + visibility: 'listed', }); const dropId = await streamOnce(api, { question: STREAM_QUESTION + ' drop', save_conversation: true, + visibility: 'listed', }); expect(keepId).not.toBe(dropId); expect( diff --git a/tests/e2e/specs/tier-b/agent-preview.spec.ts b/tests/e2e/specs/tier-b/agent-preview.spec.ts index dd1a536d..2a84cb28 100644 --- a/tests/e2e/specs/tier-b/agent-preview.spec.ts +++ b/tests/e2e/specs/tier-b/agent-preview.spec.ts @@ -2,11 +2,11 @@ * Tier-B · agent preview persists a HIDDEN conversation * * Covers B14. The preview flow in `frontend/src/agents/agentPreviewSlice.ts` - * calls `handleFetchAnswerSteaming(..., save_conversation=false)`. As of the - * conversation-visibility change, `save_conversation` no longer gates - * persistence — it controls sidebar visibility only. A preview turn therefore - * persists a `conversations` row with `visibility = 'hidden'` (decided by - * `resolve_persistence` in + * calls `handleFetchAnswerSteaming(..., save_conversation=false)`, which the + * handler maps to `visibility: 'hidden'`. Conversations always persist; + * visibility defaults to `hidden` for every request and only an explicit + * `visibility: 'listed'` (sent by the UI on normal chats) lists a row in the + * sidebar (decided by `resolve_persistence` in * application/api/answer/services/persistence_policy.py and threaded through * `BaseAnswerResource.complete_stream(visibility=...)`). * @@ -17,10 +17,11 @@ * traffic would pollute the user's sidebar history. * * API-driven: the frontend preview UI ultimately POSTs /stream with - * `save_conversation:false` and `conversation_id:null`; we exercise that + * `visibility:'hidden'` and `conversation_id:null`; we exercise that * contract directly to keep the spec fast and isolated from the AgentPreview - * modal's rendering quirks. A standard chat POST (save_conversation omitted) - * is the control — it writes a `listed` row that DOES surface in the sidebar. + * modal's rendering quirks. A standard chat POST (visibility:'listed', as the + * UI sends) is the control — it writes a `listed` row that DOES surface in + * the sidebar. */ import * as playwright from '@playwright/test'; @@ -116,12 +117,13 @@ test.describe('tier-b · agent preview', () => { expect(await countUserConversations(sub)).toBe(0); // Fire the preview payload that agentPreviewSlice.ts sends. Key - // fields: save_conversation=false, conversation_id=null, agent_id. + // fields: visibility='hidden', conversation_id=null, agent_id. await drainStream(api, { question: 'Hello preview', conversation_id: null, agent_id: agentId, save_conversation: false, + visibility: 'hidden', isNoneDoc: true, chunks: '0', }); @@ -146,7 +148,7 @@ test.describe('tier-b · agent preview', () => { } }); - test('control: a non-preview stream (save_conversation omitted) writes a LISTED conversations row', async ({ + test('control: a non-preview stream (visibility: listed, as the UI sends) writes a LISTED conversations row', async ({ browser, }) => { const { context, sub, token } = await newUserContext(browser); @@ -155,14 +157,15 @@ test.describe('tier-b · agent preview', () => { const agentId = await createDraftAgent(sub, 'non-preview-target'); expect(await countUserConversations(sub)).toBe(0); - // Standard chat path — save_conversation omitted, so a first-party - // interactive turn defaults to `listed` (resolve_persistence). This is - // the "control" that makes the preview assertion meaningful: without + // Standard chat path — the UI sends `visibility: 'listed'` on normal + // chats (visibility defaults to hidden server-side). This is the + // "control" that makes the preview assertion meaningful: without // it, a spec where nothing ever surfaces could pass trivially. const conversationId = await streamOnce(api, { question: 'Hello persistence', conversation_id: null, agent_id: agentId, + visibility: 'listed', isNoneDoc: true, chunks: '0', }); @@ -195,6 +198,7 @@ test.describe('tier-b · agent preview', () => { conversation_id: null, agent_id: agentId, save_conversation: false, + visibility: 'hidden', isNoneDoc: true, chunks: '0', }); diff --git a/tests/e2e/specs/tier-b/conversation-visibility.spec.ts b/tests/e2e/specs/tier-b/conversation-visibility.spec.ts new file mode 100644 index 00000000..24d64a27 --- /dev/null +++ b/tests/e2e/specs/tier-b/conversation-visibility.spec.ts @@ -0,0 +1,223 @@ +/** + * Tier-B · conversation visibility (persist vs. sidebar display) + * + * Exercises the conversation-visibility change end-to-end against the live + * stack. `save_conversation` no longer gates persistence — it controls sidebar + * visibility only (resolve_persistence in + * application/api/answer/services/persistence_policy.py). A conversation now + * always persists; whether it surfaces in GET /api/get_conversations is + * governed by the new `conversations.visibility` column ('listed' | 'hidden'). + * + * Matrix covered: + * 1. first-party /stream, save_conversation omitted → listed, in sidebar + * 2. first-party /stream, save_conversation=false → hidden, not in sidebar (but persisted) + * 3. api-key agent /stream → hidden by default; listed on opt-in + * 4. /v1/chat/completions (OpenAI-compat) → hidden by default; listed on docsgpt.save_conversation + * + * API-driven: assertions read the DB (`conversations.visibility`) and the real + * sidebar endpoint, not stream internals. + */ + +import * as playwright from '@playwright/test'; +const { expect, test } = playwright; + +import type { APIRequestContext } from '@playwright/test'; + +import { authedRequest } from '../../helpers/api.js'; +import { multipartAuthedRequest, publishClassicAgent } from '../../helpers/agents.js'; +import { newUserContext } from '../../helpers/auth.js'; +import { countRows, pg } from '../../helpers/db.js'; +import { resetDb } from '../../helpers/reset.js'; +import { streamOnce } from '../../helpers/streaming.js'; + +const API_URL = process.env.API_URL ?? 'http://127.0.0.1:7099'; +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** Read the `visibility` column for a conversation id. */ +async function visibilityOf(convId: string): Promise { + const { rows } = await pg.query<{ visibility: string }>( + 'SELECT visibility FROM conversations WHERE id = CAST($1 AS uuid)', + [convId], + ); + return rows[0]?.visibility ?? null; +} + +/** The actual sidebar: GET /api/get_conversations returns only listed rows. */ +async function sidebarIds(api: APIRequestContext): Promise { + const res = await api.get('/api/get_conversations'); + expect(res.ok(), `get_conversations ${res.status()}`).toBeTruthy(); + const list = (await res.json()) as Array<{ id: string }>; + return list.map((c) => c.id); +} + +/** Bearer context carrying a raw agent api_key (v1 auth shape). */ +async function agentKeyRequest(key: string): Promise { + return playwright.request.newContext({ + baseURL: API_URL, + extraHTTPHeaders: { + Authorization: `Bearer ${key}`, + 'Content-Type': 'application/json', + }, + }); +} + +test.describe('tier-b · conversation visibility', () => { + test.beforeEach(async () => { + await resetDb(); + }); + + test('first-party chat (save_conversation omitted) persists as listed and appears in the sidebar', async ({ + browser, + }) => { + const { context, token } = await newUserContext(browser); + const api = await authedRequest(playwright, token); + try { + const convId = await streamOnce(api, { + question: 'first-party listed turn', + conversation_id: null, + isNoneDoc: true, + chunks: '0', + }); + expect(convId).toMatch(UUID_RE); + expect(await visibilityOf(convId)).toBe('listed'); + expect(await sidebarIds(api)).toContain(convId); + } finally { + await api.dispose(); + await context.close(); + } + }); + + test('first-party chat with save_conversation=false persists hidden and is excluded from the sidebar', async ({ + browser, + }) => { + const { context, sub, token } = await newUserContext(browser); + const api = await authedRequest(playwright, token); + try { + const convId = await streamOnce(api, { + question: 'first-party hidden turn', + conversation_id: null, + save_conversation: false, + isNoneDoc: true, + chunks: '0', + }); + // A real UUID is emitted now (not the legacy stringified "None"). + expect(convId).toMatch(UUID_RE); + expect(await visibilityOf(convId)).toBe('hidden'); + // The row persisted... + expect( + await countRows('conversations', { + sql: 'user_id = $1', + params: [sub], + }), + ).toBe(1); + // ...but it never surfaces in the sidebar. + expect(await sidebarIds(api)).not.toContain(convId); + } finally { + await api.dispose(); + await context.close(); + } + }); + + test('api-key agent chat hides by default and lists only on explicit opt-in', async ({ + browser, + }) => { + const { context, sub, token } = await newUserContext(browser); + const api = await authedRequest(playwright, token); + const multipart = await multipartAuthedRequest(token); + try { + const { key } = await publishClassicAgent(api, multipart, sub, 'vis-agent'); + + // api_key present → hidden by default. + const hidden = await streamOnce(api, { + question: 'agent default hidden', + conversation_id: null, + api_key: key, + isNoneDoc: true, + chunks: '0', + }); + expect(await visibilityOf(hidden)).toBe('hidden'); + + // Explicit opt-in → listed. + const listed = await streamOnce(api, { + question: 'agent opt-in listed', + conversation_id: null, + api_key: key, + save_conversation: true, + isNoneDoc: true, + chunks: '0', + }); + expect(await visibilityOf(listed)).toBe('listed'); + + // The owner's sidebar shows only the opted-in conversation. + const ids = await sidebarIds(api); + expect(ids).toContain(listed); + expect(ids).not.toContain(hidden); + } finally { + await multipart.dispose(); + await api.dispose(); + await context.close(); + } + }); + + test('v1 chat completions persist hidden by default and list only on docsgpt.save_conversation', async ({ + browser, + }) => { + const { context, sub, token } = await newUserContext(browser); + const api = await authedRequest(playwright, token); + const multipart = await multipartAuthedRequest(token); + try { + const { key } = await publishClassicAgent(api, multipart, sub, 'v1-vis-agent'); + const v1 = await agentKeyRequest(key); + try { + // Default: persisted, hidden — not in the owner's sidebar. + const r1 = await v1.post('/v1/chat/completions', { + data: { + model: 'docsgpt', + messages: [{ role: 'user', content: 'hi v1 default' }], + stream: false, + }, + }); + expect(r1.status(), await r1.text()).toBe(200); + expect( + await countRows('conversations', { sql: 'user_id = $1', params: [sub] }), + ).toBe(1); + expect( + await countRows('conversations', { + sql: "user_id = $1 AND visibility = 'listed'", + params: [sub], + }), + ).toBe(0); + expect(await sidebarIds(api)).toHaveLength(0); + + // Opt-in via the docsgpt extension → listed. + const r2 = await v1.post('/v1/chat/completions', { + data: { + model: 'docsgpt', + messages: [{ role: 'user', content: 'hi v1 listed' }], + stream: false, + docsgpt: { save_conversation: true }, + }, + }); + expect(r2.status(), await r2.text()).toBe(200); + expect( + await countRows('conversations', { sql: 'user_id = $1', params: [sub] }), + ).toBe(2); + expect( + await countRows('conversations', { + sql: "user_id = $1 AND visibility = 'listed'", + params: [sub], + }), + ).toBe(1); + // Exactly the opted-in conversation surfaces in the sidebar. + expect(await sidebarIds(api)).toHaveLength(1); + } finally { + await v1.dispose(); + } + } finally { + await multipart.dispose(); + await api.dispose(); + await context.close(); + } + }); +}); diff --git a/tests/test_v1_translator.py b/tests/test_v1_translator.py index f1eace7d..ae3156fe 100644 --- a/tests/test_v1_translator.py +++ b/tests/test_v1_translator.py @@ -214,25 +214,23 @@ class TestTranslateRequest: result = translate_request(data, "test-key") assert result["question"] == "What's 2+2?" assert result["api_key"] == "test-key" - # v1 conversations persist; ``save_conversation`` only controls whether - # they list in the sidebar (default off). - assert result["save_conversation"] is False + # v1 conversations persist but never list in the agent owner's + # sidebar; the translator no longer emits a display flag at all. + assert "save_conversation" not in result history = json.loads(result["history"]) assert len(history) == 1 assert history[0]["prompt"] == "Hello" - def test_save_conversation_opt_in_via_docsgpt_extension(self): + def test_legacy_save_conversation_flag_is_ignored(self): + # Pre-visibility clients send ``docsgpt.save_conversation: true`` + # meaning "persist" (the old contract). It must not leak into the + # internal request — visibility is not request-controllable on v1. data = { "messages": [{"role": "user", "content": "Hi"}], "docsgpt": {"save_conversation": True}, } result = translate_request(data, "key") - assert result["save_conversation"] is True - - def test_save_conversation_default_false(self): - data = {"messages": [{"role": "user", "content": "Hi"}]} - result = translate_request(data, "key") - assert result["save_conversation"] is False + assert "save_conversation" not in result def test_continuation_request(self): data = { @@ -303,10 +301,10 @@ class TestTranslateRequest: result = translate_request(data, "key") assert result["persist"] is False - def test_continuation_honours_explicit_save_conversation_override(self): - """An explicit docsgpt.save_conversation=false on the continuation wins.""" + def test_continuation_ignores_legacy_save_conversation_flag(self): + """``docsgpt.save_conversation`` is a dead flag — persist is separate.""" data = { - "docsgpt": {"save_conversation": False}, + "docsgpt": {"save_conversation": True}, "messages": [ {"role": "user", "content": "Search for X"}, { @@ -317,7 +315,9 @@ class TestTranslateRequest: ], } result = translate_request(data, "key") - assert result["save_conversation"] is False + assert "save_conversation" not in result + # Stateless continuation (no conversation_id) still skips persistence. + assert result["persist"] is False def test_continuation_with_top_level_conversation_id(self): """Standard clients send conversation_id at request level, not in messages."""