Skip to content

Verify Thread dataset state after export and auto-promote when server has none#6818

Draft
agners wants to merge 2 commits intohome-assistant:mainfrom
agners:fix/thread-export-not-promoted
Draft

Verify Thread dataset state after export and auto-promote when server has none#6818
agners wants to merge 2 commits intohome-assistant:mainfrom
agners:fix/thread-export-not-promoted

Conversation

@agners
Copy link
Copy Markdown
Member

@agners agners commented May 7, 2026

Summary

When the developer-settings "Sync Thread credentials" flow exported the device's preferred dataset to Home Assistant, the app treated a successful thread/add_dataset_tlv reply as "credentials in sync". In practice the WebSocket command never makes the just-added dataset the server's preferred one — that auto-promote behaviour was removed in Core #108278, which moved the preferred-dataset selection behind the explicit thread/set_preferred_dataset call. As a result, after Core #108278 the app's export flow would loop: every sync re-detected the same mismatch, prompted the user, exported again, and reported success while the server's preferred dataset had not changed at all.

This PR fixes the export flow to behave correctly with the new Core contract:

  • sendThreadDatasetExportResult now returns a sealed ExportResult so callers can distinguish "sent and now preferred" from "sent but server still prefers another network".
  • After a successful add the server is queried again and:
    • if no dataset is preferred, the just-exported one is promoted via the new thread/set_preferred_dataset WebSocket call;
    • if a different dataset is already preferred (typically because a server-managed border router announces it), the server's choice is left untouched and the UI reports the honest mismatch with a dedicated string;
    • if the just-exported dataset is already preferred, the UI reports the in-sync state.
  • A new WebSocket repository method setThreadPreferredDataset(datasetId) wraps thread/set_preferred_dataset.
  • The developer settings screen gets a new string thread_debug_result_exported_not_preferred so the user can tell the difference between a successful sync and an export-that-did-not-promote.

The WebViewPresenterImpl and MatterCommissioningViewModel callers are adjusted to the new return type without behaviour changes.

Related: home-assistant/core#108278thread/add_dataset_tlv no longer promotes the added dataset to preferred, making this client-side change necessary.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Screenshots

Link to pull request in documentation repositories

Any other notes

The ThreadManager.sendThreadDatasetExportResult signature changes from String? to ExportResult. All in-tree callers are updated; this is a developer-facing breaking change inside the app module only.

agners and others added 2 commits May 7, 2026 14:31
The thread/add_dataset_tlv command never promotes a dataset to preferred
on the server, so a separate call is needed to make a just-added dataset
the preferred one.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously sendThreadDatasetExportResult treated a successful
thread/add_dataset_tlv call as "credentials in sync", but the server
never auto-promotes datasets added via that command. When the server
already had a different preferred dataset (typically announced by a
border router it manages), the next sync would re-detect the mismatch
and prompt the user to export again, looping indefinitely.

After a successful add, re-query the server's datasets and:
- if no dataset is preferred, promote the just-exported one;
- if a different dataset is preferred, leave it alone and report it;
- if it is already preferred, report the in-sync state.

The result type is now a sealed ExportResult so callers can distinguish
"sent and now preferred" from "sent but server still prefers another
network", and the developer settings screen shows a dedicated message
for the latter case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 7, 2026 13:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the app’s Thread credential export flow to match Home Assistant Core’s updated WebSocket contract where thread/add_dataset_tlv no longer auto-promotes the added dataset to “preferred”. The app now re-verifies the server state after export and can explicitly promote the exported dataset when the server has no preferred dataset.

Changes:

  • Introduces a sealed ThreadManager.ExportResult so callers can distinguish “not sent / failed / sent (and whether the server prefers it)”.
  • Adds a WebSocket wrapper for thread/set_preferred_dataset and uses it to auto-promote when the server has no preferred dataset.
  • Updates developer settings and WebView/Matter flows to handle the new export result and show a more precise outcome.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
common/src/main/res/values/strings.xml Adds a new developer-settings string for “exported but not preferred” outcome
common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt Documents addThreadDataset behavior and adds setThreadPreferredDataset API
common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt Implements thread/set_preferred_dataset WebSocket call
app/src/minimal/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt Updates minimal flavor stub to new ExportResult return type
app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt Adapts logging and state transitions to new ExportResult type
app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt Adds ExportResult sealed class and updates KDoc/return type
app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt Updates developer Thread sync UI messaging based on ExportResult
app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt Re-queries server after export and optionally promotes exported dataset

