Add chatIdOrNull to duckchat api module and drop chatID string duplication#8631
Open
GerardPaligot wants to merge 3 commits into
Open
Add chatIdOrNull to duckchat api module and drop chatID string duplication#8631GerardPaligot wants to merge 3 commits into
GerardPaligot wants to merge 3 commits into
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
aitorvs
reviewed
May 20, 2026
The "chatID" query parameter name was duplicated across three call sites extracting the chatID from a Duck.ai URL, each implementing the lookup differently and only two of them gating on isDuckChatUrl. Lift the operation onto the public DuckChat interface as chatIdOrNull(uri) so the literal lives in one place and the safety guard is enforced for every consumer — closing a latent foot-gun in DuckChatContextualViewModel.hasChatId that previously trusted any URL carrying a chatID query param. DuckChatConstants.CHAT_ID_PARAM is removed; the raw "chatID" string now only lives in RealDuckChat's private const. URL building stays on the impl-only DuckChatInternal since every builder is impl-internal.
…lication The "chatID" query parameter name was extracted in three different shapes across modules — a custom Uri extension in :app, a private helper in :duckchat-impl, and an inline lookup in DuckChatContextualViewModel — only two of which gated on isDuckChatUrl. Lift the operation onto a top-level Uri.toChatIdOrNull(DuckChat) extension in :duckchat-api so the literal lives in one place and the safety guard is enforced for every consumer, closing a latent foot-gun in DuckChatContextualViewModel.hasChatId that previously trusted any URL carrying a chatID query param. URL building stays on the impl-only DuckChatInternal since every builder is impl-internal and never crosses the module boundary.
GerardPaligot
force-pushed
the
feature/gerard/duckchat-extract-chatid
branch
from
09514e4 to
48d4109
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 48d4109. Configure here.
GerardPaligot
changed the title
buildChatUrl was the only Duck.ai URL builder that did not call
nativeInputParameters(), so callers that previously composed their URL
via getDuckChatUrl("", false) + manual appendQueryParameter("chatID", …)
silently lost the native-input=true param after migrating onto
buildChatUrl(chatId). Pull nativeInputParameters() into buildChatUrl so
the "single source of truth for the Duck.ai chat URL shape" actually
produces the same URL shape as every other builder.
Adds two RealDuckChatTest cases that pin the contract: with the native
input setting on, buildChatUrl emits ?chatID=…&native-input=true&duckai=5;
with it off, native-input is absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/task/1214962354449266
Description
Promotes the
"chatID"query parameter handling out of three impl-private helpers into one function on the publicDuckChatinterface:chatIdOrNull(uri: Uri): String?. As a side effect,DuckChatContextualViewModel.hasChatIdnow correctly gates onisDuckChatUrl— previously a crafted URL likehttps://evil.com?chatID=abcwas treated as a Duck.ai chat.Steps to test this PR
Chat-history clears (DuckChatDataClearingPlugin + DuckAiTabsCleanupPlugin)
Chat resume from input suggestions
chatIDin the URL.UI changes
n/a
Note
Medium Risk
Touches Duck.ai URL generation and chat-history clearing/tab-closing flows; a mistake could prevent chat resume or cause incorrect tab closures, though changes are mostly consolidations with updated tests.
Overview
Introduces a public
Uri.toChatIdOrNull(duckChat)helper induckchat-apiand migrates multiple call sites to use it, removing duplicatedchatIDextraction logic and the impl-privateCHAT_ID_PARAMconstant.Standardizes “resume chat” URL creation by routing recent-chat selection through
DuckChatInternal.buildChatUrl(chatId)and updatesRealDuckChat.buildChatUrlto include native-input parameters when enabled.Tightens “has chat” checks to require
isDuckChatUrl(viatoChatIdOrNull), avoiding treating non-Duck.ai URLs with achatIDparameter as valid chats, and updates/extends unit tests accordingly.Reviewed by Cursor Bugbot for commit 5a6e8c1. Bugbot is set up for automated code reviews on this repo. Configure here.