diff --git a/backend/app/main.py b/backend/app/main.py index dc7b39bef..15f6aa22f 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -2788,25 +2788,26 @@ async def chat_about_post( ) from exc cited_ids = list(answer.cited_post_ids) async with pool.acquire() as conn: - await persist_post_chat( - conn, - post_id, - question, - answer.answer_text, - cited_ids, - knowledge_cutoff=knowledge_cutoff, - ) - answer_evidence = await read_authorized_ask_evidence( - conn, - cited_post_ids=cited_ids, - corporate_entity_ids=account.corporate_entity_ids, - knowledge_cutoff=knowledge_cutoff, - ) - if not answer_evidence.all_citations_visible: - raise HTTPException( - status.HTTP_503_SERVICE_UNAVAILABLE, - "Post chat evidence changed before the answer could be returned", - ) + async with conn.transaction(): + await persist_post_chat( + conn, + post_id, + question, + answer.answer_text, + cited_ids, + knowledge_cutoff=knowledge_cutoff, + ) + answer_evidence = await read_authorized_ask_evidence( + conn, + cited_post_ids=cited_ids, + corporate_entity_ids=account.corporate_entity_ids, + knowledge_cutoff=knowledge_cutoff, + ) + if not answer_evidence.all_citations_visible: + raise HTTPException( + status.HTTP_503_SERVICE_UNAVAILABLE, + "Post chat evidence changed before the answer could be returned", + ) await publish_activity_event( valkey, post_id, diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 0411fd000..c1a00d7af 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -3583,6 +3583,88 @@ async def no_authorized_sources(*_args, **_kwargs): verify_conn.close() +def test_post_chat_failed_reauthorization_rolls_back_and_retry_succeeds( + client, demo_analyst_token, seeded_db, monkeypatch +) -> None: + """A post-chat citation race rolls back its cached answer before retry.""" + from lineageweave.post_chat import ChatAnswer + from backend.app.ask_project_history import AskEvidenceProjection + + class _FakePostChatClient: + available = True + + def __init__(self) -> None: + self.calls = 0 + + def answer(self, question: str, sources) -> ChatAnswer: + del question, sources + self.calls += 1 + if self.calls == 1: + return ChatAnswer( + answer_text="Synthetic answer that must be rolled back", + cited_post_ids=(seeded_db["public_post_id"],), + ) + return ChatAnswer(answer_text="Recovered post-chat answer", cited_post_ids=()) + + chat_client = _FakePostChatClient() + authorization_checks = 0 + + async def reauthorization(*_args, **_kwargs) -> AskEvidenceProjection: + nonlocal authorization_checks + authorization_checks += 1 + return AskEvidenceProjection( + all_citations_visible=authorization_checks > 1, + cited_posts=(), + project_histories=(), + project_histories_truncated=False, + knowledge_cutoff="2026-08-21T00:00:00Z", + ) + + monkeypatch.setattr("backend.app.main._post_chat_client", lambda: chat_client) + monkeypatch.setattr("backend.app.main.read_authorized_ask_evidence", reauthorization) + + post_id = seeded_db["own_private_post_id"] + first_question = "Post chat authorization race" + failed = client.post( + f"/api/posts/{post_id}/chat", + json={"question": first_question}, + headers={"Authorization": f"Bearer {demo_analyst_token}"}, + ) + assert failed.status_code == 503 + assert "Synthetic answer" not in failed.text + + retry = client.post( + f"/api/posts/{post_id}/chat", + json={"question": "Retry after post chat race"}, + headers={"Authorization": f"Bearer {demo_analyst_token}"}, + ) + assert retry.status_code == 200, retry.text + assert retry.json()["answer_text"] == "Recovered post-chat answer" + + verify_conn = psycopg2.connect(seeded_db["dsn"]) + verify_conn.autocommit = True + try: + with verify_conn.cursor() as cur: + cur.execute( + "select count(*) from post_chat_result where post_id = %s and question_text = %s", + (post_id, first_question), + ) + assert cur.fetchone()[0] == 0 + cur.execute( + "select question_text, answer_text from post_chat_result " + "where post_id = %s order by computed_at, question_norm", + (post_id,), + ) + assert cur.fetchall() == [("Retry after post chat race", "Recovered post-chat answer")] + cur.execute( + "select count(*) from post_chat_citation where post_id = %s", + (post_id,), + ) + assert cur.fetchone()[0] == 0 + finally: + verify_conn.close() + + def test_keymen_provider_error_does_not_leak_raw_error( client, demo_analyst_token, seeded_db, monkeypatch ) -> None: diff --git a/docs/adr/0129-global-ask-atomic-reauthorization.md b/docs/adr/0129-global-ask-atomic-reauthorization.md index 26e9bd49d..fb70f35eb 100644 --- a/docs/adr/0129-global-ask-atomic-reauthorization.md +++ b/docs/adr/0129-global-ask-atomic-reauthorization.md @@ -1,4 +1,4 @@ -# ADR 0129 — Global Ask turn persistence and final citation reauthorization share one transaction +# ADR 0129 — Ask turn persistence and final citation reauthorization share one transaction - Status: Accepted on this feature branch; protected-main adoption requires the normal stacked PR gates - Date: 2026-08-21 @@ -6,32 +6,33 @@ ## Context -Global Ask persists a new answer and its citation rows before performing the -final tenant, publication, and knowledge-cutoff reauthorization. If a cited -post changes between retrieval and that final check, the API correctly returns -a generic 503, but a committed rejected turn can poison the session and cause -the next request to receive 409. +Global Ask and post-scoped chat persist a new answer and its citation rows +before performing the final tenant, publication, and knowledge-cutoff +reauthorization. If a cited post changes between retrieval and that final +check, the API correctly returns a generic 503, but a committed rejected turn +can poison the session or leave an invalid cached answer. ## Decision 1. The `/api/ask` cited-answer path acquires one PostgreSQL connection and opens one outer transaction. -2. The new turn and normalized citation rows are inserted inside that outer - transaction, with the session row locked before the ordinal is allocated. -3. Final citation reauthorization runs on the same connection, with the same - tenant scope and knowledge cutoff, before the outer transaction exits. +2. The `/api/posts/{post_id}/chat` cited-answer path uses the same transaction + boundary for the result and normalized citation rows. +3. Global Ask allocates its turn ordinal while the session row is locked; both + paths run final citation reauthorization on the same connection, with the + same tenant scope and knowledge cutoff, before the transaction exits. 4. A failed final authorization raises the existing generic 503 inside the - transaction. PostgreSQL then rolls back only the new turn and citations; - previously committed turns remain unchanged. + transaction. PostgreSQL then rolls back only the new answer and citations; + previously committed turns and cached exchanges remain unchanged. 5. The no-authorized-source path keeps its existing empty-turn behavior and does not invent evidence. ## Consequences -- A visibility/publication race cannot leave a rejected citation in session - history. -- A follow-up request can reuse the same session after a rejected answer when - no previous citation remains. +- A visibility/publication race cannot leave a rejected citation in Global Ask + session history or post-scoped chat storage. +- A follow-up request can reuse the same Global Ask session after a rejected + answer, and a post-scoped question can be retried without a poisoned cache. - The answer, citation persistence, and final authorization share one database connection and lock lifetime; the transaction must not include the provider call or Valkey publication.