Comment on lines +322 to +331
val datasets = webSocket.getThreadDatasets()
val preferred = datasets?.firstOrNull { it.preferred }
when {
preferred == null -> {
val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Timber.d("Thread: promoted exported dataset to preferred on the server")
true
} else {
false
Comment on lines +156 to +162
val out = "${context.getString(
commonR.string.thread_debug_result_mismatch,
)} ${context.getString(
commonR.string.thread_debug_result_mismatch_detail,
sent.networkName,
)}"
view.onThreadDebugResult(out, null)
Comment on lines +423 to +427
override suspend fun setThreadPreferredDataset(datasetId: String): Boolean {
val response = webSocketCore.sendMessage(
mapOf(
"type" to "thread/set_preferred_dataset",
"dataset_id" to datasetId,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed

@agners agners marked this pull request as draft May 7, 2026 13:44
"Thread ${if (sent is ThreadManager.ExportResult.Sent) {
"sent credential for ${sent.networkName} (server prefers exported: ${sent.serverPrefersExported})"
} else {
"did not send credential"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"did not send credential"
"did not send credential reason $sent"

when {
preferred == null -> {
val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need to check if core has this endpoint no? Or the min version we use for thread already contains this endpoint?

if (result.resultCode != Activity.RESULT_OK || !coreSupportsThread(serverId)) {
return ThreadManager.ExportResult.NotSent
}
val credentials = ThreadNetworkCredentials.fromIntentSenderResultData(result.data!!)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid crashing here by checking if data is null and returning NotSent.

val webSocket = serverManager.webSocketRepository(serverId)
val added = try {
webSocket.addThreadDataset(credentials.activeOperationalDataset)
} catch (e: Exception) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a coroutine, we have a bad design in the app by catching all Exception but we can't change it now. But because of that we have to reThrow CancellationException otherwise it let the scope into an invalid state.

preferred.extendedPanId.equals(deviceExtPan, ignoreCase = true) -> true
else -> false
}
} catch (e: Exception) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here we need to rethrow

if (added) return threadNetworkCredentials.networkName
} catch (e: Exception) {
Timber.e(e, "Error while executing server new Thread credentials request")
@OptIn(ExperimentalStdlibApi::class)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@OptIn(ExperimentalStdlibApi::class)

Not needed anymore (you can probably also remove it in the rest of the file)

// surface an honest result we re-query and compare extended PAN IDs. If the server has
// no preferred dataset at all (e.g. no border router announcing one), promote the
// just-exported dataset; we never override an existing server preference.
val deviceExtPan = credentials.extendedPanId.toHexString(HexFormat.UpperCase)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val deviceExtPan = credentials.extendedPanId.toHexString(HexFormat.UpperCase)
val deviceExtPan = credentials.extendedPanId.toHexString()

Since you ignore the case bellow we don't care

// no preferred dataset at all (e.g. no border router announcing one), promote the
// just-exported dataset; we never override an existing server preference.
val deviceExtPan = credentials.extendedPanId.toHexString(HexFormat.UpperCase)
val serverPrefersExported = try {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole logic could be extracted into a private function.

preferred == null -> {
val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Timber.d("Thread: promoted exported dataset to preferred on the server")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Timber.d("Thread: promoted exported dataset to preferred on the server")
Timber.d("Promote exported dataset to preferred network on the server")

} catch (e: Exception) {
Timber.e(e, "Error while executing server new Thread credentials request")
@OptIn(ExperimentalStdlibApi::class)
override suspend fun sendThreadDatasetExportResult(
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are up for the task we could start unit testing this function. Claude is pretty good at this exercise on Android.

else -> false
}
} catch (e: Exception) {
Timber.w(e, "Unable to verify Thread preferred dataset after export")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also be setting the prefered dataset since you potentially makes 2 call.

commonR.string.thread_debug_result_mismatch,
)} ${context.getString(commonR.string.thread_debug_result_mismatch_detail, submitted)}"
view.onThreadDebugResult(out, null)
when (val sent = threadManager.sendThreadDatasetExportResult(result, serverId)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it can be simplified maybe something like this

                when (val sent = threadManager.sendThreadDatasetExportResult(result, serverId)) {
                    is ThreadManager.ExportResult.Sent -> when {
                        isDeviceOnly -> view.onThreadDebugResult(
                            context.getString(commonR.string.thread_debug_result_exported),
                            true,
                        )
                        sent.serverPrefersExported == true -> view.onThreadDebugResult(
                            context.getString(commonR.string.thread_debug_result_match),
                            true,
                        )
                        else -> {
                            val headlineRes = if (sent.serverPrefersExported == false) {
                                commonR.string.thread_debug_result_exported_not_preferred
                            } else {
                                commonR.string.thread_debug_result_mismatch
                            }
                            val detail = context.getString(
                                commonR.string.thread_debug_result_mismatch_detail,
                                sent.networkName,
                            )
                            view.onThreadDebugResult("${context.getString(headlineRes)} $detail", null)
                        }
                    }

Comment on lines +321 to +335
val serverPrefersExported = try {
val datasets = webSocket.getThreadDatasets()
val preferred = datasets?.firstOrNull { it.preferred }
when {
preferred == null -> {
val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Timber.d("Thread: promoted exported dataset to preferred on the server")
true
} else {
false
}
}
preferred.extendedPanId.equals(deviceExtPan, ignoreCase = true) -> true
else -> false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val serverPrefersExported = try {
val datasets = webSocket.getThreadDatasets()
val preferred = datasets?.firstOrNull { it.preferred }
when {
preferred == null -> {
val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Timber.d("Thread: promoted exported dataset to preferred on the server")
true
} else {
false
}
}
preferred.extendedPanId.equals(deviceExtPan, ignoreCase = true) -> true
else -> false
val serverPrefersExported = try {
val datasets = webSocket.getThreadDatasets().orEmpty()
val preferred = datasets.firstOrNull { it.preferred }
if (preferred != null) {
preferred.extendedPanId.equals(deviceExtPan, ignoreCase = true)
} else {
val ours = datasets.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) }
if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) {
Timber.d("Thread: promoted exported dataset to preferred on the server")
true
} else {
false
}
}

The when using instead of a if is sometimes less readable IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants