From 5401aef8eee14fade7d9b1a7376d485e4cd2e7b8 Mon Sep 17 00:00:00 2001 From: braindeadz Date: Sat, 14 Nov 2026 16:56:06 +0100 Subject: [PATCH] fix(mobile): let users rename sessions from the session list and stop inventing placeholder titles The session action menu offered view details, archive, export and delete but never rename, even though the rename path already existed: the conversation header dispatched RemoteSessionIntent.RenameSession, the shared store handled it, and a remote session's title was already sent to the desktop as update_session_title. The list, action sheet, popup and sidebar simply never drew a row for it, because SessionActionPolicy exposed no rename capability. Three separate English placeholders made unnamed sessions look named and made a rename look unapplied: the shared fallbackTitle returned "New Session", the shared RemoteResponseMapper invented "Session " from the id prefix, and the Android header fell back to "Remote session". A host that names no title sends none, so the shared layer now keeps the title blank and each surface renders its own localized "untitled" wording (sidebar_untitled on Android). Design: - SessionActionCapabilities gains an appended canRename flag rather than folding rename into archive/export: for a remote session rename is a round trip to the desktop, for a general chat it rewrites the local row, so it is neither of the local-storage operations those flags describe. The flag is appended so the existing four positional arguments keep their meaning. - The sheet, popup and sidebar take an optional onRename callback: a shell that cannot dispatch a rename renders no row instead of a row that does nothing. - The store keeps authority: the list keeps the old title until the desktop answers update_session_title. The editor enforces a 120-character client-side limit and refuses a blank or unchanged draft; the host normalizes the title and refuses an empty one. - A blank title is rendered per surface via sidebar_untitled rather than a fabricated English name. Compatibility: SessionActionCapabilities is a Kotlin data class consumed only by Kotlin code; the HarmonyOS client has its own parallel ETS SessionActionCapabilities and does not consume it, and the iOS client has no caller of it, so the appended field breaks neither. fallbackTitle's only Kotlin caller is the shared RemoteSessionStore; HarmonyOS uses its own ArkTS mapper. iOS surfaces that render the title raw will show a blank row where a fabricated "Session " used to be, while most iOS surfaces already localize empty titles. Tests: new shared unit tests cover the appended flag position and the blank-title mapping. SessionRenameActionTest (10 instrumented tests) was executed on a real Android device against the original revision of this branch and passed 10/10 (author-reported, run outside this environment). The amended revision adds aSurfaceWithoutARenameCallbackDrawsNoRenameRow and removes the unused conversation_title_default string resource; that amended revision is compile-checked only (:app:compileDebugAndroidTestKotlin BUILD SUCCESSFUL) with no device run here, and makes no claim that the added test passed. Co-authored-by: Driss --- .../mobile/app/ConversationHeaderTest.kt | 5 +- .../mobile/app/SessionRenameActionTest.kt | 326 ++++++++++++++++++ .../mobile/app/ui/chat/ConversationHeader.kt | 7 +- .../mobile/app/ui/remote/PairingScreen.kt | 60 +++- .../app/ui/remote/RemoteSessionListView.kt | 8 + .../app/ui/remote/SessionActionSheet.kt | 216 +++++++++++- .../mobile/app/ui/shell/MobileScreen.kt | 1 + .../mobile/app/ui/shell/sidebar/AppSidebar.kt | 13 + .../app/src/main/res/values-zh/strings.xml | 1 - .../app/src/main/res/values/strings.xml | 2 - .../mobile/core/domain/SessionListPolicy.kt | 11 +- .../core/domain/SessionListPolicyTest.kt | 10 +- .../feature/session/RemoteSessionStore.kt | 6 +- .../feature/session/SessionActionPolicy.kt | 30 +- .../session/RemoteResponseMapperTest.kt | 20 +- .../session/SessionActionPolicyTest.kt | 63 +++- 16 files changed, 742 insertions(+), 37 deletions(-) create mode 100644 src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/SessionRenameActionTest.kt diff --git a/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/ConversationHeaderTest.kt b/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/ConversationHeaderTest.kt index 646a950c6b..85867ed143 100644 --- a/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/ConversationHeaderTest.kt +++ b/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/ConversationHeaderTest.kt @@ -49,7 +49,10 @@ class ConversationHeaderTest { ) } - composeRule.onNodeWithText(string(R.string.conversation_title_default)).assertIsDisplayed() + // A host that has not named the session yet sends no title at all, so the + // blank case is ordinary. It reads as the same "untitled" the session rows + // use. + composeRule.onNodeWithText(string(R.string.sidebar_untitled)).assertIsDisplayed() composeRule.onNodeWithText("OpenBitFun · main").assertIsDisplayed() } diff --git a/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/SessionRenameActionTest.kt b/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/SessionRenameActionTest.kt new file mode 100644 index 0000000000..c8b199cb77 --- /dev/null +++ b/src/apps/mobile/android/app/src/androidTest/kotlin/com/openbitfun/mobile/app/SessionRenameActionTest.kt @@ -0,0 +1,326 @@ +package com.openbitfun.mobile.app + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.size +import androidx.compose.runtime.mutableStateOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTextClearance +import androidx.compose.ui.test.performTextInput +import androidx.compose.ui.unit.IntRect +import androidx.compose.ui.unit.dp +import com.openbitfun.mobile.app.ui.remote.RemoteRecentRow +import com.openbitfun.mobile.app.ui.remote.SESSION_RENAME_FIELD_TEST_TAG +import com.openbitfun.mobile.app.ui.remote.SESSION_RENAME_ROW_TEST_TAG +import com.openbitfun.mobile.app.ui.remote.SessionActionPopup +import com.openbitfun.mobile.app.ui.remote.SessionActionSheet +import com.openbitfun.mobile.app.ui.theme.OpenBitFunTheme +import com.openbitfun.mobile.core.feature.session.RemoteSessionIntent +import com.openbitfun.mobile.core.feature.session.SessionActionPolicy +import com.openbitfun.mobile.core.feature.session.SessionActionScope +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test + +/** + * The rename step of the remote session menu, on an isolated fixture. + * + * The surface takes fields rather than a session — `RemoteSession` lives in + * `core-domain`, which app sources may not import — so this exercises the sheet + * and the popup directly and records what they would hand the store. Nothing + * here proves a live host round trip: the store and `update_session_title` are + * the other half, and only a real paired desktop answers them. + */ +class SessionRenameActionTest { + @get:Rule + val composeRule = createComposeRule() + + private val sessionId = "6f1c2d9a" + private val sessionTitle = "Fix the relay timeout" + + @Test + fun theRemoteMenuOffersRenameAndSeedsTheFieldWithTheCurrentTitle() { + val intents = mutableListOf() + setSheet(intents = intents) + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).assertIsDisplayed().performClick() + + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG) + .assertIsDisplayed() + .assertTextEquals(sessionTitle) + assertTrue(intents.isEmpty()) + } + + @Test + fun submittingSendsOneRenameForThisSessionAndDismisses() { + val intents = mutableListOf() + var dismissals = 0 + setSheet(intents = intents, onDismiss = { dismissals += 1 }) + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).performClick() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextClearance() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput("New title") + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).performClick() + + assertEquals( + listOf(RemoteSessionIntent.RenameSession(sessionId, "New title")), + intents, + ) + assertEquals(1, dismissals) + } + + @Test + fun theSubmittedTitleIsTheTrimmedDraft() { + val intents = mutableListOf() + setSheet(intents = intents) + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).performClick() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextClearance() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput(" Padded title ") + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).performClick() + + // The store trims too; this pins that the surface never hands it padding + // that would be stored and rendered as part of the title. + assertEquals( + listOf(RemoteSessionIntent.RenameSession(sessionId, "Padded title")), + intents, + ) + } + + @Test + fun aBlankHostTitleIsShownAsUntitledAndCanBeGivenAName() { + val intents = mutableListOf() + // A host that has not named the session yet sends no title at all. + setSheet(intents = intents, title = "") + + composeRule.onNodeWithText(testString(R.string.sidebar_untitled)).assertIsDisplayed() + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).assertIsDisplayed().performClick() + + // Seeded from the raw title, not from the placeholder: saving without + // typing must not rename the session to "Untitled session". + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).assertIsDisplayed() + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).assertIsNotEnabled() + + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput("Named on the phone") + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).performClick() + + assertEquals( + listOf(RemoteSessionIntent.RenameSession(sessionId, "Named on the phone")), + intents, + ) + } + + @Test + fun cancellingReturnsToTheActionListAndSendsNothing() { + val intents = mutableListOf() + setSheet(intents = intents) + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).performClick() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput(" typed") + composeRule.onNodeWithText(testString(R.string.common_cancel)).performClick() + + assertTrue(intents.isEmpty()) + // The step is gone and the rows it replaced are back. + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).assertDoesNotExist() + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).assertIsDisplayed() + } + + @Test + fun anUnchangedOrBlankTitleCannotBeSubmitted() { + val intents = mutableListOf() + setSheet(intents = intents) + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).performClick() + // Seeded with the title the row already carries, so there is nothing to send. + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).assertIsNotEnabled() + + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextClearance() + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).assertIsNotEnabled() + + // Something new is exactly what re-enables it. + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput("New title") + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).assertIsEnabled() + + assertTrue(intents.isEmpty()) + } + + @Test + fun aCommandStartingWhileTheEditorIsOpenCannotBeSubmitted() { + val intents = mutableListOf() + val capabilities = mutableStateOf( + SessionActionPolicy.resolve(SessionActionScope.REMOTE, "code", busy = false), + ) + composeRule.setContent { + OpenBitFunTheme(dark = false) { + SessionActionSheet( + title = sessionTitle, + status = "idle", + capabilities = capabilities.value, + onViewDetails = {}, + onRename = { intents += RemoteSessionIntent.RenameSession(sessionId, it) }, + onDelete = {}, + onDismiss = {}, + ) + } + } + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).performClick() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextClearance() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput("New title") + + // A delete or a refresh starts while the field is open: the row that + // opened this step is gone, and submitting now would cancel that command. + // Mutated inside runOnIdle rather than from the test thread, so the write + // and the recomposition it triggers are ordered by the rule's clock + // instead of racing it. + composeRule.runOnIdle { + capabilities.value = SessionActionPolicy.resolve( + SessionActionScope.REMOTE, + "code", + busy = true, + ) + } + + // Only the disabled state is asserted here. Clicking the node to prove the + // handler re-checks would depend on a disabled clickable still publishing + // an onClick action, which is a Compose implementation detail: the day it + // changes, this test would fail for a reason that has nothing to do with + // renaming. + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)) + .assertIsNotEnabled() + + assertTrue(intents.isEmpty()) + } + + @Test + fun aCommandInFlightOffersNoRename() { + val intents = mutableListOf() + setSheet(intents = intents, busy = true) + + composeRule.onAllNodesWithTag(SESSION_RENAME_ROW_TEST_TAG).assertCountEquals(0) + } + + @Test + fun aSurfaceWithoutARenameCallbackDrawsNoRenameRow() { + // A shell that cannot dispatch a rename passes no callback; the row is + // capability-gated *and* callback-gated, so it must not be drawn at all. + composeRule.setContent { + OpenBitFunTheme(dark = false) { + SessionActionSheet( + title = sessionTitle, + status = "idle", + capabilities = SessionActionPolicy.resolve( + SessionActionScope.REMOTE, + "code", + busy = false, + ), + onViewDetails = {}, + onRename = null, + onDelete = {}, + onDismiss = {}, + ) + } + } + + composeRule.onAllNodesWithTag(SESSION_RENAME_ROW_TEST_TAG).assertCountEquals(0) + } + + @Test + fun theWideSidebarPopupOffersTheSameStep() { + val intents = mutableListOf() + composeRule.setContent { + OpenBitFunTheme(dark = false) { + Box(modifier = Modifier.size(320.dp)) { + SessionActionPopup( + // Anchored inside the window rather than at its origin: the + // popup is placed relative to this rect, and an anchor at + // (0, 0) puts the menu under the status bar on a real phone, + // where `assertIsDisplayed` would then be arguing with the + // system bars instead of with the rename row. + anchorBounds = IntRect(24, 240, 144, 280), + title = sessionTitle, + status = "idle", + capabilities = SessionActionPolicy.resolve( + SessionActionScope.REMOTE, + "code", + busy = false, + ), + onViewDetails = {}, + onRename = { intents += RemoteSessionIntent.RenameSession(sessionId, it) }, + onDelete = {}, + onDismiss = {}, + ) + } + } + } + + composeRule.onNodeWithTag(SESSION_RENAME_ROW_TEST_TAG).assertIsDisplayed().performClick() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG) + .assertIsDisplayed() + .assertTextEquals(sessionTitle) + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextClearance() + composeRule.onNodeWithTag(SESSION_RENAME_FIELD_TEST_TAG).performTextInput("Renamed in the rail") + composeRule.onNodeWithText(testString(R.string.session_rename_confirm)).performClick() + + assertEquals( + listOf(RemoteSessionIntent.RenameSession(sessionId, "Renamed in the rail")), + intents, + ) + } + + @Test + fun theHomeRecentRowShowsTheUntitledLabelForABlankHostTitle() { + composeRule.setContent { + OpenBitFunTheme(dark = false) { + Column { + RemoteRecentRow(title = "", subtitle = "Studio · main", onOpen = {}) + RemoteRecentRow(title = sessionTitle, subtitle = "Studio · main", onOpen = {}) + } + } + } + + // The home screen is the first surface the user sees: a host that has not + // named the session sends no title, and that must read as the same + // localized "untitled" the list row, rail and sheets show rather than as + // an empty line. + composeRule.onNodeWithText(testString(R.string.sidebar_untitled)).assertIsDisplayed() + // A real host title is shown verbatim, not replaced by the label. + composeRule.onNodeWithText(sessionTitle).assertIsDisplayed() + } + + private fun setSheet( + intents: MutableList, + busy: Boolean = false, + title: String = sessionTitle, + onDismiss: () -> Unit = {}, + ) { + composeRule.setContent { + OpenBitFunTheme(dark = false) { + SessionActionSheet( + title = title, + status = "idle", + capabilities = SessionActionPolicy.resolve( + SessionActionScope.REMOTE, + "code", + busy, + ), + onViewDetails = {}, + onRename = { intents += RemoteSessionIntent.RenameSession(sessionId, it) }, + onDelete = {}, + onDismiss = onDismiss, + ) + } + } + } +} diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/chat/ConversationHeader.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/chat/ConversationHeader.kt index 975595e791..a9a0bbefa5 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/chat/ConversationHeader.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/chat/ConversationHeader.kt @@ -113,7 +113,12 @@ internal fun ConversationHeader( verticalArrangement = Arrangement.spacedBy(3.dp), ) { Text( - title.ifBlank { stringResource(R.string.conversation_title_default) }, + // A host that has not named a session yet sends no title at + // all (`non_empty_title` on the host side), so the blank case + // is ordinary rather than exceptional. It reads as the same + // "untitled" the session rows use, not as a second, different + // label for the same missing thing. + title.ifBlank { stringResource(R.string.sidebar_untitled) }, style = if (hasSubtitle) MobileDesignTypography.ConversationHeaderTitle else MobileDesignTypography.TitleMedium, color = MaterialTheme.colorScheme.onSurface, diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/PairingScreen.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/PairingScreen.kt index 940d0ea200..db374fb240 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/PairingScreen.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/PairingScreen.kt @@ -272,15 +272,14 @@ internal fun RemoteCompactHome( ) { Text(stringResource(R.string.home_recent_all), fontSize = 12.sp) } } recent.forEach { session -> - Column(Modifier.fillMaxWidth().clickable { onOpen(session.id) } - .padding(vertical = MobileDesignGeometry.RecentHomeRowPadding)) { - Text(session.title, fontSize = 15.sp, maxLines = 2) - val workspace = session.workspaceName?.takeIf { it.isNotBlank() } - ?: session.workspacePath.orEmpty().trimEnd('/').substringAfterLast('/') - Text(listOf(desktopName, workspace).filter { it.isNotBlank() }.joinToString(" · "), - fontSize = 11.sp, color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 6.dp), maxLines = 1) - } + val workspace = session.workspaceName?.takeIf { it.isNotBlank() } + ?: session.workspacePath.orEmpty().trimEnd('/').substringAfterLast('/') + RemoteRecentRow( + title = session.title, + subtitle = listOf(desktopName, workspace).filter { it.isNotBlank() } + .joinToString(" · "), + onOpen = { onOpen(session.id) }, + ) HorizontalDivider(color = MaterialTheme.colorScheme.outlineVariant) } if (recent.isEmpty() && remoteState !is RemoteSessionUiState.Loading) { @@ -292,6 +291,49 @@ internal fun RemoteCompactHome( } } +/** + * One row of the phone home screen's recent list, ported from the desktop's + * recent-session row. + * + * Split out of [RemoteCompactHome] so the row can be driven from a plain title + * in a test: `RemoteSession` lives in `core-domain`, which app sources may not + * reference (AGENTS.md, the seam rule), so a fixture `Ready` state cannot be + * built here. The contract is the one every other session surface keeps — a real + * host title verbatim, a blank one as the shared "untitled" label — and + * [RemoteCompactHome] passes `session.title` through untouched. + */ +@Composable +internal fun RemoteRecentRow( + title: String, + subtitle: String, + onOpen: () -> Unit, + modifier: Modifier = Modifier, +) { + Column( + modifier.fillMaxWidth().clickable(onClick = onOpen) + .padding(vertical = MobileDesignGeometry.RecentHomeRowPadding), + ) { + Text( + // A host that has not named the session yet sends no title at all + // (`non_empty_title` on the host side). Rendering the blank string + // here would put an empty line on the first screen the user sees — + // the fabricated "Session " this replaced was hiding exactly + // that, and the list row, rail, header and sheets already show the + // localized label instead. + title.ifBlank { stringResource(R.string.sidebar_untitled) }, + fontSize = 15.sp, + maxLines = 2, + ) + Text( + subtitle, + fontSize = 11.sp, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 6.dp), + maxLines = 1, + ) + } +} + @Composable private fun RemoteShellHeader( onOpenSidebar: (() -> Unit)?, diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/RemoteSessionListView.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/RemoteSessionListView.kt index bc63f6d45e..d7ad006273 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/RemoteSessionListView.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/RemoteSessionListView.kt @@ -348,11 +348,18 @@ internal fun RemoteSessionListContent( } val openDetails = { detailsFor = session.id } val delete = { onIntent(RemoteSessionIntent.DeleteSession(session.id)) } + // The store owns the authority update: this only names the + // new title, and the row keeps the old one until the desktop + // answers. + val rename = { newTitle: String -> + onIntent(RemoteSessionIntent.RenameSession(session.id, newTitle)) + } if (compact || actionAnchor == null) SessionActionSheet( title = session.title, status = session.status, capabilities = capabilities, onViewDetails = openDetails, + onRename = rename, onDelete = delete, onDismiss = dismissActions, ) else SessionActionPopup( @@ -361,6 +368,7 @@ internal fun RemoteSessionListContent( status = session.status, capabilities = capabilities, onViewDetails = openDetails, + onRename = rename, onDelete = delete, onDismiss = dismissActions, ) diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/SessionActionSheet.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/SessionActionSheet.kt index c1bae373b9..d955279e97 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/SessionActionSheet.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/remote/SessionActionSheet.kt @@ -16,6 +16,7 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.BasicTextField import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.foundation.verticalScroll import androidx.compose.material3.ExperimentalMaterial3Api @@ -35,6 +36,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.draw.shadow +import androidx.compose.ui.graphics.SolidColor import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource @@ -62,6 +64,25 @@ import com.openbitfun.mobile.app.ui.theme.generated.MobileDesignGeometry internal const val SESSION_ACTIONS_TEST_TAG: String = "session-actions" internal const val SESSION_DETAILS_TEST_TAG: String = "session-details" +/** The row that opens the rename step, and the field that step shows. */ +internal const val SESSION_RENAME_ROW_TEST_TAG: String = "session-rename-row" +internal const val SESSION_RENAME_FIELD_TEST_TAG: String = "session-rename-field" + +/** + * The longest title this surface will submit. + * + * The host normalizes a title and refuses only an empty one, so this is not a + * protocol limit: it is the client's own guard against pasting a document into + * a row that is drawn on one line. Nothing is truncated on the way out — a + * draft over the limit simply cannot be submitted. + * + * A title the host already holds that is longer than this therefore shows Save + * disabled with no explanation of its own. `ui/chat/ConversationHeader.TitleEditor` + * caps nothing, so the two editors differ here; that inconsistency is known and + * accepted rather than fixed by widening this one. + */ +private const val SESSION_TITLE_MAX_LENGTH: Int = 120 + /** * A session row's overflow menu, ported from * `pages/components/SessionActionSurface.ets`. @@ -76,7 +97,7 @@ internal const val SESSION_DETAILS_TEST_TAG: String = "session-details" * top of it, as the source does. Two layers of scrim over a bottom sheet gives * the confirmation two dismiss gestures with different meanings — tapping * outside would cancel the delete or the whole menu depending on how far the - * finger landed. + * finger landed. Renaming is the same kind of step for the same reason. * * Takes the fields rather than the session: `RemoteSession` lives in * `core-domain`, which the app is not allowed to import (design doc §5 rule 3), @@ -90,6 +111,7 @@ internal fun SessionActionSheet( status: String, capabilities: SessionActionCapabilities, onViewDetails: () -> Unit, + onRename: ((String) -> Unit)? = null, onArchive: (() -> Unit)? = null, onExport: (() -> Unit)? = null, onDelete: () -> Unit, @@ -99,11 +121,15 @@ internal fun SessionActionSheet( // sheet that is itself gone after a process death, and restoring "about to // delete" without the tap that got there is not a state worth rebuilding. var confirmingDelete by remember { mutableStateOf(false) } + var renaming by remember { mutableStateOf(false) } + var renameDraft by remember { mutableStateOf("") } // Archive and export are local-storage operations a remote session can never // offer, so those callbacks are optional: a surface without them renders no - // row instead of rendering one that does nothing. + // row instead of rendering one that does nothing. Rename is optional for the + // same reason — the row is capability-gated *and* callback-gated. val showArchive = capabilities.canArchive && onArchive != null val showExport = capabilities.canExport && onExport != null + val showRename = capabilities.canRename && onRename != null ModalBottomSheet( onDismissRequest = onDismiss, @@ -179,6 +205,9 @@ internal fun SessionActionSheet( HorizontalDivider(modifier = Modifier.padding(top = 6.dp, bottom = 8.dp)) + // Pre-existing and out of scope here: this branch reads no capability + // either, so an already-open confirmation can still submit after + // `busy` flips. Not audited and cleared — see the rename step below. if (confirmingDelete) { Text( stringResource(R.string.session_delete_confirm), @@ -212,12 +241,49 @@ internal fun SessionActionSheet( return@Column } + if (renaming) { + RenameStep( + draft = renameDraft, + // The capability is re-read here rather than only at the row + // that opened this step: a command starting while the field + // is open must not let it submit, because the store's + // `beginWork()` cancels whatever is already in flight. + submitEnabled = renameSubmittable(renameDraft, title) && capabilities.canRename, + onDraftChange = { renameDraft = it }, + onSave = { + // Gated twice on purpose: the button is disabled, and the + // verdict is re-decided here. Which semantics a disabled + // clickable publishes is a Compose implementation detail, + // so the handler must not rely on the button being the only + // way in. + if (renameSubmittable(renameDraft, title) && capabilities.canRename) { + // The store trims and owns the authority update; this + // surface never retitles the row itself. + onRename?.invoke(renameDraft.trim()) + onDismiss() + } + }, + onCancel = { renaming = false }, + ) + return@Column + } + if (capabilities.canViewDetails) { ActionRow(R.drawable.ic_symbol_info_circle, stringResource(R.string.session_view_details)) { onViewDetails() onDismiss() } } + if (showRename) { + ActionRow( + R.drawable.ic_symbol_square_and_pencil, + stringResource(R.string.session_rename), + modifier = Modifier.testTag(SESSION_RENAME_ROW_TEST_TAG), + ) { + renameDraft = title + renaming = true + } + } if (showArchive) { val archived = status.equals("archived", ignoreCase = true) val label = stringResource( @@ -263,14 +329,18 @@ internal fun SessionActionPopup( status: String, capabilities: SessionActionCapabilities, onViewDetails: () -> Unit, + onRename: ((String) -> Unit)? = null, onArchive: (() -> Unit)? = null, onExport: (() -> Unit)? = null, onDelete: () -> Unit, onDismiss: () -> Unit, ) { var confirmingDelete by remember { mutableStateOf(false) } + var renaming by remember { mutableStateOf(false) } + var renameDraft by remember { mutableStateOf("") } val showArchive = capabilities.canArchive && onArchive != null val showExport = capabilities.canExport && onExport != null + val showRename = capabilities.canRename && onRename != null val targetBounds = anchorBounds val positionProvider = remember(targetBounds) { object : PopupPositionProvider { @@ -348,6 +418,9 @@ internal fun SessionActionPopup( } } HorizontalDivider(modifier = Modifier.padding(top = 6.dp, bottom = 8.dp)) + // Pre-existing and out of scope here: this branch reads no capability + // either, so an already-open confirmation can still submit after + // `busy` flips. Not audited and cleared — see the rename step below. if (confirmingDelete) { Text( stringResource(R.string.session_delete_confirm), @@ -376,12 +449,40 @@ internal fun SessionActionPopup( onDismiss() } } + } else if (renaming) { + RenameStep( + draft = renameDraft, + // Re-read here for the reason the sheet does: the row that + // opened this step can be withdrawn while it is open. + submitEnabled = renameSubmittable(renameDraft, title) && capabilities.canRename, + onDraftChange = { renameDraft = it }, + onSave = { + // Gated twice for the reason the sheet's step is: the + // verdict is re-decided in the handler rather than trusted + // to the button's disabled state alone. + if (renameSubmittable(renameDraft, title) && capabilities.canRename) { + onRename?.invoke(renameDraft.trim()) + onDismiss() + } + }, + onCancel = { renaming = false }, + ) } else { if (capabilities.canViewDetails) { ActionRow(R.drawable.ic_symbol_info_circle, stringResource(R.string.session_view_details)) { onViewDetails(); onDismiss() } } + if (showRename) { + ActionRow( + R.drawable.ic_symbol_square_and_pencil, + stringResource(R.string.session_rename), + modifier = Modifier.testTag(SESSION_RENAME_ROW_TEST_TAG), + ) { + renameDraft = title + renaming = true + } + } if (showArchive) { ActionRow( R.drawable.ic_symbol_archivebox, @@ -414,12 +515,109 @@ internal fun SessionActionPopup( } } +/** + * The rename step: a field seeded with the title the row carries now, and the + * two verdicts. + * + * A step inside the menu rather than a dialog over it, for the reason the + * delete confirmation is one: a second scrim gives one gesture two meanings. + */ +@Composable +private fun RenameStep( + draft: String, + submitEnabled: Boolean, + onDraftChange: (String) -> Unit, + onSave: () -> Unit, + onCancel: () -> Unit, +) { + val fieldShape = RoundedCornerShape(8.dp) + Column(modifier = Modifier.fillMaxWidth().padding(top = 4.dp)) { + Text( + stringResource(R.string.session_rename_label), + fontSize = 13.sp, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.fillMaxWidth(), + ) + BasicTextField( + value = draft, + onValueChange = onDraftChange, + singleLine = true, + textStyle = MaterialTheme.typography.bodyMedium.copy( + fontSize = 15.sp, + color = MaterialTheme.colorScheme.onSurface, + ), + cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), + decorationBox = { field -> + // A blank host title seeds an empty draft, which would otherwise + // be an empty box with no hint of what belongs in it. Same + // pattern, and the same label, as `ui/chat/ConversationHeader`. + Box(contentAlignment = Alignment.CenterStart) { + if (draft.isEmpty()) { + Text( + stringResource(R.string.session_rename_label), + fontSize = 15.sp, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + field() + } + }, + modifier = Modifier + .fillMaxWidth() + .padding(top = 8.dp) + .height(44.dp) + .clip(fieldShape) + .background(MaterialTheme.colorScheme.surfaceVariant) + .border(1.dp, MaterialTheme.colorScheme.outlineVariant, fieldShape) + .padding(horizontal = 12.dp) + .testTag(SESSION_RENAME_FIELD_TEST_TAG), + ) + Row( + modifier = Modifier.fillMaxWidth().padding(top = 12.dp), + horizontalArrangement = Arrangement.spacedBy(10.dp), + ) { + ConfirmationButton( + label = stringResource(R.string.common_cancel), + containerColor = MaterialTheme.colorScheme.surfaceVariant, + contentColor = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.weight(1f), + onClick = onCancel, + ) + ConfirmationButton( + label = stringResource(R.string.session_rename_confirm), + containerColor = if (submitEnabled) { + MaterialTheme.colorScheme.primary + } else { + MaterialTheme.colorScheme.surfaceVariant + }, + contentColor = if (submitEnabled) { + MaterialTheme.colorScheme.onPrimary + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + modifier = Modifier.weight(1f), + enabled = submitEnabled, + onClick = onSave, + ) + } + } +} + +/** A draft is worth sending when it says something new and fits the field. */ +private fun renameSubmittable(draft: String, currentTitle: String): Boolean { + val trimmed = draft.trim() + return trimmed.isNotEmpty() && + trimmed != currentTitle.trim() && + trimmed.length <= SESSION_TITLE_MAX_LENGTH +} + @Composable private fun ConfirmationButton( label: String, containerColor: androidx.compose.ui.graphics.Color, contentColor: androidx.compose.ui.graphics.Color, modifier: Modifier = Modifier, + enabled: Boolean = true, onClick: () -> Unit, ) { Box( @@ -427,7 +625,7 @@ private fun ConfirmationButton( .height(44.dp) .clip(RoundedCornerShape(8.dp)) .background(containerColor) - .clickable(onClick = onClick), + .clickable(enabled = enabled, onClick = onClick), contentAlignment = Alignment.Center, ) { Text( @@ -449,6 +647,7 @@ private fun ActionRow( @DrawableRes icon: Int, label: String, destructive: Boolean, + modifier: Modifier = Modifier, onClick: () -> Unit, ) { val tint = if (destructive) { @@ -457,7 +656,7 @@ private fun ActionRow( MaterialTheme.colorScheme.onSurfaceVariant } Row( - modifier = Modifier + modifier = modifier .fillMaxWidth() .clip(RoundedCornerShape(8.dp)) .clickable(onClick = onClick) @@ -485,8 +684,13 @@ private fun ActionRow( } @Composable -private fun ActionRow(@DrawableRes icon: Int, label: String, onClick: () -> Unit) { - ActionRow(icon, label, destructive = false, onClick = onClick) +private fun ActionRow( + @DrawableRes icon: Int, + label: String, + modifier: Modifier = Modifier, + onClick: () -> Unit, +) { + ActionRow(icon, label, destructive = false, modifier = modifier, onClick = onClick) } /** diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/MobileScreen.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/MobileScreen.kt index 95f7a7ca1b..416cec1928 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/MobileScreen.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/MobileScreen.kt @@ -421,6 +421,7 @@ internal fun MobileScreen(onAccountRestored: (Boolean) -> Unit = {}) { ) }, onDeleteRemoteSession = { id -> dispatchActiveSession(RemoteSessionIntent.DeleteSession(id)) }, + onRenameRemoteSession = { id, title -> dispatchActiveSession(RemoteSessionIntent.RenameSession(id, title)) }, onOpenSettings = { // The gear and the remote home header open the same page. shell.openSettings() diff --git a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/sidebar/AppSidebar.kt b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/sidebar/AppSidebar.kt index 252127deb6..db5f3be885 100644 --- a/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/sidebar/AppSidebar.kt +++ b/src/apps/mobile/android/app/src/main/kotlin/com/openbitfun/mobile/app/ui/shell/sidebar/AppSidebar.kt @@ -101,6 +101,13 @@ internal fun AppSidebar( onAddRemoteWorkspace: (() -> Unit)? = null, onWorkspaceTool: (String, String?, Boolean) -> Unit, onDeleteRemoteSession: (String) -> Unit, + /** + * Optional for the same reason the sheet's archive and export are: a shell + * that cannot dispatch a rename renders no rename row rather than a row that + * does nothing. The session id and the new title travel together because the + * store owns the authority update and needs both. + */ + onRenameRemoteSession: ((String, String) -> Unit)? = null, onOpenSettings: () -> Unit, onOpenAccount: () -> Unit, modifier: Modifier, @@ -203,6 +210,9 @@ internal fun AppSidebar( status = "", capabilities = capabilities, onViewDetails = { remoteDetailsSessionId = session.id }, + onRename = onRenameRemoteSession?.let { rename -> + { newTitle: String -> rename(session.id, newTitle) } + }, onDelete = { onDeleteRemoteSession(session.id) }, onDismiss = { remoteActionSession = null }, ) @@ -212,6 +222,9 @@ internal fun AppSidebar( status = "", capabilities = capabilities, onViewDetails = { remoteDetailsSessionId = session.id }, + onRename = onRenameRemoteSession?.let { rename -> + { newTitle: String -> rename(session.id, newTitle) } + }, onDelete = { onDeleteRemoteSession(session.id) }, onDismiss = { remoteActionSession = null }, ) diff --git a/src/apps/mobile/android/app/src/main/res/values-zh/strings.xml b/src/apps/mobile/android/app/src/main/res/values-zh/strings.xml index f381448984..82c775f9ea 100644 --- a/src/apps/mobile/android/app/src/main/res/values-zh/strings.xml +++ b/src/apps/mobile/android/app/src/main/res/values-zh/strings.xml @@ -334,7 +334,6 @@ 返回会话列表 会话设置 - 远程会话 还没有消息 可以就这个工作区向智能体提问。 正在加载对话… diff --git a/src/apps/mobile/android/app/src/main/res/values/strings.xml b/src/apps/mobile/android/app/src/main/res/values/strings.xml index 4456541efe..a4385149e8 100644 --- a/src/apps/mobile/android/app/src/main/res/values/strings.xml +++ b/src/apps/mobile/android/app/src/main/res/values/strings.xml @@ -348,8 +348,6 @@ Back to sessions Session settings - - Remote session No messages yet Ask the agent anything about this workspace. Loading conversation… diff --git a/src/apps/mobile/shared/core-domain/src/commonMain/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicy.kt b/src/apps/mobile/shared/core-domain/src/commonMain/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicy.kt index 1972f0f51d..28102f11a8 100644 --- a/src/apps/mobile/shared/core-domain/src/commonMain/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicy.kt +++ b/src/apps/mobile/shared/core-domain/src/commonMain/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicy.kt @@ -77,7 +77,14 @@ public object SessionNaming { @Suppress("UNUSED_PARAMETER") public fun wireSessionName(agentType: String, title: String): String = title.trim() - /** Compatibility placeholder until a host title is received. */ + /** + * The title of a session the host has not named yet. + * + * Nothing, on purpose. "New Session" was an English placeholder shown to every + * reader in every language, and it also read as a real title the host had sent. + * A blank title is the honest answer, and every surface already renders a blank + * title in its own words. + */ @Suppress("UNUSED_PARAMETER") - public fun fallbackTitle(agentType: String): String = "New Session" + public fun fallbackTitle(agentType: String): String = "" } diff --git a/src/apps/mobile/shared/core-domain/src/commonTest/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicyTest.kt b/src/apps/mobile/shared/core-domain/src/commonTest/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicyTest.kt index 1fb91a0934..0382ff0af6 100644 --- a/src/apps/mobile/shared/core-domain/src/commonTest/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicyTest.kt +++ b/src/apps/mobile/shared/core-domain/src/commonTest/kotlin/com/openbitfun/mobile/core/domain/SessionListPolicyTest.kt @@ -114,9 +114,11 @@ class SessionListPolicyTest { } @Test - fun fallbackTitleIsIndependentOfMode() { - assertEquals("New Session", SessionNaming.fallbackTitle("code")) - assertEquals("New Session", SessionNaming.fallbackTitle("cowork")) - assertEquals("New Session", SessionNaming.fallbackTitle("assistant")) + fun fallbackTitleStaysBlankForEveryMode() { + // No placeholder is invented in shared code: a surface that has no title to + // show supplies its own localized wording for the blank. + assertEquals("", SessionNaming.fallbackTitle("code")) + assertEquals("", SessionNaming.fallbackTitle("cowork")) + assertEquals("", SessionNaming.fallbackTitle("assistant")) } } diff --git a/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/RemoteSessionStore.kt b/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/RemoteSessionStore.kt index aa2f1ecd89..baeb8921ac 100644 --- a/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/RemoteSessionStore.kt +++ b/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/RemoteSessionStore.kt @@ -2352,7 +2352,11 @@ internal object RemoteResponseMapper { val id = item.id.orEmpty() return RemoteSession( id = id, - title = item.title?.takeIf(String::isNotEmpty) ?: "Session ${id.take(6)}", + // A host that named no title has none, and this layer must not invent one: + // the id is not a name, and a placeholder in English would be shown to + // every reader in every language. Each surface renders a blank title in + // its own words. + title = item.title?.takeIf(String::isNotEmpty).orEmpty(), agentType = item.agentType ?: "code", status = item.status ?: "idle", updatedAt = item.updatedAt, diff --git a/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicy.kt b/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicy.kt index 7dc81eb3db..0d40566dc4 100644 --- a/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicy.kt +++ b/src/apps/mobile/shared/core-feature/src/commonMain/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicy.kt @@ -14,6 +14,19 @@ public data class SessionActionCapabilities public constructor( public val canArchive: Boolean, public val canExport: Boolean, public val canDelete: Boolean, + /** + * Appended after the four flags that already existed, so their positions + * keep their meaning. Renaming is not one of the local-storage operations + * above — for a remote session it is a round trip to the desktop, for a + * general chat it rewrites the stored row — so folding it into one of them + * would read wrong in one scope or the other. + * + * No Swift-compatible secondary constructor accompanies this addition: the + * iOS client has no caller of [SessionActionCapabilities] today, so there is + * no `init(canViewDetails:canArchive:…)` to keep in step. If iOS ever starts + * reading these flags, it will need one, and this note is the reminder. + */ + public val canRename: Boolean, ) /** @@ -23,6 +36,13 @@ public data class SessionActionCapabilities public constructor( * local-storage operations, so a remote session never offers them however it is * typed; while a command is in flight nothing is offered at all, because the * row the menu was opened over may not survive the answer. + * + * Rename is offered in both scopes, for different reasons. A remote session's + * title belongs to the desktop and `update_session_title` is already on the + * wire; a general chat's title belongs to this device, where + * `GeneralChatIntent.RenameSession` reaches `GeneralChatStore.renameSession` and + * rewrites the stored row. Neither is one of the local-storage operations above, + * which is why it is a flag of its own rather than folded into them. */ public object SessionActionPolicy { public fun resolve( @@ -30,14 +50,18 @@ public object SessionActionPolicy { agentType: String, busy: Boolean, ): SessionActionCapabilities { - if (busy) return SessionActionCapabilities(false, false, false, false) + if (busy) return SessionActionCapabilities(false, false, false, false, false) if (scope == SessionActionScope.REMOTE) { - return SessionActionCapabilities(true, false, false, true) + return SessionActionCapabilities(true, false, false, true, true) } // Two names for one kind: the desktop and the HarmonyOS client say // "chat", the on-device store writes "general_chat" into its own rows. val isGeneralChat = agentType.lowercase() in GENERAL_CHAT_AGENT_TYPES - return SessionActionCapabilities(true, isGeneralChat, isGeneralChat, true) + // Rename is true here because the path really exists on this device + // (`GeneralChatIntent.RenameSession` -> `GeneralChatStore.renameSession`), + // not because the two lists should look alike. A surface that offers the + // row still has to pass a rename callback; without one no row is drawn. + return SessionActionCapabilities(true, isGeneralChat, isGeneralChat, true, true) } private val GENERAL_CHAT_AGENT_TYPES = setOf("chat", "general_chat") diff --git a/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/RemoteResponseMapperTest.kt b/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/RemoteResponseMapperTest.kt index 2b8a1e0f91..e856f0e532 100644 --- a/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/RemoteResponseMapperTest.kt +++ b/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/RemoteResponseMapperTest.kt @@ -214,10 +214,28 @@ class RemoteResponseMapperTest { ) assertEquals("session-1", session.id) - assertEquals("Session sessio", session.title) + // A host that named no title has none: the id is not a name, and no English + // placeholder is fabricated here — the surface renders a blank in its own words. + assertEquals("", session.title) assertEquals("code", session.agentType) assertEquals("idle", session.status) assertEquals("1781092800000", session.updatedAt) assertEquals(0, session.messageCount) } + + /** + * A title is the host's word or nothing. Inventing one in English here meant every + * reader in every language was shown a name the host never sent, and the surface + * could no longer tell "untitled" from "titled 'Session sessio'". + */ + @Test + fun aHostTitleThatIsMissingStaysBlankAndATitledSessionKeepsItsName() { + fun mapped(title: String?) = RemoteResponseMapper.session( + SessionItemResponse(id = "session-1", title = title), + ) + + assertEquals("", mapped(null).title) + assertEquals("", mapped("").title) + assertEquals("Fix the parser", mapped("Fix the parser").title) + } } diff --git a/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicyTest.kt b/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicyTest.kt index e8abc58f25..05d39fd6b7 100644 --- a/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicyTest.kt +++ b/src/apps/mobile/shared/core-feature/src/commonTest/kotlin/com/openbitfun/mobile/core/feature/session/SessionActionPolicyTest.kt @@ -5,30 +5,81 @@ import kotlin.test.assertEquals class SessionActionPolicyTest { @Test - fun aRemoteSessionNeverOffersTheLocalStorageActions() { + fun aRemoteSessionOffersRenameButNeverTheLocalStorageActions() { val capabilities = SessionActionPolicy.resolve(SessionActionScope.REMOTE, "code", busy = false) - assertEquals(SessionActionCapabilities(true, false, false, true), capabilities) + assertEquals( + SessionActionCapabilities( + canViewDetails = true, + canArchive = false, + canExport = false, + canDelete = true, + canRename = true, + ), + capabilities, + ) } @Test fun archiveAndExportBelongToGeneralChatAlone() { assertEquals( - SessionActionCapabilities(true, true, true, true), + SessionActionCapabilities( + canViewDetails = true, + canArchive = true, + canExport = true, + canDelete = true, + // A general chat is renamed on this device by + // `GeneralChatIntent.RenameSession`, so the row may offer it. + canRename = true, + ), SessionActionPolicy.resolve(SessionActionScope.GENERAL, "chat", busy = false), ) assertEquals( - SessionActionCapabilities(true, false, false, true), + SessionActionCapabilities( + canViewDetails = true, + canArchive = false, + canExport = false, + canDelete = true, + canRename = true, + ), SessionActionPolicy.resolve(SessionActionScope.GENERAL, "code", busy = false), ) } + @Test + fun theRenameFlagWasAppendedSoEarlierPositionsKeepTheirMeaning() { + // Named and positional construction of the same value: a caller that + // still writes the four flags in order cannot silently be read as + // asking for rename instead of delete. + assertEquals( + SessionActionCapabilities( + canViewDetails = true, + canArchive = false, + canExport = false, + canDelete = true, + canRename = true, + ), + SessionActionCapabilities(true, false, false, true, true), + ) + assertEquals( + SessionActionPolicy.resolve(SessionActionScope.REMOTE, "code", busy = false), + SessionActionCapabilities(true, false, false, true, true), + ) + } + @Test fun aCommandInFlightWithdrawsEverything() { - // The row the menu was opened over may not survive the answer. + // The row the menu was opened over may not survive the answer, and that + // includes a rename the menu could otherwise have offered. listOf(SessionActionScope.GENERAL, SessionActionScope.REMOTE).forEach { scope -> assertEquals( - SessionActionCapabilities(false, false, false, false), + SessionActionCapabilities( + canViewDetails = false, + canArchive = false, + canExport = false, + canDelete = false, + canRename = false, + ), SessionActionPolicy.resolve(scope, "chat", busy = true), scope.name, )