Open
Conversation
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Concurrent deleteChat calls corrupt shared singleton state
- Added a Mutex to wrap the entire deleteChat method, ensuring only one concurrent call can access the shared mutable state at a time.
- ✅ Fixed: Duplicate JSONObjectAdapter in same Gradle module
- Extracted the duplicate JSONObjectAdapter class to a shared location at duckchat-impl/common/JSONObjectAdapter.kt and updated both files to import from there.
- ✅ Fixed: Chat not deleted when tab navigated away
- Added TabChatIdsRepository to track chatIds when Duck.ai URLs are visited, and DataClearing now uses this repository to delete all chats associated with a tab regardless of current URL.
Or push these changes by commenting:
@cursor push 20b1229255
Preview (20b1229255)
diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
--- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
+++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
@@ -242,6 +242,7 @@
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository
import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ALWAYS
import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ASK_EVERY_TIME
+import com.duckduckgo.app.fire.store.TabChatIdsRepository
import com.duckduckgo.app.fire.store.TabVisitedSitesRepository
import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler
import com.duckduckgo.app.global.events.db.UserEventKey
@@ -508,6 +509,7 @@
private val serpEasterEggLogosToggles: SerpEasterEggLogosToggles,
private val serpLogos: SerpLogos,
private val tabVisitedSitesRepository: TabVisitedSitesRepository,
+ private val tabChatIdsRepository: TabChatIdsRepository,
private val pageLoadWideEvent: PageLoadWideEvent,
private val queryUrlPredictor: QueryUrlPredictor,
private val browserUiLockFeature: BrowserUiLockFeature,
@@ -993,6 +995,9 @@
if (domain != null) {
tabVisitedSitesRepository.recordVisitedSite(tabId, domain)
}
+ duckChat.extractChatId(url)?.let { chatId ->
+ tabChatIdsRepository.recordChatId(tabId, chatId)
+ }
}
}
}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
@@ -17,6 +17,7 @@
package com.duckduckgo.app.fire
import com.duckduckgo.app.fire.store.FireDataStore
+import com.duckduckgo.app.fire.store.TabChatIdsRepository
import com.duckduckgo.app.fire.store.TabVisitedSitesRepository
import com.duckduckgo.app.fire.wideevents.DataClearingWideEvent
import com.duckduckgo.app.global.view.ClearDataAction
@@ -28,7 +29,6 @@
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.duckchat.api.DuckAiChatClearer
import com.duckduckgo.duckchat.api.DuckAiFeatureState
-import com.duckduckgo.duckchat.api.DuckChat
import com.duckduckgo.history.api.NavigationHistory
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
@@ -57,9 +57,9 @@
private val duckAiFeatureState: DuckAiFeatureState,
private val dataClearingWideEvent: DataClearingWideEvent,
private val tabVisitedSitesRepository: TabVisitedSitesRepository,
+ private val tabChatIdsRepository: TabChatIdsRepository,
private val navigationHistory: NavigationHistory,
private val tabRepository: TabRepository,
- private val duckChat: DuckChat,
private val duckAiChatClearer: DuckAiChatClearer,
) : ManualDataClearing, AutomaticDataClearing {
@@ -69,8 +69,7 @@
val visitedSites = tabVisitedSitesRepository.getVisitedSites(tabId)
val clearDataResult = clearDataAction.clearDataForSpecificDomains(visitedSites)
- val tabUrl = tabRepository.getTab(tabId)?.url
- clearDuckAiChatIfNeeded(tabUrl)
+ clearDuckAiChatsForTab(tabId)
navigationHistory.removeHistoryForTab(tabId)
tabRepository.deleteTabAndSelectSource(tabId)
@@ -79,10 +78,12 @@
return clearDataResult
}
- private suspend fun clearDuckAiChatIfNeeded(tabUrl: String?) {
- if (tabUrl == null) return
- val chatId = duckChat.extractChatId(tabUrl) ?: return
- duckAiChatClearer.deleteChat(chatId)
+ private suspend fun clearDuckAiChatsForTab(tabId: String) {
+ val chatIds = tabChatIdsRepository.getChatIds(tabId)
+ for (chatId in chatIds) {
+ duckAiChatClearer.deleteChat(chatId)
+ }
+ tabChatIdsRepository.clearTab(tabId)
}
override suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean, wasAppUsedSinceLastClear: Boolean) {
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt
@@ -1,0 +1,28 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import androidx.room.Entity
+
+@Entity(
+ tableName = "tab_chat_ids",
+ primaryKeys = ["tabId", "chatId"],
+)
+data class TabChatIdEntity(
+ val tabId: String,
+ val chatId: String,
+)
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt
@@ -1,0 +1,37 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import androidx.room.Dao
+import androidx.room.Insert
+import androidx.room.OnConflictStrategy
+import androidx.room.Query
+
+@Dao
+interface TabChatIdsDao {
+ @Insert(onConflict = OnConflictStrategy.IGNORE)
+ suspend fun insert(entity: TabChatIdEntity)
+
+ @Query("SELECT chatId FROM tab_chat_ids WHERE tabId = :tabId")
+ suspend fun getChatIds(tabId: String): List<String>
+
+ @Query("DELETE FROM tab_chat_ids WHERE tabId = :tabId")
+ suspend fun clearTab(tabId: String)
+
+ @Query("DELETE FROM tab_chat_ids")
+ suspend fun clearAll()
+}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt
@@ -1,0 +1,66 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import com.duckduckgo.di.scopes.AppScope
+import com.squareup.anvil.annotations.ContributesBinding
+import dagger.SingleInstanceIn
+import javax.inject.Inject
+
+/**
+ * Tracks Duck.ai chat IDs associated with browser tabs.
+ *
+ * Used during single-tab burning to ensure chat data is cleared
+ * even if the user navigated away from the Duck.ai chat URL.
+ */
+interface TabChatIdsRepository {
+
+ /** Records that a Duck.ai chat with [chatId] was opened in the given [tabId]. */
+ suspend fun recordChatId(tabId: String, chatId: String)
+
+ /** Returns the set of chat IDs that were opened in the given [tabId]. */
+ suspend fun getChatIds(tabId: String): Set<String>
+
+ /** Removes all chat ID records for the given [tabId]. */
+ suspend fun clearTab(tabId: String)
+
+ /** Removes all chat ID records across all tabs. */
+ suspend fun clearAll()
+}
+
+@ContributesBinding(AppScope::class)
+@SingleInstanceIn(AppScope::class)
+class RealTabChatIdsRepository @Inject constructor(
+ private val dao: TabChatIdsDao,
+) : TabChatIdsRepository {
+
+ override suspend fun recordChatId(tabId: String, chatId: String) {
+ dao.insert(TabChatIdEntity(tabId = tabId, chatId = chatId))
+ }
+
+ override suspend fun getChatIds(tabId: String): Set<String> {
+ return dao.getChatIds(tabId).toSet()
+ }
+
+ override suspend fun clearTab(tabId: String) {
+ dao.clearTab(tabId)
+ }
+
+ override suspend fun clearAll() {
+ dao.clearAll()
+ }
+}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
@@ -21,9 +21,10 @@
@Database(
exportSchema = true,
- version = 1,
- entities = [TabVisitedSiteEntity::class],
+ version = 2,
+ entities = [TabVisitedSiteEntity::class, TabChatIdEntity::class],
)
abstract class TabVisitedSitesDatabase : RoomDatabase() {
abstract fun dao(): TabVisitedSitesDao
+ abstract fun chatIdsDao(): TabChatIdsDao
}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
@@ -39,4 +39,8 @@
@SingleInstanceIn(AppScope::class)
@Provides
fun provideDao(db: TabVisitedSitesDatabase): TabVisitedSitesDao = db.dao()
+
+ @SingleInstanceIn(AppScope::class)
+ @Provides
+ fun provideChatIdsDao(db: TabVisitedSitesDatabase): TabChatIdsDao = db.chatIdsDao()
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
@@ -18,6 +18,7 @@
import android.webkit.JavascriptInterface
import android.webkit.WebView
+import com.duckduckgo.duckchat.impl.common.JSONObjectAdapter
import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.JsMessage
import com.duckduckgo.js.messaging.api.JsMessageCallback
@@ -27,14 +28,8 @@
import com.duckduckgo.js.messaging.api.JsRequestResponse
import com.duckduckgo.js.messaging.api.SubscriptionEvent
import com.duckduckgo.js.messaging.api.SubscriptionEventData
-import com.squareup.moshi.FromJson
-import com.squareup.moshi.JsonReader
-import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
-import com.squareup.moshi.ToJson
import logcat.logcat
-import okio.Buffer
-import org.json.JSONException
import org.json.JSONObject
import javax.inject.Inject
@@ -125,21 +120,3 @@
const val METHOD_CLEAR_DATA_FAILED = "duckAiClearDataFailed"
}
}
-
-internal class JSONObjectAdapter {
- @FromJson
- fun fromJson(reader: JsonReader): JSONObject? {
- return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
- try {
- JSONObject(data)
- } catch (_: JSONException) {
- null
- }
- }
- }
-
- @ToJson
- fun toJson(writer: JsonWriter, value: JSONObject?) {
- value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
- }
-}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
@@ -37,6 +37,8 @@
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import logcat.logcat
@@ -55,6 +57,7 @@
private val duckAiDataClearingFeature: DuckAiDataClearingFeature,
) : DuckAiChatClearer {
+ private val mutex = Mutex()
private var webView: WebView? = null
private var pageLoadDeferred: CompletableDeferred<Unit>? = null
private var readyDeferred: CompletableDeferred<Unit>? = null
@@ -64,25 +67,27 @@
private var cachedScript: String? = null
override suspend fun deleteChat(chatId: String): Boolean {
- return withContext(dispatchers.main()) {
- try {
- val script = getScript()
- val wv = getOrCreateWebView(script)
+ return mutex.withLock {
+ withContext(dispatchers.main()) {
+ try {
+ val script = getScript()
+ val wv = getOrCreateWebView(script)
- var allSucceeded = true
- for (domain in DOMAINS) {
- val success = clearFromDomain(wv, domain, chatId)
- if (!success) {
- logcat { "DuckAiChatClearer: clearing failed for domain $domain, chatId $chatId" }
- allSucceeded = false
+ var allSucceeded = true
+ for (domain in DOMAINS) {
+ val success = clearFromDomain(wv, domain, chatId)
+ if (!success) {
+ logcat { "DuckAiChatClearer: clearing failed for domain $domain, chatId $chatId" }
+ allSucceeded = false
+ }
}
+ allSucceeded
+ } catch (e: Exception) {
+ logcat { "DuckAiChatClearer: deleteChat failed with ${e.message}" }
+ false
+ } finally {
+ tearDown()
}
- allSucceeded
- } catch (e: Exception) {
- logcat { "DuckAiChatClearer: deleteChat failed with ${e.message}" }
- false
- } finally {
- tearDown()
}
}
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt
new file mode 100644
--- /dev/null
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt
@@ -1,0 +1,43 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.duckchat.impl.common
+
+import com.squareup.moshi.FromJson
+import com.squareup.moshi.JsonReader
+import com.squareup.moshi.JsonWriter
+import com.squareup.moshi.ToJson
+import okio.Buffer
+import org.json.JSONException
+import org.json.JSONObject
+
+internal class JSONObjectAdapter {
+ @FromJson
+ fun fromJson(reader: JsonReader): JSONObject? {
+ return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
+ try {
+ JSONObject(data)
+ } catch (_: JSONException) {
+ null
+ }
+ }
+ }
+
+ @ToJson
+ fun toJson(writer: JsonWriter, value: JSONObject?) {
+ value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
+ }
+}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
@@ -18,6 +18,7 @@
import android.webkit.JavascriptInterface
import android.webkit.WebView
+import com.duckduckgo.duckchat.impl.common.JSONObjectAdapter
import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.JsMessage
import com.duckduckgo.js.messaging.api.JsMessageCallback
@@ -27,14 +28,8 @@
import com.duckduckgo.js.messaging.api.JsRequestResponse
import com.duckduckgo.js.messaging.api.SubscriptionEvent
import com.duckduckgo.js.messaging.api.SubscriptionEventData
-import com.squareup.moshi.FromJson
-import com.squareup.moshi.JsonReader
-import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
-import com.squareup.moshi.ToJson
import logcat.logcat
-import okio.Buffer
-import org.json.JSONException
import org.json.JSONObject
import javax.inject.Inject
@@ -117,21 +112,3 @@
const val JS_INTERFACE_NAME = "chatSuggestionsInterface"
}
}
-
-internal class JSONObjectAdapter {
- @FromJson
- fun fromJson(reader: JsonReader): JSONObject? {
- return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
- try {
- JSONObject(data)
- } catch (_: JSONException) {
- null
- }
- }
- }
-
- @ToJson
- fun toJson(writer: JsonWriter, value: JSONObject?) {
- value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
- }
-}
...hat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
Show resolved
Hide resolved
...hat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Feature flag gates chat ID extraction, blocking deletion
- Extracted URL-matching logic into matchesDuckChatUrlPattern so extractChatId no longer depends on the feature flag, allowing chat deletion regardless of feature flag state.
Or push these changes by commenting:
@cursor push d57be15e83
Preview (d57be15e83)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
@@ -642,7 +642,16 @@
override fun isDuckChatUrl(uri: Uri): Boolean {
if (!isDuckChatFeatureEnabled) return false
+ return matchesDuckChatUrlPattern(uri)
+ }
+ override fun extractChatId(url: String): String? {
+ val uri = Uri.parse(url) ?: return null
+ if (!matchesDuckChatUrlPattern(uri)) return null
+ return uri.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() }
+ }
+
+ private fun matchesDuckChatUrlPattern(uri: Uri): Boolean {
if (isDuckChatBang(uri)) return true
if (uri.host == DUCK_AI_HOST || uri.toString() == DUCK_AI_HOST) return true
@@ -654,12 +663,6 @@
}.getOrDefault(false)
}
- override fun extractChatId(url: String): String? {
- val uri = Uri.parse(url) ?: return null
- if (!isDuckChatUrl(uri)) return null
- return uri.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() }
- }
-
private fun isDuckChatBang(uri: Uri): Boolean = bangRegex?.containsMatchIn(uri.toString()) == true
override suspend fun wasOpenedBefore(): Boolean = duckChatFeatureRepository.wasOpenedBefore()
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Feature flag dependency changes
hasChatIdUI behavior- Changed hasChatId to perform a direct URL parse using toUri().getQueryParameter() instead of delegating to duckChat.extractChatId() which has feature flag dependency.
Or push these changes by commenting:
@cursor push 7444da476b
Preview (7444da476b)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
@@ -16,6 +16,7 @@
package com.duckduckgo.duckchat.impl.contextual
+import androidx.core.net.toUri
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.duckduckgo.anvil.annotations.ContributesViewModel
@@ -30,6 +31,7 @@
import com.duckduckgo.duckchat.impl.store.DuckChatContextualDataStore
import com.duckduckgo.js.messaging.api.SubscriptionEventData
import com.google.android.material.bottomsheet.BottomSheetBehavior
+import javax.inject.Inject
import kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.MutableStateFlow
@@ -41,7 +43,6 @@
import kotlinx.coroutines.withContext
import logcat.logcat
import org.json.JSONObject
-import javax.inject.Inject
@ContributesViewModel(FragmentScope::class)
class DuckChatContextualViewModel @Inject constructor(
@@ -529,9 +530,13 @@
}
private fun hasChatId(url: String?): Boolean {
- return url != null && duckChat.extractChatId(url) != null
+ return url?.toUri()?.getQueryParameter(CHAT_ID_PARAM)?.isNotBlank() == true
}
+ companion object {
+ private const val CHAT_ID_PARAM = "chatID"
+ }
+
private suspend fun shouldReuseStoredChatUrl(tabId: String): Boolean {
val lastClosedTimestamp = contextualDataStore.getTabClosedTimestamp(tabId) ?: return true
val timeoutMs = sessionTimeoutProvider.sessionTimeoutMillis()
...at-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
Outdated
Show resolved
Hide resolved
0nko
force-pushed
the
feature/ondrej/single-tab-burn-chat-deletion
branch
from
60840a9 to
ac2e2fe
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duck.ai chat deleted twice for Duck.ai tabs
- Removed the redundant duckChat.deleteChat() call from SingleTabFireDialogViewModel.onDeleteThisTabClicked() since DataClearing.clearSingleTabData() already handles Duck.ai chat deletion via clearDuckAiChatIfNeeded().
Or push these changes by commenting:
@cursor push f213d78551
Preview (f213d78551)
diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt
--- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt
+++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt
@@ -219,12 +219,8 @@
}
val result = withContext(dispatcherProvider.io()) {
- val selectedTab = tabRepository.getSelectedTab()
- val selectedTabId = selectedTab?.tabId
+ val selectedTabId = tabRepository.getSelectedTab()?.tabId
if (selectedTabId != null) {
- if (_viewState.value.isDuckAiTab) {
- selectedTab.url?.let { duckChat.deleteChat(it) }
- }
dataClearing.clearSingleTabData(selectedTabId)
} else {
null
app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated
CHAT_ID_PARAMconstant across three files- Consolidated the duplicated CHAT_ID_PARAM constant from RealDuckChat, DuckChatContextualViewModel, and InputScreenViewModel into the shared DuckChatConstants object.
Or push these changes by commenting:
@cursor push 18b003cd26
Preview (18b003cd26)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt
@@ -19,6 +19,7 @@
object DuckChatConstants {
const val HOST_DUCK_AI = "duck.ai"
const val JS_MESSAGING_FEATURE_NAME = "aiChat"
+ const val CHAT_ID_PARAM = "chatID"
object JsResponseKeys {
const val OK = "ok"
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
@@ -36,6 +36,7 @@
import com.duckduckgo.duckchat.api.DuckAiFeatureState
import com.duckduckgo.duckchat.api.DuckChat
import com.duckduckgo.duckchat.api.DuckChatSettingsNoParams
+import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM
import com.duckduckgo.duckchat.impl.DuckChatConstants.HOST_DUCK_AI
import com.duckduckgo.duckchat.impl.clearing.DuckChatDeleter
import com.duckduckgo.duckchat.impl.feature.AIChatImageUploadFeature
@@ -850,7 +851,6 @@
private const val PLACEMENT_QUERY_VALUE = "sidebar"
private const val BANG_QUERY_NAME = "bang"
private const val BANG_QUERY_VALUE = "true"
- private const val CHAT_ID_PARAM = "chatID"
private const val DEFAULT_SESSION_ALIVE = 60
private const val REVOKE_URL = "https://duckduckgo.com/revoke-duckai-access"
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
@@ -23,6 +23,7 @@
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.FragmentScope
import com.duckduckgo.duckchat.api.DuckChat
+import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM
import com.duckduckgo.duckchat.impl.DuckChatInternal
import com.duckduckgo.duckchat.impl.helper.DuckChatJSHelper
import com.duckduckgo.duckchat.impl.helper.NativeAction
@@ -542,8 +543,4 @@
val elapsedMs = timeProvider.currentTimeMillis() - lastClosedTimestamp
return elapsedMs <= timeoutMs
}
-
- companion object {
- private const val CHAT_ID_PARAM = "chatID"
- }
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt
@@ -44,6 +44,7 @@
import com.duckduckgo.common.utils.extensions.toBinaryString
import com.duckduckgo.duckchat.api.DuckAiFeatureState
import com.duckduckgo.duckchat.api.DuckChat
+import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM
import com.duckduckgo.duckchat.impl.feature.DuckChatFeature
import com.duckduckgo.duckchat.impl.inputscreen.ui.InputScreenConfigResolver
import com.duckduckgo.duckchat.impl.inputscreen.ui.command.Command
@@ -803,7 +804,6 @@
companion object {
const val DUCK_SCHEME = "duck"
private const val CHAT_SUGGESTIONS_DEBOUNCE_MS = 150L
- private const val CHAT_ID_PARAM = "chatID"
}
}
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
Outdated
Show resolved
Hide resolved
CDRussell
approved these changes
Mar 9, 2026
| } | ||
| } | ||
| allSucceeded | ||
| } catch (e: Exception) { |
Member
There was a problem hiding this comment.
since this also catches cancellation exceptions, would be good to include ensureActive()
CDRussell
reviewed
Mar 9, 2026
| ) | ||
| interface DuckAiDataClearingFeature { | ||
|
|
||
| @Toggle.DefaultValue(DefaultFeatureValue.TRUE) |
Member
There was a problem hiding this comment.
does it make sense to have this be true when it has no settings? should leave it as false by default and let remote config handle setting it to true and providing the extra settings?
0nko
force-pushed
the
feature/ondrej/single-tab-burn-chat-deletion
branch
from
a65ad17 to
03d70b0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Caught Exception swallows CancellationException breaking structured concurrency
- Added coroutineContext.ensureActive() at the start of the catch block to re-throw CancellationException and preserve structured concurrency.
- ✅ Fixed: Feature defaults to enabled without remote config settings
- Changed DefaultFeatureValue.TRUE to DefaultFeatureValue.FALSE so remote config must explicitly enable the feature with proper settings.
Or push these changes by commenting:
@cursor push 7ec5a3466c
Preview (7ec5a3466c)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
@@ -36,6 +36,7 @@
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
@@ -87,6 +88,7 @@
}
allSucceeded
} catch (e: Exception) {
+ coroutineContext.ensureActive()
logcat { "DuckChatDeleter: deleteChat failed with ${e.message}" }
false
} finally {
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/feature/DuckAiDataClearingFeature.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/feature/DuckAiDataClearingFeature.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/feature/DuckAiDataClearingFeature.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/feature/DuckAiDataClearingFeature.kt
@@ -27,6 +27,6 @@
)
interface DuckAiDataClearingFeature {
- @Toggle.DefaultValue(DefaultFeatureValue.TRUE)
+ @Toggle.DefaultValue(DefaultFeatureValue.FALSE)
fun self(): Toggle
}
...hat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
Show resolved
Hide resolved
...uckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/feature/DuckAiDataClearingFeature.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing early-return for disabled feature causes unnecessary WebView work
- Added early-return check at the start of deleteChat() that returns false immediately when duckAiDataClearingFeature is disabled, preventing all unnecessary WebView creation and timeout delays.
Or push these changes by commenting:
@cursor push e4efef8e49
Preview (e4efef8e49)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
@@ -72,6 +72,8 @@
private var cachedScript: String? = null
override suspend fun deleteChat(chatId: String): Boolean {
+ if (!duckAiDataClearingFeature.self().isEnabled()) return false
+
return mutex.withLock {
withContext(dispatchers.main()) {
try {
...hat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
Show resolved
Hide resolved
When a Duck.ai tab was burned, duckChat.deleteChat() was being called twice - once directly in SingleTabFireDialogViewModel.onDeleteThisTabClicked() when isDuckAiTab was true, and again in DataClearing.clearSingleTabData() via clearDuckAiChatIfNeeded(). Each deleteChat() invocation spins up a headless WebView and performs JS messaging, so calling it twice is wasteful and could cause unexpected behavior. The fix removes the redundant direct call in the ViewModel since clearSingleTabData() already properly handles Duck.ai chat deletion. Applied via @cursor push command
Move the duplicated CHAT_ID_PARAM constant from three separate companion objects into the shared DuckChatConstants object to reduce code duplication and maintenance risk. - RealDuckChat.kt: remove private const, add import - DuckChatContextualViewModel.kt: remove companion object, add import - InputScreenViewModel.kt: remove private const, add import - DuckChatConstants.kt: add shared CHAT_ID_PARAM constant Applied via @cursor push command
- Add ensureActive() in catch block to properly re-throw CancellationException - Change DuckAiDataClearingFeature default from TRUE to FALSE to let remote config enable it with correct settings Applied via @cursor push command
When duckAiDataClearingFeature is disabled (the default), deleteChat() now returns false immediately instead of spinning up a WebView, loading pages for two domains, injecting JS, and waiting for timeouts that will never complete. This avoids up to ~10 seconds of unnecessary suspended delay per tab burn when the feature is disabled. Applied via @cursor push command
0nko
force-pushed
the
feature/ondrej/single-tab-burn-chat-deletion
branch
from
bbd0bf6 to
19b07f5
Compare
8 tasks
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/project/488551667048375/task/1213087856649984?focus=true
Description
This PR adds Duck.ai chat deletion when a Duck.ai tab is burned.
Steps to test this PR
For testing this feature, you need to use the following privacy config URL:
https://raw.githubusercontent.com/duckduckgo/privacy-configuration/90d195f34fb14f4eb47f519294376ad068e48df7/pr-4676/v3/android-config.jsonimprovedDataClearingOptionsandsingleTabFireDialogFFs enabledNote
Medium Risk
Adds new Duck.ai chat-deletion flow using a headless WebView + JS messaging and wires it into single-tab burning, which can affect user data retention/clearing behavior. While guarded by a remote toggle, failures/timeouts or URL parsing issues could lead to unexpected chats not being deleted (or extra work done) during tab burn.
Overview
Single-tab burning now triggers Duck.ai conversation deletion by passing the tab URL to a new
DuckChat.deleteChatAPI, which extractschatIDand delegates to a newDuckChatDeleterthat clears chat storage via a headless WebView/JS-messaging flow (behind theduckAiDataClearingremote toggle).clearDataForSpecificDomainsis simplified to always excludeduckduckgo.com/duck.aifrom WebStorage deletion (no longer conditional), and the single-tab fire dialog updates copy to show “Delete This Chat” when burning a Duck.ai tab. Tests and fakes are updated to cover the new deletion behavior and shared JSON adapter usage.Written by Cursor Bugbot for commit 19b07f5. This will update automatically on new commits. Configure here.