-
Notifications
You must be signed in to change notification settings - Fork 1
fix: roll back unauthorized Global Ask turns #399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5822948
779d337
f101ab9
2150bcc
19c6821
b9acb07
d1ea448
825d276
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,10 @@ class GlobalAskConversationNotFound(LookupError): | |
| """The requested conversation is absent or owned by another account.""" | ||
|
|
||
|
|
||
| class GlobalAskEvidenceChanged(RuntimeError): | ||
| """A cited post became unauthorized before the new turn could commit.""" | ||
|
|
||
|
|
||
| def conversation_title(question: str) -> str: | ||
| """Use the first question as a bounded, readable transcript label.""" | ||
| compact = " ".join(question.strip().split()) | ||
|
|
@@ -250,6 +254,32 @@ async def fetch_conversation( | |
| } | ||
|
|
||
|
|
||
| async def _ensure_citations_visible( | ||
| conn: asyncpg.Connection, | ||
| conversation_id: UUID, | ||
| turn_ordinal: int, | ||
| cited_post_count: int, | ||
| can_see_post: Callable[[asyncpg.Record], bool], | ||
| ) -> None: | ||
| """Lock and re-authorize new citations before their transaction commits.""" | ||
| rows = await conn.fetch( | ||
| """ | ||
| select relation.cited_post_id::text as post_id, | ||
| post.post_title, post.visibility_code, post.corporate_entity_id, | ||
| post.author_account_id, post.source_detail_state_code | ||
| from global_ask_turn_citation relation | ||
| join source_post post on post.post_id = relation.cited_post_id | ||
| where relation.global_ask_session_id = $1 | ||
| and relation.turn_ordinal = $2 | ||
| for share of post | ||
| """, | ||
| conversation_id, | ||
| turn_ordinal, | ||
| ) | ||
| if len(rows) != cited_post_count or any(not can_see_post(row) for row in rows): | ||
| raise GlobalAskEvidenceChanged | ||
|
Comment on lines
+257
to
+280
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Re-authorization covers only cited posts, not sources The re-check at global_ask_history.py re-authorizes cited posts under Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
|
|
||
| async def persist_turn( | ||
| conn: asyncpg.Connection, | ||
| user_account_id: str, | ||
|
|
@@ -260,6 +290,7 @@ async def persist_turn( | |
| source_post_ids: Iterable[str], | ||
| cited_post_ids: Iterable[str], | ||
| cited_post_evidence: Iterable[dict[str, Any]], | ||
| can_see_post: Callable[[asyncpg.Record], bool] | None = None, | ||
| ) -> UUID: | ||
| source_ids = list(dict.fromkeys(str(post_id) for post_id in source_post_ids)) | ||
| source_set = set(source_ids) | ||
|
|
@@ -348,5 +379,13 @@ async def persist_turn( | |
| "update global_ask_session set updated_at = now() where global_ask_session_id = $1", | ||
| conversation_id, | ||
| ) | ||
| if can_see_post is not None: | ||
| await _ensure_citations_visible( | ||
| conn, | ||
| conversation_id, | ||
| ordinal, | ||
| len(cited_ids), | ||
| can_see_post, | ||
| ) | ||
|
Comment on lines
+382
to
+389
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Row-lock re-check closes the retrieval/persist race Sources are gathered on a separate connection, then Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| assert conversation_id is not None | ||
| return conversation_id | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Rolled-back turn leaves the conversation reusable
For an existing conversation the ordinal is
max+1and theupdated_atbump lives inside the transaction, so a rollback leaves no inserted turn, no ordinal gap, and no stale timestamp. A follow-up retry recomputes the ordinal cleanly, which the new test confirms.(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.