From 3db962997f348d225887740a806241dd564daac1 Mon Sep 17 00:00:00 2001 From: Jebran Syed Date: Fri, 31 Jul 2026 16:59:02 -0700 Subject: [PATCH 1/2] Port shell display semantics and add set-timer to mobile-2 sample The Android sample rendered every appendDisplay chunk into one growing bubble, so a request like "create grocery list" showed the dispatcher's routing annotation ("routed to list - recent topic") glued in front of the answer, while the Electron shell shows only "Created list: grocery". Display pipeline - Add AgentDisplayThread, a Kotlin port of the shell's AgentMessageContainer.setMessage accumulation rules: content for a request is kept as ordered chunks, honouring DisplayAppendMode (inline / block / temporary / step). A trailing "temporary" chunk is flushed when the next update arrives, and "step" seals the bubble so the next update starts a new one - matching chatPanel.ts. - Message now carries a list of MessageSegment, each with its own DisplayMessageKind, so info/status annotations render de-emphasised instead of being concatenated into the answer (the shell's chat-message-kind-* styling). - Drop AgentMessageKind "toast"/"inline" messages, which the shell renders outside the chat bubble. - Clear the per-request display state on disconnect as well as connect. set-timer client action - Add TimerActionParser for the takeAction("set-timer", ...) payload emitted by the androidMobile setTimer action. The duration is re-validated client side (floor, positive, <= 86400s per the documented AlarmClock.EXTRA_LENGTH range) because takeAction is fire-and-forget and carries no schema guarantee over the wire. Out-of-range values are rejected rather than clamped so a mistranslated duration surfaces in the log. - Launch ACTION_SET_TIMER with EXTRA_SKIP_UI = true so the countdown starts in the background and a chat request never pulls the user out of the conversation; a toast confirms. Alarm and timer now share one intent-launch helper. - Declare the SET_TIMER entry required by Android 11+ package visibility. No new permission is needed: the existing com.android.alarm.permission.SET_ALARM covers both. - Unknown client actions stay logged-and-ignored, so this is compatible with servers that do not emit set-timer. Server-side setTimer support is in PR #2780; until that lands the client path is simply never exercised. Tests: 44 unit tests (14 new for AgentDisplayThread, 9 for TimerActionParser); assembleDebug and lintDebug clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- android/samples/mobile-2/README.md | 23 +- .../mobile-2/app/src/main/AndroidManifest.xml | 3 + .../typeagentchat/AgentDisplayThread.kt | 147 ++++++++++ .../typeagentchat/DisplayContentParser.kt | 51 +++- .../com/example/typeagentchat/MainActivity.kt | 123 +++++++-- .../java/com/example/typeagentchat/Message.kt | 65 ++++- .../typeagentchat/TimerActionParser.kt | 70 +++++ .../example/typeagentchat/WebSocketManager.kt | 254 ++++++++++++------ .../typeagentchat/AgentDisplayThreadTest.kt | 168 ++++++++++++ .../typeagentchat/DisplayContentParserTest.kt | 42 +++ .../typeagentchat/TimerActionParserTest.kt | 88 ++++++ 11 files changed, 904 insertions(+), 130 deletions(-) create mode 100644 android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt create mode 100644 android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt create mode 100644 android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt create mode 100644 android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt diff --git a/android/samples/mobile-2/README.md b/android/samples/mobile-2/README.md index 5109e9b14e..dda6f2faee 100644 --- a/android/samples/mobile-2/README.md +++ b/android/samples/mobile-2/README.md @@ -9,10 +9,31 @@ An Android Jetpack Compose chat client that connects to a TypeAgent agent-server - TypeAgent agent-server RPC protocol: - `joinConversation` / `submitCommand` - Inbound `appendDisplay`, `setDisplay`, `setDisplayInfo`, and command completion events -- Incremental assistant response streaming into a single bubble per `requestId` + - Inbound `takeAction` client actions +- Incremental assistant response streaming into a single bubble per `requestId`, honouring + the SDK's `DisplayAppendMode` (`inline`, `block`, `temporary`, `step`) and + `DisplayMessageKind` styling the same way the Electron shell does - DevTunnel authentication via `X-Tunnel-Authorization` header - Build-time configuration via environment variables and `BuildConfig` +## Device actions + +The app implements the `takeAction` client actions emitted by the `androidMobile` +agent. Unknown actions are logged and ignored, so the app stays compatible with +servers that emit actions this sample does not support. + +| Client action | Android intent | Notes | +|---|---|---| +| `set-alarm` | `AlarmClock.ACTION_SET_ALARM` | Opens the clock app so the user can confirm the alarm. | +| `set-timer` | `AlarmClock.ACTION_SET_TIMER` | Starts the countdown in the background (`EXTRA_SKIP_UI = true`) and confirms with a toast, so a chat request never yanks the user out of the conversation. Durations outside the documented 1..86400 second range are rejected rather than clamped. | + +Both require the `com.android.alarm.permission.SET_ALARM` permission (declared in +the manifest, install-time only) and matching `` entries so +`resolveActivity` works under Android 11+ package visibility rules. + +> `set-timer` requires the server-side `androidMobile` `setTimer` action. Install +> the agent with `@package install androidMobile` in the TypeAgent CLI/shell. + ## Prerequisites - Android Studio (recent stable version) diff --git a/android/samples/mobile-2/app/src/main/AndroidManifest.xml b/android/samples/mobile-2/app/src/main/AndroidManifest.xml index 980f9fd1ea..3d17dc289e 100644 --- a/android/samples/mobile-2/app/src/main/AndroidManifest.xml +++ b/android/samples/mobile-2/app/src/main/AndroidManifest.xml @@ -9,6 +9,9 @@ + + + diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt new file mode 100644 index 0000000000..e1c02d0faa --- /dev/null +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt @@ -0,0 +1,147 @@ +package com.example.typeagentchat + +/** + * Mirrors TypeAgent's `DisplayAppendMode` from `@typeagent/agent-sdk`. + * + * `REPLACE` stands in for the `undefined` append mode the shell uses when + * `ClientIO.setDisplay` is called: it clears everything already rendered for + * the request before rendering the new content. + */ +internal enum class DisplayAppendMode { + REPLACE, + INLINE, + BLOCK, + TEMPORARY, + STEP +} + +internal fun parseDisplayAppendMode(raw: String?): DisplayAppendMode { + return when (raw?.trim()?.lowercase()) { + "inline" -> DisplayAppendMode.INLINE + "temporary" -> DisplayAppendMode.TEMPORARY + "step" -> DisplayAppendMode.STEP + else -> DisplayAppendMode.BLOCK + } +} + +/** + * `AgentMessageKind` values that the shell renders outside of the agent bubble. + */ +internal fun isEphemeralAgentMessageKind(kind: String?): Boolean { + return when (kind?.trim()?.lowercase()) { + "toast", "inline" -> true + else -> false + } +} + +internal data class RenderedAgentMessage( + val segments: List +) { + val text: String + get() = segments.joinToString("\n\n") { it.text } + + val format: MessageFormat + get() = segments.fold(MessageFormat.TEXT) { acc, segment -> + acc.mergeWith(segment.format) + } + + val isEmpty: Boolean + get() = segments.isEmpty() +} + +/** + * Kotlin port of the shell's `AgentMessageContainer.setMessage` accumulation + * rules (ts/packages/chat-ui/src/chatPanel.ts). Content for one request is kept + * as an ordered list of chunks so that a trailing `temporary` chunk can be + * discarded when the next display update arrives - which is how the shell ends + * up showing only "Created list: grocery" instead of every progress update. + */ +internal class AgentDisplayThread { + + private class Chunk( + var text: String, + var format: MessageFormat, + val kind: MessageKind + ) + + private val chunks = mutableListOf() + private var lastMode: DisplayAppendMode? = null + private var hasTemporaryTail = false + + val isEmpty: Boolean + get() = chunks.isEmpty() + + fun setMessage(content: ParsedDisplayContent, mode: DisplayAppendMode) { + flushTemporary() + + val text = content.text + when (mode) { + DisplayAppendMode.REPLACE -> { + chunks.clear() + if (text.isNotEmpty()) { + chunks += Chunk(text, content.format, content.kind) + } + lastMode = null + } + + DisplayAppendMode.INLINE -> { + if (text.isNotEmpty()) { + // The shell only reuses the previous content div when its + // kind style matches (`matchKindStyle`). + val last = chunks.lastOrNull() + if (lastMode == DisplayAppendMode.INLINE && + last != null && + last.kind == content.kind + ) { + last.text += text + last.format = last.format.mergeWith(content.format) + } else { + chunks += Chunk(text, content.format, content.kind) + } + } + lastMode = DisplayAppendMode.INLINE + } + + DisplayAppendMode.TEMPORARY -> { + if (text.isNotEmpty()) { + chunks += Chunk(text, content.format, content.kind) + hasTemporaryTail = true + } + lastMode = DisplayAppendMode.TEMPORARY + } + + DisplayAppendMode.BLOCK, DisplayAppendMode.STEP -> { + if (text.isNotEmpty()) { + chunks += Chunk(text, content.format, content.kind) + } + lastMode = mode + } + } + } + + /** + * Drops a trailing `temporary` chunk, matching the shell's + * `messageDiv.lastChild?.remove()` flush. Returns true when content changed. + */ + fun flushTemporary(): Boolean { + if (!hasTemporaryTail) { + return false + } + hasTemporaryTail = false + lastMode = null + return chunks.removeLastOrNull() != null + } + + fun render(): RenderedAgentMessage { + val segments = chunks + .map { chunk -> + MessageSegment( + text = chunk.text.trim('\n'), + format = chunk.format, + kind = chunk.kind + ) + } + .filter { it.text.isNotEmpty() } + return RenderedAgentMessage(segments) + } +} diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/DisplayContentParser.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/DisplayContentParser.kt index f324f39ff4..34902414e2 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/DisplayContentParser.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/DisplayContentParser.kt @@ -5,7 +5,8 @@ import org.json.JSONObject internal data class ParsedDisplayContent( val text: String, - val format: MessageFormat + val format: MessageFormat, + val kind: MessageKind = MessageKind.NONE ) internal fun extractAgentMessageContent(value: Any?): ParsedDisplayContent { @@ -16,6 +17,16 @@ internal fun extractAgentMessageContent(value: Any?): ParsedDisplayContent { return extractDisplayContent(agentMessage.optNullable("message")) } +/** + * `IAgentMessage.kind` (AgentMessageKind). The shell routes "toast" and + * "inline" kinds outside of the chat bubble entirely. + */ +internal fun extractAgentMessageKind(value: Any?): String? { + val agentMessage = value as? JSONObject ?: return null + val kind = agentMessage.optString("kind") + return kind.ifBlank { null } +} + private fun extractDisplayContent(value: Any?): ParsedDisplayContent { return when (value) { null, JSONObject.NULL -> ParsedDisplayContent("", MessageFormat.TEXT) @@ -26,12 +37,14 @@ private fun extractDisplayContent(value: Any?): ParsedDisplayContent { ) is JSONObject -> { + val kind = MessageKind.parse(value.optString("kind")) when { value.optString("type") == "structured" -> { - extractBestAlternate(value.optJSONArray("alternates")) + extractBestAlternate(value.optJSONArray("alternates"), kind) ?: ParsedDisplayContent( text = "Structured content is not supported in this client yet.", - format = MessageFormat.TEXT + format = MessageFormat.TEXT, + kind = kind ) } @@ -40,7 +53,8 @@ private fun extractDisplayContent(value: Any?): ParsedDisplayContent { extractSupportedTypedContent( declaredType = declaredType, content = value.optNullable("content"), - alternates = value.optJSONArray("alternates") + alternates = value.optJSONArray("alternates"), + kind = kind ) } @@ -56,43 +70,53 @@ private fun extractDisplayContent(value: Any?): ParsedDisplayContent { private fun extractSupportedTypedContent( declaredType: String, content: Any?, - alternates: JSONArray? + alternates: JSONArray?, + kind: MessageKind ): ParsedDisplayContent { val normalizedType = declaredType.lowercase() return when (normalizedType) { "markdown" -> ParsedDisplayContent( text = messageContentToText(content, declaredType), - format = MessageFormat.MARKDOWN + format = MessageFormat.MARKDOWN, + kind = kind ) "text" -> ParsedDisplayContent( text = messageContentToText(content, declaredType), - format = MessageFormat.TEXT + format = MessageFormat.TEXT, + kind = kind ) - else -> extractBestAlternate(alternates) ?: fallbackTypedContent(normalizedType, content) + else -> extractBestAlternate(alternates, kind) + ?: fallbackTypedContent(normalizedType, content, kind) } } private fun fallbackTypedContent( declaredType: String, - content: Any? + content: Any?, + kind: MessageKind ): ParsedDisplayContent { val fallbackText = messageContentToText(content, declaredType) return when (declaredType) { "html", "iframe" -> ParsedDisplayContent( text = htmlFallbackText(fallbackText), - format = MessageFormat.TEXT + format = MessageFormat.TEXT, + kind = kind ) else -> ParsedDisplayContent( text = fallbackText, - format = MessageFormat.TEXT + format = MessageFormat.TEXT, + kind = kind ) } } -private fun extractBestAlternate(alternates: JSONArray?): ParsedDisplayContent? { +private fun extractBestAlternate( + alternates: JSONArray?, + kind: MessageKind +): ParsedDisplayContent? { if (alternates == null || alternates.length() == 0) { return null } @@ -109,7 +133,8 @@ private fun extractBestAlternate(alternates: JSONArray?): ParsedDisplayContent? return extractSupportedTypedContent( declaredType = alternateType, content = alternate.optNullable("content"), - alternates = null + alternates = null, + kind = kind ) } } diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt index 0efa3ec9b8..4e6a5abf71 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt @@ -58,6 +58,8 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.TextStyle +import androidx.compose.ui.text.font.FontStyle import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.input.ImeAction import androidx.compose.foundation.text.KeyboardActions @@ -75,11 +77,15 @@ class MainActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() - webSocketManager.setClientActionHandler { action -> - runOnUiThread { - launchSetAlarmIntent(action) + webSocketManager.setClientActionHandler(object : WebSocketManager.ClientActionHandler { + override fun onSetAlarm(action: SetAlarmAction) { + runOnUiThread { launchSetAlarmIntent(action) } } - } + + override fun onSetTimer(action: SetTimerAction) { + runOnUiThread { launchSetTimerIntent(action) } + } + }) webSocketManager.connect( url = tunnelUrl, tunnelToken = tunnelToken @@ -111,33 +117,67 @@ class MainActivity : ComponentActivity() { putExtra(AlarmClock.EXTRA_MESSAGE, action.originalRequest) } } + launchClockIntent( + intent = intent, + actionName = "set-alarm", + detail = "hour=${action.hour} minute=${action.minute}", + successMessage = "Alarm set for %02d:%02d".format(action.hour, action.minute), + missingAppMessage = "No alarm app is available on this device.", + deniedMessage = "This app is not allowed to set alarms." + ) + } + + /** + * Handles `takeAction("set-timer", ...)` from the androidMobile agent. + * + * `EXTRA_SKIP_UI` is true so the clock app starts the countdown in the + * background instead of coming to the foreground. The reference + * implementation in TypeAgent PR #2780 (`JavaScriptInterface.setTimer`) + * passes false; we diverge deliberately so a voice/chat request never + * yanks the user out of the conversation. The confirmation toast is then + * the only in-app feedback, so it is not optional. + */ + private fun launchSetTimerIntent(action: SetTimerAction) { + val intent = Intent(AlarmClock.ACTION_SET_TIMER).apply { + putExtra(AlarmClock.EXTRA_LENGTH, action.durationInSeconds) + putExtra(AlarmClock.EXTRA_SKIP_UI, true) + if (action.originalRequest.isNotBlank()) { + putExtra(AlarmClock.EXTRA_MESSAGE, action.originalRequest) + } + } + launchClockIntent( + intent = intent, + actionName = "set-timer", + detail = "durationInSeconds=${action.durationInSeconds}", + successMessage = "Timer set for ${formatTimerDuration(action.durationInSeconds)}", + missingAppMessage = "No timer app is available on this device.", + deniedMessage = "This app is not allowed to set timers." + ) + } + + private fun launchClockIntent( + intent: Intent, + actionName: String, + detail: String, + successMessage: String, + missingAppMessage: String, + deniedMessage: String + ) { val target = intent.resolveActivity(packageManager) Log.d( TAG, - "Launching set-alarm intent hour=${action.hour} minute=${action.minute} target=${target?.flattenToShortString() ?: "none"}" + "Launching $actionName intent $detail target=${target?.flattenToShortString() ?: "none"}" ) try { startActivity(intent) - Log.d(TAG, "set-alarm intent dispatched to clock app") - Toast.makeText( - this, - "Alarm set for %02d:%02d".format(action.hour, action.minute), - Toast.LENGTH_SHORT - ).show() + Log.d(TAG, "$actionName intent dispatched to clock app") + Toast.makeText(this, successMessage, Toast.LENGTH_SHORT).show() } catch (_: ActivityNotFoundException) { - Log.e(TAG, "No alarm app available to handle set-alarm intent") - Toast.makeText( - this, - "No alarm app is available on this device.", - Toast.LENGTH_SHORT - ).show() + Log.e(TAG, "No app available to handle $actionName intent") + Toast.makeText(this, missingAppMessage, Toast.LENGTH_SHORT).show() } catch (error: SecurityException) { - Log.e(TAG, "Missing permission to set alarm", error) - Toast.makeText( - this, - "This app is not allowed to set alarms.", - Toast.LENGTH_SHORT - ).show() + Log.e(TAG, "Missing permission for $actionName", error) + Toast.makeText(this, deniedMessage, Toast.LENGTH_SHORT).show() } } @@ -585,12 +625,14 @@ private fun MessageBubble(message: Message) { style = MaterialTheme.typography.labelMedium, fontWeight = FontWeight.SemiBold ) - ChatMessageText( - text = message.text, - format = message.format, - color = textColor, - style = MaterialTheme.typography.bodyLarge - ) + message.segments.forEach { segment -> + ChatMessageText( + text = segment.text, + format = segment.format, + color = segmentColor(segment.kind, textColor), + style = segmentTextStyle(segment.kind) + ) + } if (!message.isUser && !message.isFinal) { Text( text = "Responding...", @@ -602,3 +644,26 @@ private fun MessageBubble(message: Message) { } } } + +/** + * Mirrors the shell's `chat-message-kind-*` styling: routing notes and other + * `info`/`status` annotations are de-emphasised so the actual answer stands out. + */ +@Composable +private fun segmentColor(kind: MessageKind, baseColor: Color): Color { + return when (kind) { + MessageKind.INFO, MessageKind.STATUS -> baseColor.copy(alpha = 0.6f) + MessageKind.WARNING -> MaterialTheme.colorScheme.tertiary + MessageKind.ERROR -> MaterialTheme.colorScheme.error + MessageKind.SUCCESS, MessageKind.NONE -> baseColor + } +} + +@Composable +private fun segmentTextStyle(kind: MessageKind): TextStyle { + return if (kind.isSecondary) { + MaterialTheme.typography.bodySmall.copy(fontStyle = FontStyle.Italic) + } else { + MaterialTheme.typography.bodyLarge + } +} diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/Message.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/Message.kt index efe914de21..465825ccb4 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/Message.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/Message.kt @@ -4,11 +4,72 @@ import java.util.UUID data class Message( val id: String = UUID.randomUUID().toString(), - val text: String, - val format: MessageFormat = MessageFormat.TEXT, + val segments: List, val isUser: Boolean, val requestId: String? = null, val isFinal: Boolean = false +) { + constructor( + text: String, + format: MessageFormat = MessageFormat.TEXT, + isUser: Boolean, + requestId: String? = null, + isFinal: Boolean = false + ) : this( + segments = listOf(MessageSegment(text = text, format = format)), + isUser = isUser, + requestId = requestId, + isFinal = isFinal + ) + + val text: String + get() = segments.joinToString("\n\n") { it.text } + + val format: MessageFormat + get() = segments.fold(MessageFormat.TEXT) { acc, segment -> + acc.mergeWith(segment.format) + } +} + +/** + * Mirrors TypeAgent's `DisplayMessageKind` from `@typeagent/agent-sdk`. The + * shell turns this into a `chat-message-kind-` CSS class so routing + * annotations such as "routed to list - recent topic" render de-emphasised + * next to the actual answer. + */ +enum class MessageKind { + NONE, + INFO, + STATUS, + WARNING, + ERROR, + SUCCESS; + + val isSecondary: Boolean + get() = this == INFO || this == STATUS + + companion object { + fun parse(raw: String?): MessageKind { + return when (raw?.trim()?.lowercase()) { + "info" -> INFO + "status" -> STATUS + "warning" -> WARNING + "error" -> ERROR + "success" -> SUCCESS + else -> NONE + } + } + } +} + +/** + * One rendered block inside a chat bubble, equivalent to a single content + * `
` the shell appends to `.chat-message-content`. + */ +data class MessageSegment( + val text: String, + val format: MessageFormat = MessageFormat.TEXT, + val kind: MessageKind = MessageKind.NONE ) enum class MessageFormat { diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt new file mode 100644 index 0000000000..cb6ec402f9 --- /dev/null +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt @@ -0,0 +1,70 @@ +package com.example.typeagentchat + +import org.json.JSONObject +import kotlin.math.floor + +internal data class SetTimerAction( + val originalRequest: String, + val durationInSeconds: Int +) + +/** + * `AlarmClock.EXTRA_LENGTH` is documented as accepting 1..86400 seconds + * (24 hours). Anything outside that is rejected rather than clamped so a + * mistranslated duration surfaces in the log instead of silently setting a + * timer the user did not ask for. + */ +private const val MAX_TIMER_SECONDS = 86_400L + +/** + * Parses the payload of `takeAction("set-timer", ...)` emitted by the + * androidMobile agent's `SetTimerAction` (TypeAgent PR #2780): + * + * ```ts + * parameters: { originalRequest: string; durationInSeconds: number } + * ``` + * + * The agent already floors the value and rejects non-positive durations, but + * this client re-validates because `takeAction` is fire-and-forget and carries + * no schema guarantee over the wire. + */ +internal fun parseSetTimerActionPayload(data: Any?): SetTimerAction? { + val payload = data as? JSONObject ?: return null + val originalRequest = payload.optString("originalRequest").trim() + val duration = readDurationSeconds(payload) ?: return null + if (duration <= 0L || duration > MAX_TIMER_SECONDS) { + return null + } + + return SetTimerAction( + originalRequest = originalRequest, + durationInSeconds = duration.toInt() + ) +} + +private fun readDurationSeconds(payload: JSONObject): Long? { + val raw = payload.opt("durationInSeconds") + val value = when (raw) { + is Number -> raw.toDouble() + is String -> raw.trim().toDoubleOrNull() + else -> null + } ?: return null + + if (value.isNaN() || value.isInfinite()) { + return null + } + return floor(value).toLong() +} + +/** Human-readable duration for the confirmation toast, e.g. "1 h 5 min 30 s". */ +internal fun formatTimerDuration(totalSeconds: Int): String { + val hours = totalSeconds / 3600 + val minutes = (totalSeconds % 3600) / 60 + val seconds = totalSeconds % 60 + val parts = buildList { + if (hours > 0) add("$hours h") + if (minutes > 0) add("$minutes min") + if (seconds > 0 || isEmpty()) add("$seconds s") + } + return parts.joinToString(" ") +} diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt index af690dcd03..e310864ff9 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt @@ -22,6 +22,8 @@ class WebSocketManager { private val nextCallId = AtomicInteger(0) private val connectionGeneration = AtomicInteger(0) private val pendingInvokes = mutableMapOf() + private val displayThreads = mutableMapOf() + private val displayMessageIds = mutableMapOf() private var webSocket: WebSocket? = null private var conversationId: String? = null @@ -68,6 +70,8 @@ class WebSocketManager { pendingUserInteraction = null conversationId = null connectionId = null + displayThreads.clear() + displayMessageIds.clear() } _pendingYesNoPrompt.value = null webSocket?.cancel() @@ -207,6 +211,8 @@ class WebSocketManager { webSocket = null synchronized(lock) { pendingUserInteraction = null + displayThreads.clear() + displayMessageIds.clear() } _pendingYesNoPrompt.value = null failPendingInvokes("App closed") @@ -328,9 +334,10 @@ class WebSocketManager { requestId = null, content = text ) - appendAssistantContent( + applyDisplay( requestId = null, - content = ParsedDisplayContent(text = text, format = MessageFormat.TEXT) + content = ParsedDisplayContent(text = text, format = MessageFormat.TEXT), + mode = DisplayAppendMode.BLOCK ) } } @@ -391,15 +398,17 @@ class WebSocketManager { when (methodName) { "appendDisplay" -> { val requestId = extractRequestId(args.opt(0)) + val kind = extractAgentMessageKind(args.opt(0)) val content = extractAgentMessageContent(args.opt(0)) - val mode = args.optString(1) + val mode = parseDisplayAppendMode(args.optString(1)) logInboundEvent( type = "append-display", requestId = requestId, - content = content.text + content = content.text, + detail = "mode=$mode kind=${kind.orEmpty()}" ) - if (shouldAppendToAssistantBubble(content, mode)) { - appendAssistantContent(requestId = requestId, content = content) + if (!isEphemeralAgentMessageKind(kind)) { + applyDisplay(requestId = requestId, content = content, mode = mode) } } @@ -425,13 +434,21 @@ class WebSocketManager { "setDisplay" -> { val requestId = extractRequestId(args.opt(0)) + val kind = extractAgentMessageKind(args.opt(0)) val content = extractAgentMessageContent(args.opt(0)) logInboundEvent( type = "set-display", requestId = requestId, - content = content.text + content = content.text, + detail = "kind=${kind.orEmpty()}" ) - replaceAssistantContent(requestId = requestId, content = content) + if (!isEphemeralAgentMessageKind(kind)) { + applyDisplay( + requestId = requestId, + content = content, + mode = DisplayAppendMode.REPLACE + ) + } } "notify" -> { @@ -529,11 +546,14 @@ class WebSocketManager { TAG, "takeAction received action=$actionName requestId=${requestId.orEmpty()} data=${stringifyDisplayValue(actionData)}" ) - if (actionName != "set-alarm") { - Log.d(TAG, "takeAction ignored: unsupported action=$actionName") - return + when (actionName) { + "set-alarm" -> handleSetAlarmAction(actionData) + "set-timer" -> handleSetTimerAction(actionData) + else -> Log.d(TAG, "takeAction ignored: unsupported action=$actionName") } + } + private fun handleSetAlarmAction(actionData: Any?) { val alarm = parseSetAlarmActionPayload(actionData) if (alarm == null) { Log.e( @@ -542,19 +562,49 @@ class WebSocketManager { ) return } - val handler = synchronized(lock) { clientActionHandler } - if (handler == null) { + val handler = requireClientActionHandler( + "set-alarm", + "hour=${alarm.hour} minute=${alarm.minute}" + ) ?: return + Log.d( + TAG, + "Dispatching set-alarm to client handler hour=${alarm.hour} minute=${alarm.minute}" + ) + handler.onSetAlarm(alarm) + } + + private fun handleSetTimerAction(actionData: Any?) { + val timer = parseSetTimerActionPayload(actionData) + if (timer == null) { Log.e( TAG, - "set-alarm parsed (hour=${alarm.hour} minute=${alarm.minute}) but no client action handler is registered" + "Invalid set-timer payload: ${stringifyDisplayValue(actionData)}" ) return } + val handler = requireClientActionHandler( + "set-timer", + "durationInSeconds=${timer.durationInSeconds}" + ) ?: return Log.d( TAG, - "Dispatching set-alarm to client handler hour=${alarm.hour} minute=${alarm.minute}" + "Dispatching set-timer to client handler durationInSeconds=${timer.durationInSeconds}" ) - handler.onSetAlarm(alarm) + handler.onSetTimer(timer) + } + + private fun requireClientActionHandler( + actionName: String, + detail: String + ): ClientActionHandler? { + val handler = synchronized(lock) { clientActionHandler } + if (handler == null) { + Log.e( + TAG, + "$actionName parsed ($detail) but no client action handler is registered" + ) + } + return handler } private fun handleDisplayLogEvent(event: JSONObject) { @@ -562,9 +612,23 @@ class WebSocketManager { when (eventType) { "append-display" -> { val requestId = extractRequestId(event.opt("requestId")) ?: extractRequestId(event.optJSONObject("message")) + val kind = extractAgentMessageKind(event.opt("message")) + val content = extractAgentMessageContent(event.opt("message")) + val mode = parseDisplayAppendMode(event.optString("mode")) + logInboundEvent(eventType, requestId, content.text, "mode=$mode") + if (!isEphemeralAgentMessageKind(kind)) { + applyDisplay(requestId, content, mode) + } + } + + "set-display" -> { + val requestId = extractRequestId(event.opt("requestId")) ?: extractRequestId(event.optJSONObject("message")) + val kind = extractAgentMessageKind(event.opt("message")) val content = extractAgentMessageContent(event.opt("message")) logInboundEvent(eventType, requestId, content.text) - appendAssistantContent(requestId, content) + if (!isEphemeralAgentMessageKind(kind)) { + applyDisplay(requestId, content, DisplayAppendMode.REPLACE) + } } "set-display-info" -> { @@ -604,66 +668,95 @@ class WebSocketManager { } } - private fun appendAssistantContent(requestId: String?, content: ParsedDisplayContent) { - val normalizedText = normalizeAssistantContentText(content) - if (normalizedText.isEmpty()) { - return - } - + /** + * Port of the shell's `ChatPanel.addAgentMessage` / `replaceAgentMessage`: + * all display content for one request accumulates in a single bubble, and a + * trailing `temporary` status chunk is discarded as soon as the next update + * arrives. + */ + private fun applyDisplay( + requestId: String?, + content: ParsedDisplayContent, + mode: DisplayAppendMode + ) { synchronized(lock) { - val updated = _messages.value.toMutableList() - val existingIndex = updated.indexOfLast { - !it.isUser && it.requestId == requestId && !it.isFinal + val key = threadKey(requestId) + val thread = displayThreads.getOrPut(key) { AgentDisplayThread() } + thread.setMessage(content, mode) + syncThreadMessage(key, requestId, thread) + if (mode == DisplayAppendMode.STEP) { + commitThread(key, requestId, thread) } - if (existingIndex >= 0) { - val existing = updated[existingIndex] - updated[existingIndex] = existing.copy( - text = existing.text + normalizedText, - format = existing.format.mergeWith(content.format) - ) - } else { - updated += Message( - text = normalizedText, - format = content.format, - isUser = false, - requestId = requestId - ) - } - _messages.value = updated } } - private fun replaceAssistantContent(requestId: String?, content: ParsedDisplayContent) { - val normalizedText = normalizeAssistantContentText(content) - if (normalizedText.isEmpty()) { - return - } + private fun threadKey(requestId: String?): String { + return requestId ?: DEFAULT_THREAD_KEY + } - synchronized(lock) { - val updated = _messages.value.toMutableList() - val existingIndex = updated.indexOfLast { - !it.isUser && it.requestId == requestId && !it.isFinal - } - if (existingIndex >= 0) { - val existing = updated[existingIndex] - updated[existingIndex] = existing.copy( - text = normalizedText, - format = content.format - ) + private fun syncThreadMessage( + key: String, + requestId: String?, + thread: AgentDisplayThread + ) { + val rendered = thread.render() + val updated = _messages.value.toMutableList() + val messageId = displayMessageIds[key] + val index = if (messageId == null) -1 else updated.indexOfFirst { it.id == messageId } + + if (index >= 0) { + if (rendered.isEmpty) { + updated.removeAt(index) + displayMessageIds.remove(key) } else { - updated += Message( - text = normalizedText, - format = content.format, - isUser = false, - requestId = requestId - ) + updated[index] = updated[index].copy(segments = rendered.segments) } + } else { + if (rendered.isEmpty) { + return + } + val message = Message( + segments = rendered.segments, + isUser = false, + requestId = requestId + ) + displayMessageIds[key] = message.id + updated += message + } + _messages.value = updated + } + + /** + * Equivalent of the shell's `completeRequest`: drop any lingering temporary + * status text, seal the bubble, and forget the thread so the next display + * update starts a fresh bubble. + */ + private fun commitThread( + key: String, + requestId: String?, + thread: AgentDisplayThread + ) { + thread.flushTemporary() + syncThreadMessage(key, requestId, thread) + displayThreads.remove(key) + val messageId = displayMessageIds.remove(key) ?: return + val updated = _messages.value.toMutableList() + val index = updated.indexOfFirst { it.id == messageId } + if (index >= 0) { + updated[index] = updated[index].copy(isFinal = true) _messages.value = updated } } private fun finalizeAssistantMessage(requestId: String?) { synchronized(lock) { + val key = threadKey(requestId) + val thread = displayThreads[key] + if (thread != null) { + commitThread(key, requestId, thread) + return + } + val updated = _messages.value.toMutableList() val existingIndex = if (requestId == null) { updated.indexOfLast { !it.isUser && !it.isFinal } @@ -761,10 +854,15 @@ class WebSocketManager { pending.forEach { it.onError(reason) } } - private fun logInboundEvent(type: String, requestId: String?, content: String) { + private fun logInboundEvent( + type: String, + requestId: String?, + content: String, + detail: String? = null + ) { Log.d( TAG, - "Inbound event type=$type requestId=${requestId.orEmpty()} connectionId=${connectionId.orEmpty()} contentLength=${content.length}" + "Inbound event type=$type requestId=${requestId.orEmpty()} connectionId=${connectionId.orEmpty()} contentLength=${content.length}${detail?.let { " $it" }.orEmpty()}" ) } @@ -791,23 +889,6 @@ class WebSocketManager { } } - private fun shouldAppendToAssistantBubble(content: ParsedDisplayContent, mode: String): Boolean { - if (content.text.isEmpty()) { - return false - } - if (mode == "temporary") { - return false - } - if (content.format == MessageFormat.MARKDOWN) { - return true - } - return !content.text.startsWith("[") - } - - private fun normalizeAssistantContentText(content: ParsedDisplayContent): String { - return content.text - } - private fun handleRequestChoiceCall(args: JSONArray) { val requestId = extractRequestId(args.opt(0)) val choiceId = args.optString(1).orEmpty() @@ -1095,12 +1176,13 @@ class WebSocketManager { "${choiceLines.joinToString("\n")}\nType the option number." } } - appendAssistantContent( + applyDisplay( requestId = requestId, content = ParsedDisplayContent( text = "$displayPrompt\n$instructions", format = MessageFormat.TEXT - ) + ), + mode = DisplayAppendMode.BLOCK ) } @@ -1168,10 +1250,12 @@ class WebSocketManager { private const val NORMAL_CLOSURE_STATUS = 1000 private const val AGENT_SERVER_CHANNEL = "agent-server" private const val CLIENT_IO_CHANNEL_PREFIX = "clientio:" + private const val DEFAULT_THREAD_KEY = "__no_request__" } - internal fun interface ClientActionHandler { + internal interface ClientActionHandler { fun onSetAlarm(action: SetAlarmAction) + fun onSetTimer(action: SetTimerAction) } } diff --git a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt new file mode 100644 index 0000000000..fd48b690e6 --- /dev/null +++ b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt @@ -0,0 +1,168 @@ +package com.example.typeagentchat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class AgentDisplayThreadTest { + + private fun text(value: String) = ParsedDisplayContent(value, MessageFormat.TEXT) + + private fun markdown(value: String) = ParsedDisplayContent(value, MessageFormat.MARKDOWN) + + private fun info(value: String) = + ParsedDisplayContent(value, MessageFormat.TEXT, MessageKind.INFO) + + @Test + fun `temporary status updates are flushed by the final result`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Translating 'create grocery list'"), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("routed to list"), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("recent topic"), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("Created list: grocery"), DisplayAppendMode.BLOCK) + + assertEquals("Created list: grocery", thread.render().text) + } + + @Test + fun `set display replaces everything already rendered`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("partial"), DisplayAppendMode.BLOCK) + thread.setMessage(text("more"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Created list: grocery"), DisplayAppendMode.REPLACE) + + assertEquals("Created list: grocery", thread.render().text) + } + + @Test + fun `temporary content is visible until the next update`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + + assertEquals("Thinking...", thread.render().text) + } + + @Test + fun `flush temporary removes a trailing status`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + assertTrue(thread.flushTemporary()) + + assertEquals("Answer", thread.render().text) + } + + @Test + fun `flush temporary is a no-op after a block append`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + + assertEquals(false, thread.flushTemporary()) + assertEquals("Answer", thread.render().text) + } + + @Test + fun `empty temporary content does not consume a real chunk`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + thread.setMessage(text(""), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("More"), DisplayAppendMode.BLOCK) + + assertEquals("Answer\n\nMore", thread.render().text) + } + + @Test + fun `consecutive inline appends merge into one chunk`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Hello"), DisplayAppendMode.INLINE) + thread.setMessage(text(" world"), DisplayAppendMode.INLINE) + + assertEquals("Hello world", thread.render().text) + } + + @Test + fun `first inline after a block starts a new chunk`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Block"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Inline"), DisplayAppendMode.INLINE) + + assertEquals("Block\n\nInline", thread.render().text) + } + + @Test + fun `blocks are separated so markdown stays valid`() { + val thread = AgentDisplayThread() + + thread.setMessage(markdown("### Lists"), DisplayAppendMode.BLOCK) + thread.setMessage(markdown("1. grocery"), DisplayAppendMode.BLOCK) + + val rendered = thread.render() + assertEquals("### Lists\n\n1. grocery", rendered.text) + assertEquals(MessageFormat.MARKDOWN, rendered.format) + } + + @Test + fun `append mode parsing matches the agent sdk values`() { + assertEquals(DisplayAppendMode.INLINE, parseDisplayAppendMode("inline")) + assertEquals(DisplayAppendMode.TEMPORARY, parseDisplayAppendMode("temporary")) + assertEquals(DisplayAppendMode.STEP, parseDisplayAppendMode("step")) + assertEquals(DisplayAppendMode.BLOCK, parseDisplayAppendMode("block")) + assertEquals(DisplayAppendMode.BLOCK, parseDisplayAppendMode(null)) + assertEquals(DisplayAppendMode.BLOCK, parseDisplayAppendMode("")) + } + + @Test + fun `toast and inline kinds stay out of the bubble`() { + assertTrue(isEphemeralAgentMessageKind("toast")) + assertTrue(isEphemeralAgentMessageKind("inline")) + assertEquals(false, isEphemeralAgentMessageKind(null)) + assertEquals(false, isEphemeralAgentMessageKind("notification")) + } + + @Test + fun `routing note keeps its info kind alongside the answer`() { + val thread = AgentDisplayThread() + + thread.setMessage(info("\u21aa routed to list \u2014 recent topic"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Created list: to-do"), DisplayAppendMode.BLOCK) + + val segments = thread.render().segments + assertEquals(2, segments.size) + assertEquals(MessageKind.INFO, segments[0].kind) + assertEquals("\u21aa routed to list \u2014 recent topic", segments[0].text) + assertEquals(MessageKind.NONE, segments[1].kind) + assertEquals("Created list: to-do", segments[1].text) + } + + @Test + fun `inline appends do not merge across different kinds`() { + val thread = AgentDisplayThread() + + thread.setMessage(info("note"), DisplayAppendMode.INLINE) + thread.setMessage(text(" answer"), DisplayAppendMode.INLINE) + + val segments = thread.render().segments + assertEquals(2, segments.size) + assertEquals(MessageKind.INFO, segments[0].kind) + assertEquals(MessageKind.NONE, segments[1].kind) + } + + @Test + fun `display message kind parsing`() { + assertEquals(MessageKind.INFO, MessageKind.parse("info")) + assertEquals(MessageKind.STATUS, MessageKind.parse("status")) + assertEquals(MessageKind.WARNING, MessageKind.parse("warning")) + assertEquals(MessageKind.ERROR, MessageKind.parse("error")) + assertEquals(MessageKind.SUCCESS, MessageKind.parse("success")) + assertEquals(MessageKind.NONE, MessageKind.parse(null)) + assertEquals(MessageKind.NONE, MessageKind.parse("")) + } +} diff --git a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/DisplayContentParserTest.kt b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/DisplayContentParserTest.kt index 0c8599a610..a8dc2efb8a 100644 --- a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/DisplayContentParserTest.kt +++ b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/DisplayContentParserTest.kt @@ -118,4 +118,46 @@ class DisplayContentParserTest { display.text ) } + + @Test + fun `carries the display message kind`() { + val display = extractAgentMessageContent( + JSONObject() + .put( + "message", + JSONObject() + .put("type", "text") + .put("content", "\u21aa routed to list \u2014 recent topic") + .put("kind", "info") + ) + ) + + assertEquals(MessageKind.INFO, display.kind) + assertEquals("\u21aa routed to list \u2014 recent topic", display.text) + } + + @Test + fun `defaults to no kind when absent`() { + val display = extractAgentMessageContent( + JSONObject() + .put( + "message", + JSONObject() + .put("type", "text") + .put("content", "Created list: to-do") + ) + ) + + assertEquals(MessageKind.NONE, display.kind) + } + + @Test + fun `reads the agent message kind for ephemeral routing`() { + val agentMessage = JSONObject() + .put("kind", "toast") + .put("message", "hi") + + assertEquals("toast", extractAgentMessageKind(agentMessage)) + assertEquals(null, extractAgentMessageKind(JSONObject().put("message", "hi"))) + } } diff --git a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt new file mode 100644 index 0000000000..e60681a12c --- /dev/null +++ b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt @@ -0,0 +1,88 @@ +package com.example.typeagentchat + +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class TimerActionParserTest { + + private fun payload(duration: Any, request: String = "Set a timer for 30 seconds") = + JSONObject() + .put("originalRequest", request) + .put("durationInSeconds", duration) + + @Test + fun `parses set-timer payload`() { + val timer = parseSetTimerActionPayload(payload(30)) + + requireNotNull(timer) + assertEquals("Set a timer for 30 seconds", timer.originalRequest) + assertEquals(30, timer.durationInSeconds) + } + + @Test + fun `floors fractional durations`() { + val timer = parseSetTimerActionPayload(payload(30.9)) + + requireNotNull(timer) + assertEquals(30, timer.durationInSeconds) + } + + @Test + fun `accepts a numeric string duration`() { + val timer = parseSetTimerActionPayload(payload("300")) + + requireNotNull(timer) + assertEquals(300, timer.durationInSeconds) + } + + @Test + fun `rejects zero and negative durations`() { + assertNull(parseSetTimerActionPayload(payload(0))) + assertNull(parseSetTimerActionPayload(payload(-5))) + // Floors to 0 rather than rounding up to 1. + assertNull(parseSetTimerActionPayload(payload(0.4))) + } + + @Test + fun `rejects durations beyond the AlarmClock 24 hour limit`() { + assertEquals(86_400, parseSetTimerActionPayload(payload(86_400))?.durationInSeconds) + assertNull(parseSetTimerActionPayload(payload(86_401))) + } + + @Test + fun `rejects missing or non-numeric durations`() { + assertNull( + parseSetTimerActionPayload(JSONObject().put("originalRequest", "Set a timer")) + ) + assertNull(parseSetTimerActionPayload(payload("half an hour"))) + // org.json forbids NaN/Infinity as JSON numbers, but String.toDoubleOrNull + // happily parses these spellings, so the guard is reachable via a string. + assertNull(parseSetTimerActionPayload(payload("NaN"))) + assertNull(parseSetTimerActionPayload(payload("Infinity"))) + } + + @Test + fun `rejects a non-object payload`() { + assertNull(parseSetTimerActionPayload(null)) + assertNull(parseSetTimerActionPayload("30")) + } + + @Test + fun `tolerates a missing originalRequest`() { + val timer = parseSetTimerActionPayload(JSONObject().put("durationInSeconds", 60)) + + requireNotNull(timer) + assertEquals("", timer.originalRequest) + assertEquals(60, timer.durationInSeconds) + } + + @Test + fun `formats durations for the confirmation toast`() { + assertEquals("30 s", formatTimerDuration(30)) + assertEquals("5 min", formatTimerDuration(300)) + assertEquals("1 h 5 min 30 s", formatTimerDuration(3930)) + assertEquals("24 h", formatTimerDuration(86_400)) + } +} From a0b89f41b7e486da6a0374b004a3efaa904d075c Mon Sep 17 00:00:00 2001 From: Jebran Syed Date: Fri, 31 Jul 2026 17:18:56 -0700 Subject: [PATCH 2/2] Address review findings in the mobile-2 sample Functional / security fixes from a pre-merge review of the display and set-timer work: - launchClockIntent claimed success unconditionally. Android 10+ refuses background activity starts *silently* - it throws neither ActivityNotFoundException nor SecurityException - so a set-timer or set-alarm request arriving while the activity was stopped showed a "Timer set" toast with no timer. Because EXTRA_SKIP_UI hides the clock app's own confirmation, that toast is the user's only feedback. Now the intent target and the lifecycle state are both checked up front and the failure is reported honestly. - originalRequest was an unbounded network-controlled string copied into AlarmClock.EXTRA_MESSAGE. A large enough value pushes the startActivity binder transaction past its ~1 MB limit and raises TransactionTooLargeException, a RuntimeException that is not caught - a remote input crashing the process. Cap it at 256 characters. - Read originalRequest with opt(), not optString(). Android's org.json renders a JSON null as the literal string "null", so {"originalRequest": null} labelled the timer "null" on device. The unit tests use JSON-java, which returns "" instead, so this never showed up in test. - AgentDisplayThread.setMessage flushed the temporary tail before checking for empty content. DisplayContentParser deliberately renders suppressed reasoning traces as empty text, so such an update emptied the thread; WebSocketManager then removed the bubble and re-appended it at the end of the transcript on the next update, losing its chronological position. Empty content is now a no-op for every mode except REPLACE. - The raw-text fallback for unparseable frames accumulated into the single shared no-requestId thread, which nothing ever completes, so the bubble grew without bound and showed "Responding..." forever. Emit it as a sealed step instead. - Finalize, rather than discard, open display threads on socket close, failure and disconnect, so a request in flight when the connection drops does not strand a bubble in the "Responding..." state. Tests: 56 unit tests (up from 44), covering empty block/replace content, step after temporary, inline after a temporary flush, newline-only content, mixed markdown formats, JSON-null and oversized originalRequest, and Long durations. assembleDebug and lintDebug clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- android/samples/mobile-2/README.md | 4 + .../typeagentchat/AgentDisplayThread.kt | 10 ++- .../com/example/typeagentchat/MainActivity.kt | 41 +++++++++- .../typeagentchat/TimerActionParser.kt | 16 +++- .../example/typeagentchat/WebSocketManager.kt | 27 ++++++- .../typeagentchat/AgentDisplayThreadTest.kt | 76 +++++++++++++++++++ .../typeagentchat/TimerActionParserTest.kt | 48 ++++++++++++ 7 files changed, 213 insertions(+), 9 deletions(-) diff --git a/android/samples/mobile-2/README.md b/android/samples/mobile-2/README.md index dda6f2faee..27464b84af 100644 --- a/android/samples/mobile-2/README.md +++ b/android/samples/mobile-2/README.md @@ -27,6 +27,10 @@ servers that emit actions this sample does not support. | `set-alarm` | `AlarmClock.ACTION_SET_ALARM` | Opens the clock app so the user can confirm the alarm. | | `set-timer` | `AlarmClock.ACTION_SET_TIMER` | Starts the countdown in the background (`EXTRA_SKIP_UI = true`) and confirms with a toast, so a chat request never yanks the user out of the conversation. Durations outside the documented 1..86400 second range are rejected rather than clamped. | +Both actions require the app to be in the foreground: Android 10+ silently refuses +background activity starts (no exception is thrown), so the app checks its own +lifecycle state first and reports a failure rather than a false confirmation. + Both require the `com.android.alarm.permission.SET_ALARM` permission (declared in the manifest, install-time only) and matching `` entries so `resolveActivity` works under Android 11+ package visibility rules. diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt index e1c02d0faa..18b4a93a4d 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/AgentDisplayThread.kt @@ -72,9 +72,17 @@ internal class AgentDisplayThread { get() = chunks.isEmpty() fun setMessage(content: ParsedDisplayContent, mode: DisplayAppendMode) { + val text = content.text + // An empty update is a no-op for every mode but REPLACE. Suppressed + // content (e.g. a reasoning trace stripped by DisplayContentParser) + // must not flush the temporary tail, because emptying the thread makes + // the caller drop the bubble and re-create it at the end of the + // transcript, losing its chronological position. + if (text.isEmpty() && mode != DisplayAppendMode.REPLACE) { + return + } flushTemporary() - val text = content.text when (mode) { DisplayAppendMode.REPLACE -> { chunks.clear() diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt index 4e6a5abf71..408a79667b 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/MainActivity.kt @@ -66,6 +66,7 @@ import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.ui.unit.dp import androidx.core.content.ContextCompat +import androidx.lifecycle.Lifecycle import com.example.typeagentchat.ui.theme.TypeAgentChatTheme class MainActivity : ComponentActivity() { @@ -123,7 +124,8 @@ class MainActivity : ComponentActivity() { detail = "hour=${action.hour} minute=${action.minute}", successMessage = "Alarm set for %02d:%02d".format(action.hour, action.minute), missingAppMessage = "No alarm app is available on this device.", - deniedMessage = "This app is not allowed to set alarms." + deniedMessage = "This app is not allowed to set alarms.", + backgroundMessage = "Could not set the alarm while the app was in the background." ) } @@ -135,7 +137,8 @@ class MainActivity : ComponentActivity() { * implementation in TypeAgent PR #2780 (`JavaScriptInterface.setTimer`) * passes false; we diverge deliberately so a voice/chat request never * yanks the user out of the conversation. The confirmation toast is then - * the only in-app feedback, so it is not optional. + * the only in-app feedback, so it is not optional - and it must not claim + * success when the launch was refused. See [launchClockIntent]. */ private fun launchSetTimerIntent(action: SetTimerAction) { val intent = Intent(AlarmClock.ACTION_SET_TIMER).apply { @@ -151,23 +154,53 @@ class MainActivity : ComponentActivity() { detail = "durationInSeconds=${action.durationInSeconds}", successMessage = "Timer set for ${formatTimerDuration(action.durationInSeconds)}", missingAppMessage = "No timer app is available on this device.", - deniedMessage = "This app is not allowed to set timers." + deniedMessage = "This app is not allowed to set timers.", + backgroundMessage = "Could not set the timer while the app was in the background." ) } + /** + * Starts a clock intent and reports the outcome truthfully. + * + * Two failure modes are checked up front because neither raises an + * exception: + * - no matching activity, which `startActivity` only surfaces as + * `ActivityNotFoundException` for implicit intents that resolve to + * nothing at dispatch time; + * - a background activity start, which Android 10+ refuses *silently* - + * no `ActivityNotFoundException`, no `SecurityException`, just a logcat + * warning. Without this guard the success toast would fire while no + * alarm or timer was created, and with `EXTRA_SKIP_UI` that toast is the + * user's only feedback. + */ private fun launchClockIntent( intent: Intent, actionName: String, detail: String, successMessage: String, missingAppMessage: String, - deniedMessage: String + deniedMessage: String, + backgroundMessage: String ) { val target = intent.resolveActivity(packageManager) Log.d( TAG, "Launching $actionName intent $detail target=${target?.flattenToShortString() ?: "none"}" ) + if (target == null) { + Log.e(TAG, "No app available to handle $actionName intent") + Toast.makeText(this, missingAppMessage, Toast.LENGTH_SHORT).show() + return + } + if (!lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) { + Log.w( + TAG, + "Skipping $actionName intent: activity is ${lifecycle.currentState}, " + + "background activity starts are refused without an exception" + ) + Toast.makeText(this, backgroundMessage, Toast.LENGTH_LONG).show() + return + } try { startActivity(intent) Log.d(TAG, "$actionName intent dispatched to clock app") diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt index cb6ec402f9..b4b3cedffb 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/TimerActionParser.kt @@ -16,6 +16,15 @@ internal data class SetTimerAction( */ private const val MAX_TIMER_SECONDS = 86_400L +/** + * `originalRequest` is echoed into `AlarmClock.EXTRA_MESSAGE`. Intent extras + * travel through a binder transaction with a ~1 MB budget, and an oversized + * extra makes `startActivity` throw `TransactionTooLargeException` - a + * `RuntimeException` no caller expects. Cap the label so a hostile or buggy + * server cannot crash the app from the network. + */ +private const val MAX_ORIGINAL_REQUEST_CHARS = 256 + /** * Parses the payload of `takeAction("set-timer", ...)` emitted by the * androidMobile agent's `SetTimerAction` (TypeAgent PR #2780): @@ -30,7 +39,12 @@ private const val MAX_TIMER_SECONDS = 86_400L */ internal fun parseSetTimerActionPayload(data: Any?): SetTimerAction? { val payload = data as? JSONObject ?: return null - val originalRequest = payload.optString("originalRequest").trim() + // Not `optString`: Android's org.json renders a JSON null as the literal + // string "null", which would end up as the timer label. + val originalRequest = (payload.opt("originalRequest") as? String) + .orEmpty() + .trim() + .take(MAX_ORIGINAL_REQUEST_CHARS) val duration = readDurationSeconds(payload) ?: return null if (duration <= 0L || duration > MAX_TIMER_SECONDS) { return null diff --git a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt index e310864ff9..0222e47cac 100644 --- a/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt +++ b/android/samples/mobile-2/app/src/main/java/com/example/typeagentchat/WebSocketManager.kt @@ -112,6 +112,7 @@ class WebSocketManager { pendingUserInteraction = null conversationId = null connectionId = null + finalizeOpenDisplayThreads() } _pendingYesNoPrompt.value = null _connectionStatus.value = ConnectionStatus( @@ -131,6 +132,7 @@ class WebSocketManager { failPendingInvokes(errorMessage) synchronized(lock) { pendingUserInteraction = null + finalizeOpenDisplayThreads() } _pendingYesNoPrompt.value = null _connectionStatus.value = ConnectionStatus( @@ -211,8 +213,7 @@ class WebSocketManager { webSocket = null synchronized(lock) { pendingUserInteraction = null - displayThreads.clear() - displayMessageIds.clear() + finalizeOpenDisplayThreads() } _pendingYesNoPrompt.value = null failPendingInvokes("App closed") @@ -337,7 +338,11 @@ class WebSocketManager { applyDisplay( requestId = null, content = ParsedDisplayContent(text = text, format = MessageFormat.TEXT), - mode = DisplayAppendMode.BLOCK + // STEP seals the bubble immediately. Without it every + // unparseable frame in the session would accumulate into the + // same no-requestId bubble, which would never be finalized and + // would show "Responding..." forever. + mode = DisplayAppendMode.STEP ) } } @@ -748,6 +753,22 @@ class WebSocketManager { } } + /** + * Seals every bubble that still has an open display thread. Used when the + * socket goes away mid-request so a bubble is not stranded showing + * "Responding..." forever, and so the per-request state is not retained + * across a reconnect. + */ + private fun finalizeOpenDisplayThreads() { + for (key in displayThreads.keys.toList()) { + val thread = displayThreads[key] ?: continue + val requestId = if (key == DEFAULT_THREAD_KEY) null else key + commitThread(key, requestId, thread) + } + displayThreads.clear() + displayMessageIds.clear() + } + private fun finalizeAssistantMessage(requestId: String?) { synchronized(lock) { val key = threadKey(requestId) diff --git a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt index fd48b690e6..65b0cd097b 100644 --- a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt +++ b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/AgentDisplayThreadTest.kt @@ -155,6 +155,82 @@ class AgentDisplayThreadTest { assertEquals(MessageKind.NONE, segments[1].kind) } + @Test + fun `empty block content leaves an existing bubble intact`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + // A suppressed reasoning trace parses to empty text. It must neither + // flush the temporary tail nor empty the thread, because an empty + // thread makes WebSocketManager drop and re-append the bubble. + thread.setMessage(text(""), DisplayAppendMode.BLOCK) + + assertEquals("Answer\n\nThinking...", thread.render().text) + assertEquals(false, thread.render().isEmpty) + } + + @Test + fun `empty replace clears the thread`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + thread.setMessage(text(""), DisplayAppendMode.REPLACE) + + assertTrue(thread.render().isEmpty) + } + + @Test + fun `step content survives the temporary flush`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("Phase 1 done"), DisplayAppendMode.STEP) + + assertEquals("Phase 1 done", thread.render().text) + } + + @Test + fun `inline after a temporary flush starts a new chunk`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Hello"), DisplayAppendMode.INLINE) + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + thread.setMessage(text(" world"), DisplayAppendMode.INLINE) + + assertEquals("Hello\n\n world", thread.render().text) + } + + @Test + fun `replace after a temporary flush keeps only the new content`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("Answer"), DisplayAppendMode.BLOCK) + thread.setMessage(text("Thinking..."), DisplayAppendMode.TEMPORARY) + thread.setMessage(text("Final"), DisplayAppendMode.REPLACE) + + assertEquals("Final", thread.render().text) + } + + @Test + fun `newline only content renders nothing`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("\n\n"), DisplayAppendMode.BLOCK) + + assertTrue(thread.render().isEmpty) + } + + @Test + fun `one markdown segment makes the whole message markdown`() { + val thread = AgentDisplayThread() + + thread.setMessage(text("plain"), DisplayAppendMode.BLOCK) + thread.setMessage(markdown("**bold**"), DisplayAppendMode.BLOCK) + + assertEquals(MessageFormat.MARKDOWN, thread.render().format) + } + @Test fun `display message kind parsing`() { assertEquals(MessageKind.INFO, MessageKind.parse("info")) diff --git a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt index e60681a12c..cbbac73b80 100644 --- a/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt +++ b/android/samples/mobile-2/app/src/test/java/com/example/typeagentchat/TimerActionParserTest.kt @@ -78,11 +78,59 @@ class TimerActionParserTest { assertEquals(60, timer.durationInSeconds) } + @Test + fun `treats a JSON null originalRequest as absent`() { + val timer = parseSetTimerActionPayload( + JSONObject() + .put("originalRequest", JSONObject.NULL) + .put("durationInSeconds", 45) + ) + + requireNotNull(timer) + assertEquals("", timer.originalRequest) + } + + @Test + fun `rejects a JSON null duration`() { + assertNull( + parseSetTimerActionPayload( + JSONObject() + .put("originalRequest", "Set a timer") + .put("durationInSeconds", JSONObject.NULL) + ) + ) + } + + @Test + fun `accepts a long duration value`() { + val timer = parseSetTimerActionPayload(payload(120L)) + + requireNotNull(timer) + assertEquals(120, timer.durationInSeconds) + } + + @Test + fun `caps an oversized originalRequest`() { + val timer = parseSetTimerActionPayload(payload(60, "a".repeat(10_000))) + + requireNotNull(timer) + assertEquals(256, timer.originalRequest.length) + } + + @Test + fun `blank originalRequest normalizes to empty`() { + val timer = parseSetTimerActionPayload(payload(60, " ")) + + requireNotNull(timer) + assertEquals("", timer.originalRequest) + } + @Test fun `formats durations for the confirmation toast`() { assertEquals("30 s", formatTimerDuration(30)) assertEquals("5 min", formatTimerDuration(300)) assertEquals("1 h 5 min 30 s", formatTimerDuration(3930)) assertEquals("24 h", formatTimerDuration(86_400)) + assertEquals("0 s", formatTimerDuration(0)) } }