From 54dbb99a3dda483ea3f174fb28ec724163314b9b Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:03:36 +0300 Subject: [PATCH 1/7] refactor(chat): extract approvals delegate with tests --- .../ui/chat/ChatApprovalsDelegate.kt | 192 ++++++++++++++++++ .../ui/chat/ChatApprovalsDelegateTest.kt | 169 +++++++++++++++ 2 files changed, 361 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt new file mode 100644 index 00000000..99c3094c --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt @@ -0,0 +1,192 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch + +/** + * Owns approval rendering, deduplication, user response submission, and + * reconnect/resume pending approval replay. + * + * Extracted behavior-preservingly from [ChatViewModel]. + */ +class ChatApprovalsDelegate( + private val scope: CoroutineScope, + private val ioDispatcher: CoroutineDispatcher, + private val uiState: MutableStateFlow, + private val runtimeSessionId: () -> String?, + private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, + private val trackRequest: (id: String, method: String) -> Unit, + private val addSystemMessage: (text: String) -> Unit, +) { + fun handleApprovalRequest(event: WsEvent.ApprovalRequest) { + val description = event.description ?: event.command ?: "Unknown command" + val content = "**Approval Required**\n$description" + val msg = + ChatMessage( + role = MessageRole.SYSTEM, + content = content, + approvalInfo = + ApprovalInfo( + command = event.command, + description = event.description, + patternKeys = event.patternKeys, + requestId = event.requestId, + choices = event.choices, + allowPermanent = event.allowPermanent, + smartDenied = event.smartDenied, + ), + ) + uiState.update { state -> + state.copy( + messages = state.messages + msg, + isAgentTyping = false, + ) + } + // Desktop parity (`prompts.ts` receiveApprovalRequest): ack the render + // so the backend knows this client holds the prompt. Fire-and-forget. + val requestId = event.requestId + val sessionId = runtimeSessionId() ?: event.sessionId ?: uiState.value.currentSessionId + if (requestId != null && sessionId != null) { + scope.launch(ioDispatcher) { + runCatching { + wsSend( + WsMethods.APPROVAL_RECEIVED, + mapOf( + "session_id" to sessionId, + "request_id" to requestId, + ), + null, + ) + } + } + } + } + + fun replayPendingApproval(sessionId: String) { + val targetSessionId = runtimeSessionId() ?: sessionId + scope.launch(ioDispatcher) { + wsSend( + WsMethods.APPROVAL_PENDING, + mapOf("session_id" to targetSessionId), + ) { id -> trackRequest(id, WsMethods.APPROVAL_PENDING) } + } + } + + fun handleApprovalPendingResult(result: Any?) { + @Suppress("UNCHECKED_CAST") + val approvals = (result as? Map<*, *>)?.get("approvals") as? List<*> + val first = approvals?.filterIsInstance>()?.firstOrNull() ?: return + val sessionId = runtimeSessionId() ?: uiState.value.currentSessionId + maybeSurfacePendingApproval(first, sessionId) + } + + fun handleApprovalRespondResult(result: Any?) { + val map = result as? Map<*, *> + val resolved = (map?.get("resolved") as? Number)?.toInt() ?: 0 + if (resolved > 0) { + addSystemMessage("Approval submitted") + } + } + + fun maybeSurfacePendingApproval( + map: Map<*, *>, + sessionId: String?, + ) { + val requestId = map["request_id"] as? String + val alreadyShown = + requestId != null && + uiState.value.messages.any { it.approvalInfo?.requestId == requestId } + if (!alreadyShown) { + handleApprovalRequest(parseApprovalMap(map, sessionId)) + } + } + + fun respondToApproval(action: String) { + val state = uiState.value + val approvalMsg = state.messages.lastOrNull { it.approvalInfo != null } ?: return + val sessionId = runtimeSessionId() ?: state.currentSessionId ?: return + // Desktop sends `once` for a single run; legacy mobile sent `approve` + // (any non-deny still unblocks, but stay on-spec going forward). + val choice = if (action == "approve") "once" else action + val requestId = approvalMsg.approvalInfo?.requestId + + // Clear buttons immediately + uiState.update { s -> + s.copy( + messages = + s.messages.map { + if (it.id == approvalMsg.id) { + it.copy(approvalInfo = null) + } else { + it + } + }, + ) + } + + scope.launch(ioDispatcher) { + val params = + mutableMapOf( + "session_id" to sessionId, + "choice" to choice, + "all" to false, + ) + if (requestId != null) params["request_id"] = requestId + wsSend( + WsMethods.APPROVAL_RESPOND, + params, + ) { id -> trackRequest(id, WsMethods.APPROVAL_RESPOND) } + // The queue can hold more pendings — surface the next one. + replayPendingApproval(sessionId) + } + } +} + +/** + * Convert a backend approval map (snake_case, from `approval.pending` or + * `session.info` `pending_approval`) into a typed event. Mirrors + * [EventParser] defaults so replay and live paths agree. + */ +internal fun parseApprovalMap( + map: Map<*, *>, + sessionId: String?, +): WsEvent.ApprovalRequest { + @Suppress("UNCHECKED_CAST") + val patternKeys = (map["pattern_keys"] as? List<*>)?.filterIsInstance() + + @Suppress("UNCHECKED_CAST") + val rawChoices = (map["choices"] as? List<*>)?.filterIsInstance() + val allowPermanent = map["allow_permanent"] as? Boolean + val allowSession = map["allow_session"] as? Boolean + val smartDenied = map["smart_denied"] as? Boolean + val choices = + rawChoices ?: run { + if (smartDenied == true) { + listOf("once", "deny") + } else { + buildList { + add("once") + if (allowSession != false) { + add("session") + if (allowPermanent != false) add("always") + } + add("deny") + } + } + } + return WsEvent.ApprovalRequest( + command = map["command"] as? String, + description = map["description"] as? String, + patternKeys = patternKeys, + sessionId = sessionId, + requestId = map["request_id"] as? String, + choices = choices, + allowPermanent = allowPermanent, + smartDenied = smartDenied, + ) +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt new file mode 100644 index 00000000..4f3d298d --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt @@ -0,0 +1,169 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ChatApprovalsDelegateTest { + private val testDispatcher = StandardTestDispatcher() + private val testScope = TestScope(testDispatcher) + private val uiState = MutableStateFlow(ChatUiState(currentSessionId = "storage-session-1")) + private var runtimeId: String? = null + private val sentMethods = mutableListOf() + private val sentParams = mutableListOf>() + private val trackedRequests = mutableListOf>() + private val systemMessages = mutableListOf() + + private val delegate = + ChatApprovalsDelegate( + scope = testScope, + ioDispatcher = testDispatcher, + uiState = uiState, + runtimeSessionId = { runtimeId }, + wsSend = { method, params, onSent -> + sentMethods.add(method) + sentParams.add(params) + onSent?.invoke("req-id-${sentMethods.size}") + }, + trackRequest = { id, method -> trackedRequests.add(id to method) }, + addSystemMessage = { text -> systemMessages.add(text) }, + ) + + @Test + fun parseApprovalMap_honorsSmartDeniedAndChoicePermutations() { + val denied = + parseApprovalMap( + mapOf("smart_denied" to true), + sessionId = "s1", + ) + assertEquals(listOf("once", "deny"), denied.choices) + + val full = + parseApprovalMap( + mapOf("allow_session" to true, "allow_permanent" to true), + sessionId = "s1", + ) + assertEquals(listOf("once", "session", "always", "deny"), full.choices) + + val noAlways = + parseApprovalMap( + mapOf("allow_session" to true, "allow_permanent" to false), + sessionId = "s1", + ) + assertEquals(listOf("once", "session", "deny"), noAlways.choices) + + val explicit = + parseApprovalMap( + mapOf("choices" to listOf("custom", "deny")), + sessionId = "s1", + ) + assertEquals(listOf("custom", "deny"), explicit.choices) + } + + @Test + fun handleApprovalRequest_appendsSystemMessageAndSendsAckWithPreferredRuntimeId() = + testScope.runTest { + runtimeId = "runtime-session-xyz" + val event = + WsEvent.ApprovalRequest( + command = "git status", + description = "inspect working tree", + patternKeys = null, + sessionId = "event-session", + requestId = "req-123", + ) + + delegate.handleApprovalRequest(event) + advanceUntilIdle() + + val lastMsg = uiState.value.messages.last() + assertEquals(MessageRole.SYSTEM, lastMsg.role) + assertEquals("**Approval Required**\ninspect working tree", lastMsg.content) + assertEquals("req-123", lastMsg.approvalInfo?.requestId) + assertFalse(uiState.value.isAgentTyping) + + assertEquals(listOf(WsMethods.APPROVAL_RECEIVED), sentMethods) + assertEquals("runtime-session-xyz", sentParams.first()["session_id"]) + assertEquals("req-123", sentParams.first()["request_id"]) + } + + @Test + fun respondToApproval_clearsButtonsImmediatelyAndSendsChoiceOnce() = + testScope.runTest { + runtimeId = "runtime-session-xyz" + delegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "ls", + description = null, + patternKeys = null, + sessionId = null, + requestId = "req-999", + ), + ) + advanceUntilIdle() + sentMethods.clear() + sentParams.clear() + + delegate.respondToApproval("approve") + val messageBeforeIdle = uiState.value.messages.last() + assertNull(messageBeforeIdle.approvalInfo) + + advanceUntilIdle() + assertEquals(listOf(WsMethods.APPROVAL_RESPOND, WsMethods.APPROVAL_PENDING), sentMethods) + assertEquals("once", sentParams[0]["choice"]) + assertEquals("req-999", sentParams[0]["request_id"]) + assertEquals(false, sentParams[0]["all"]) + assertEquals("runtime-session-xyz", sentParams[0]["session_id"]) + assertEquals("runtime-session-xyz", sentParams[1]["session_id"]) + } + + @Test + fun maybeSurfacePendingApproval_dedupesExistingOnScreenRequestId() { + delegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "ls", + description = null, + patternKeys = null, + sessionId = null, + requestId = "dup-1", + ), + ) + val initialCount = uiState.value.messages.size + + delegate.maybeSurfacePendingApproval( + mapOf("command" to "ls", "request_id" to "dup-1"), + sessionId = "s1", + ) + assertEquals(initialCount, uiState.value.messages.size) + + delegate.maybeSurfacePendingApproval( + mapOf("command" to "pwd", "request_id" to "new-2"), + sessionId = "s1", + ) + assertEquals(initialCount + 1, uiState.value.messages.size) + assertEquals( + "new-2", + uiState.value.messages + .last() + .approvalInfo + ?.requestId, + ) + } + + @Test + fun handleApprovalRespondResult_appendsDesktopParitySystemMessage() { + delegate.handleApprovalRespondResult(mapOf("resolved" to 1)) + assertEquals(listOf("Approval submitted"), systemMessages) + } +} From 9d35bf7cdcda6f10159d872666be395918fd1fc3 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:03:42 +0300 Subject: [PATCH 2/7] refactor(chat): extract credential prompts delegate with tests --- .../ui/chat/ChatCredentialPromptsDelegate.kt | 180 ++++++++++++++++++ .../chat/ChatCredentialPromptsDelegateTest.kt | 87 +++++++++ 2 files changed, 267 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegateTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt new file mode 100644 index 00000000..8b47c45d --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt @@ -0,0 +1,180 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch + +/** + * Owns sudo and secret prompt state, timeouts, dismissal, and response RPCs. + * + * Extracted behavior-preservingly from [ChatViewModel] (issue #524). + */ +class ChatCredentialPromptsDelegate( + private val scope: CoroutineScope, + private val ioDispatcher: CoroutineDispatcher, + private val uiState: MutableStateFlow, + private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, + private val trackRequest: (id: String, method: String) -> Unit, +) { + /** + * The agent needs the user's sudo password. Surface a secure dialog and + * reply via sudo.respond. + */ + fun handleSudoRequest(event: WsEvent.SudoRequest) { + uiState.update { + it.copy( + sudoPrompt = SudoPromptUi(event.requestId, event.sessionId), + isAgentTyping = false, + ) + } + } + + /** + * Backend sudo timeout (120s) — clear only the matching dialog so a late + * expire for an old prompt never kills the current one. + */ + fun handleSudoExpire(event: WsEvent.SudoExpire) { + uiState.update { state -> + val current = state.sudoPrompt ?: return@update state + if (event.requestId != null && current.requestId != null && + event.requestId != current.requestId + ) { + return@update state + } + state.copy(sudoPrompt = null) + } + } + + /** + * The agent needs a secret value (token/password). Surface a secure dialog + * and reply via secret.respond. + */ + fun handleSecretRequest(event: WsEvent.SecretRequest) { + uiState.update { + it.copy( + secretPrompt = + SecretPromptUi( + event.requestId, + event.sessionId, + event.envVar, + event.prompt, + ), + isAgentTyping = false, + ) + } + } + + /** + * Backend secret timeout — match-only clear like [handleSudoExpire]. + */ + fun handleSecretExpire(event: WsEvent.SecretExpire) { + uiState.update { state -> + val current = state.secretPrompt ?: return@update state + if (event.requestId != null && current.requestId != null && + event.requestId != current.requestId + ) { + return@update state + } + state.copy(secretPrompt = null) + } + } + + /** + * Cancel → send empty password (desktop parity). Backend treats empty sudo as + * failed sudo (no command runs), unblocking the turn instantly. + */ + fun dismissSudo() { + val prompt = uiState.value.sudoPrompt ?: return + val sessionId = prompt.sessionId ?: uiState.value.currentSessionId + uiState.update { it.copy(sudoPrompt = null) } + if (sessionId == null) return + scope.launch(ioDispatcher) { + val params = + mutableMapOf( + "session_id" to sessionId, + "password" to "", + ) + prompt.requestId?.let { id -> params["request_id"] = id } + wsSend( + WsMethods.SUDO_RESPOND, + params, + ) { id -> trackRequest(id, WsMethods.SUDO_RESPOND) } + } + } + + /** + * Cancel → send empty value (desktop parity). Backend secret_cb returns + * skipped=True on empty, unblocking the turn instantly. + */ + fun dismissSecret() { + val prompt = uiState.value.secretPrompt ?: return + val sessionId = prompt.sessionId ?: uiState.value.currentSessionId + uiState.update { it.copy(secretPrompt = null) } + if (sessionId == null) return + scope.launch(ioDispatcher) { + val params = + mutableMapOf( + "session_id" to sessionId, + "value" to "", + ) + prompt.requestId?.let { id -> params["request_id"] = id } + wsSend( + WsMethods.SECRET_RESPOND, + params, + ) { id -> trackRequest(id, WsMethods.SECRET_RESPOND) } + } + } + + /** + * Send the user's sudo password back to the gateway. Clear prompt immediately, + * then fire the RPC. + */ + fun respondToSudo(password: String) { + val prompt = uiState.value.sudoPrompt ?: return + val sessionId = prompt.sessionId ?: uiState.value.currentSessionId ?: return + if (password.isBlank()) return + + uiState.update { it.copy(sudoPrompt = null) } + + scope.launch(ioDispatcher) { + val params = + mutableMapOf( + "session_id" to sessionId, + "password" to password, + ) + prompt.requestId?.let { id -> params["request_id"] = id } + wsSend( + WsMethods.SUDO_RESPOND, + params, + ) { id -> trackRequest(id, WsMethods.SUDO_RESPOND) } + } + } + + /** + * Send the user's secret value back to the gateway. + */ + fun respondToSecret(value: String) { + val prompt = uiState.value.secretPrompt ?: return + val sessionId = prompt.sessionId ?: uiState.value.currentSessionId ?: return + if (value.isBlank()) return + + uiState.update { it.copy(secretPrompt = null) } + + scope.launch(ioDispatcher) { + val params = + mutableMapOf( + "session_id" to sessionId, + "value" to value, + ) + prompt.requestId?.let { id -> params["request_id"] = id } + wsSend( + WsMethods.SECRET_RESPOND, + params, + ) { id -> trackRequest(id, WsMethods.SECRET_RESPOND) } + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegateTest.kt new file mode 100644 index 00000000..c888cdd1 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegateTest.kt @@ -0,0 +1,87 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ChatCredentialPromptsDelegateTest { + private val testDispatcher = StandardTestDispatcher() + private val testScope = TestScope(testDispatcher) + private val uiState = MutableStateFlow(ChatUiState(currentSessionId = "sess-1")) + private val sentMethods = mutableListOf() + private val sentParams = mutableListOf>() + private val trackedRequests = mutableListOf>() + + private val delegate = + ChatCredentialPromptsDelegate( + scope = testScope, + ioDispatcher = testDispatcher, + uiState = uiState, + wsSend = { method, params, onSent -> + sentMethods.add(method) + sentParams.add(params) + onSent?.invoke("req-${sentMethods.size}") + }, + trackRequest = { id, method -> trackedRequests.add(id to method) }, + ) + + @Test + fun handleSudoRequest_andRespond_clearsPromptAndSendsRpc() = + testScope.runTest { + delegate.handleSudoRequest(WsEvent.SudoRequest(requestId = "r-sudo", sessionId = "s-1")) + assertEquals("r-sudo", uiState.value.sudoPrompt?.requestId) + assertFalse(uiState.value.isAgentTyping) + + delegate.respondToSudo("secret-pass") + assertNull(uiState.value.sudoPrompt) + + advanceUntilIdle() + assertEquals(listOf(WsMethods.SUDO_RESPOND), sentMethods) + assertEquals("secret-pass", sentParams.first()["password"]) + assertEquals("r-sudo", sentParams.first()["request_id"]) + assertEquals("s-1", sentParams.first()["session_id"]) + } + + @Test + fun handleSecretRequest_andDismiss_sendsEmptyValue() = + testScope.runTest { + delegate.handleSecretRequest( + WsEvent.SecretRequest( + requestId = "r-sec", + sessionId = "s-1", + envVar = "OPENAI_API_KEY", + prompt = "Enter key", + ), + ) + assertEquals("OPENAI_API_KEY", uiState.value.secretPrompt?.envVar) + + delegate.dismissSecret() + assertNull(uiState.value.secretPrompt) + + advanceUntilIdle() + assertEquals(listOf(WsMethods.SECRET_RESPOND), sentMethods) + assertEquals("", sentParams.first()["value"]) + assertEquals("r-sec", sentParams.first()["request_id"]) + } + + @Test + fun expireHandlers_ignoreMismatchedRequestId() { + delegate.handleSudoRequest(WsEvent.SudoRequest(requestId = "keep-me", sessionId = "s-1")) + delegate.handleSudoExpire(WsEvent.SudoExpire(requestId = "other-req", sessionId = "s-1")) + assertNotNull(uiState.value.sudoPrompt) + + delegate.handleSudoExpire(WsEvent.SudoExpire(requestId = "keep-me", sessionId = "s-1")) + assertNull(uiState.value.sudoPrompt) + } +} From 280a03ea235afa8cf27d592c6797a0530955eaaf Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:03:46 +0300 Subject: [PATCH 3/7] refactor(chat): extract model switch delegate with tests --- .../ui/chat/ChatModelSwitchDelegate.kt | 286 ++++++++++++++++++ .../ui/chat/ChatModelSwitchDelegateTest.kt | 141 +++++++++ 2 files changed, 427 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt new file mode 100644 index 00000000..44e2996c --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt @@ -0,0 +1,286 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.model.ModelCapabilities +import com.m57.hermescontrol.data.model.ModelProvider +import com.m57.hermescontrol.data.model.PinnedModel +import com.m57.hermescontrol.data.remote.ApiClient +import com.m57.hermescontrol.data.remote.NetworkResult +import com.m57.hermescontrol.data.remote.safeApiCall +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import java.util.concurrent.ConcurrentHashMap + +/** + * Tracks an in-flight `config.set key=model` RPC and the model it would replace + * so confirmation dialog dismiss can roll back optimistically. + */ +data class ActiveModelSwitch( + val spec: String, + val previousModel: String?, +) + +/** + * Owns model picker loading/caching, in-session model switches, model capabilities, + * pinned model toggling, and expensive model switch confirmation dialogs. + * + * Extracted behavior-preservingly from [ChatViewModel] (issue #589). + */ +class ChatModelSwitchDelegate( + private val scope: CoroutineScope, + private val ioDispatcher: CoroutineDispatcher, + private val uiState: MutableStateFlow, + private val runtimeSessionId: () -> String?, + private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, + private val trackRequest: (id: String, method: String) -> Unit, + private val addAssistantMessage: (text: String) -> Unit, + private val handleSlashCommand: (command: String) -> Unit, + private val fetchContextUsage: () -> Unit, + private val getModelOptionsCall: suspend ( + refresh: Boolean, + ) -> NetworkResult = + { refresh -> safeApiCall { ApiClient.hermesApi.getModelOptions(refresh = refresh) } }, + private val getPinnedModels: () -> List = { AuthManager.getPinnedModels() }, + private val savePinnedModels: (List) -> Unit = { AuthManager.savePinnedModels(it) }, +) { + val pendingModelSwitchRequests = ConcurrentHashMap() + var activeModelSwitchConfirmation: ActiveModelSwitch? = null + var optimisticPreviousModel: String? = null + var cachedModelOptions: List = emptyList() + + fun isModelPickerCommand(command: String): Boolean { + val trimmed = command.trim() + if (!trimmed.startsWith("/", ignoreCase = true)) return false + val body = trimmed.removePrefix("/").trimStart() + return body.equals("model", ignoreCase = true) || + ( + body.startsWith("model ", ignoreCase = true) && + body.substringAfter("model").trim().isEmpty() + ) + } + + fun preloadModelOptions() { + scope.launch(ioDispatcher) { + val result = getModelOptionsCall(false) + if (result is NetworkResult.Success) { + cachedModelOptions = result.data.providers.orEmpty() + uiState.update { it.copy(modelPickerPinned = getPinnedModels()) } + syncCurrentModelCapabilities() + } + } + } + + fun openModelPicker() { + val hasCached = cachedModelOptions.isNotEmpty() + uiState.update { + it.copy( + showModelPicker = true, + modelPickerProviders = if (hasCached) cachedModelOptions else emptyList(), + modelPickerPinned = getPinnedModels(), + modelPickerLoading = !hasCached, + ) + } + if (!hasCached) { + refreshModelOptions() + } + } + + fun refreshModelOptions() { + uiState.update { it.copy(modelPickerLoading = true) } + scope.launch(ioDispatcher) { + val result = getModelOptionsCall(true) + when (result) { + is NetworkResult.Success -> { + cachedModelOptions = result.data.providers.orEmpty() + uiState.update { + it.copy( + modelPickerProviders = cachedModelOptions, + modelPickerLoading = false, + ) + } + syncCurrentModelCapabilities() + } + + is NetworkResult.Failure -> { + uiState.update { + it.copy( + modelPickerLoading = false, + errorMessage = "Failed to load models: ${result.error.message}", + ) + } + } + } + } + } + + fun closeModelPicker() { + uiState.update { it.copy(showModelPicker = false, modelPickerLoading = false) } + } + + fun togglePinModel( + providerSlug: String, + modelName: String, + ) { + val currentPinned = getPinnedModels().toMutableList() + val target = PinnedModel(providerSlug, modelName) + if (currentPinned.contains(target)) { + currentPinned.remove(target) + } else { + currentPinned.add(target) + } + savePinnedModels(currentPinned) + uiState.update { it.copy(modelPickerPinned = currentPinned) } + } + + fun sendSlashModel( + provider: String, + model: String, + ) { + optimisticPreviousModel = uiState.value.currentSessionModel + uiState.update { + it.copy( + showModelPicker = false, + modelPickerLoading = false, + currentSessionModel = "$provider/$model", + ) + } + syncCurrentModelCapabilities() + fetchContextUsage() + handleSlashCommand("/model $model --provider $provider --session") + } + + fun handleModelSwitch( + command: String, + confirmExpensive: Boolean = false, + ) { + val sessionId = runtimeSessionId() + if (sessionId == null) { + addAssistantMessage("No active session. Use `/new` to create one.") + return + } + val spec = + if (command.startsWith("/model", ignoreCase = true)) { + command.substring(6).trim() + } else { + command.trim() + } + val previousModel = optimisticPreviousModel ?: uiState.value.currentSessionModel + optimisticPreviousModel = null + val params = + mutableMapOf( + "key" to "model", + "value" to spec, + "session_id" to sessionId, + ) + if (confirmExpensive) { + params["confirm_expensive_model"] = true + } + scope.launch(ioDispatcher) { + wsSend( + WsMethods.CONFIG_SET, + params, + ) { id -> + trackRequest(id, WsMethods.CONFIG_SET) + pendingModelSwitchRequests[id] = ActiveModelSwitch(spec, previousModel) + } + } + } + + fun handleConfigSetResult( + id: String, + result: Any?, + ) { + val pending = pendingModelSwitchRequests.remove(id) + val map = result as? Map<*, *> ?: return + val key = map["key"] as? String + if (key == "model") { + val confirmRequired = map["confirm_required"] as? Boolean ?: false + if (confirmRequired) { + val confirmMessage = + (map["confirm_message"] as? String) + ?: (map["warning"] as? String) + ?: "This model requires confirmation to switch. Continue?" + activeModelSwitchConfirmation = pending + uiState.update { + it.copy(modelSwitchConfirmMessage = confirmMessage) + } + } + } + } + + fun dismissModelSwitchConfirm() { + val pending = activeModelSwitchConfirmation + activeModelSwitchConfirmation = null + uiState.update { + it.copy( + modelSwitchConfirmMessage = null, + currentSessionModel = pending?.previousModel ?: it.currentSessionModel, + ) + } + syncCurrentModelCapabilities() + fetchContextUsage() + } + + fun confirmModelSwitchExpensive() { + val pending = activeModelSwitchConfirmation + activeModelSwitchConfirmation = null + uiState.update { it.copy(modelSwitchConfirmMessage = null) } + if (pending != null) { + handleModelSwitch(pending.spec, confirmExpensive = true) + } + } + + fun setReasoningLevel(level: String?) { + uiState.update { it.copy(reasoningLevel = level) } + val sessionId = runtimeSessionId() ?: return + if (level == null) return + scope.launch(ioDispatcher) { + wsSend( + WsMethods.CONFIG_SET, + mapOf( + "key" to "reasoning", + "value" to level, + "session_id" to sessionId, + ), + ) { id -> trackRequest(id, WsMethods.CONFIG_SET) } + } + } + + fun getModelCapabilities( + providerSlug: String, + modelName: String, + ): ModelCapabilities? = cachedModelOptions.find { it.slug == providerSlug }?.capabilities?.get(modelName) + + fun getCurrentModelCapabilities(): ModelCapabilities? { + val label = uiState.value.currentSessionModel ?: return null + val idx = label.indexOf('/') + if (idx <= 0) return null + val provider = label.substring(0, idx) + val model = label.substring(idx + 1) + return getModelCapabilities(provider, model) + } + + fun syncCurrentModelCapabilities() { + val label = uiState.value.currentSessionModel + val caps = + if (label == null) { + null + } else { + val idx = label.indexOf('/') + if (idx <= 0) { + null + } else { + val provider = label.substring(0, idx) + val model = label.substring(idx + 1) + cachedModelOptions.find { it.slug == provider }?.capabilities?.get(model) + } + } + uiState.update { state -> + if (state.currentModelCapabilities != caps) state.copy(currentModelCapabilities = caps) else state + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt new file mode 100644 index 00000000..346af7c8 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt @@ -0,0 +1,141 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.model.ModelCapabilities +import com.m57.hermescontrol.data.model.ModelOptionsResponse +import com.m57.hermescontrol.data.model.ModelProvider +import com.m57.hermescontrol.data.model.PinnedModel +import com.m57.hermescontrol.data.remote.NetworkResult +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ChatModelSwitchDelegateTest { + private val testDispatcher = StandardTestDispatcher() + private val testScope = TestScope(testDispatcher) + private val uiState = + MutableStateFlow(ChatUiState(currentSessionId = "sess-1", currentSessionModel = "anthropic/claude-3")) + private var runtimeId: String? = "runtime-1" + private val sentMethods = mutableListOf() + private val sentParams = mutableListOf>() + private val slashCommands = mutableListOf() + private val assistantMessages = mutableListOf() + private var contextRefetched = 0 + private var pinnedList = mutableListOf(PinnedModel("anthropic", "claude-3")) + + private val fakeResponse = + ModelOptionsResponse( + providers = + listOf( + ModelProvider( + slug = "openai", + name = "OpenAI", + models = listOf("gpt-4o"), + capabilities = mapOf("gpt-4o" to ModelCapabilities(reasoning = true)), + ), + ), + ) + + private val delegate = + ChatModelSwitchDelegate( + scope = testScope, + ioDispatcher = testDispatcher, + uiState = uiState, + runtimeSessionId = { runtimeId }, + wsSend = { method, params, onSent -> + sentMethods.add(method) + sentParams.add(params) + onSent?.invoke("req-${sentMethods.size}") + }, + trackRequest = { _, _ -> }, + addAssistantMessage = { assistantMessages.add(it) }, + handleSlashCommand = { slashCommands.add(it) }, + fetchContextUsage = { contextRefetched++ }, + getModelOptionsCall = { NetworkResult.Success(fakeResponse) }, + getPinnedModels = { pinnedList }, + savePinnedModels = { pinnedList = it.toMutableList() }, + ) + + @Test + fun isModelPickerCommand_identifiesBareModelCommandOnly() { + assertTrue(delegate.isModelPickerCommand("/model")) + assertTrue(delegate.isModelPickerCommand("/MODEL ")) + assertTrue(delegate.isModelPickerCommand(" /model ")) + assertFalse(delegate.isModelPickerCommand("/model openai/gpt-4o")) + assertFalse(delegate.isModelPickerCommand("hello /model")) + } + + @Test + fun openModelPicker_andPreload_populatesProvidersAndPinned() = + testScope.runTest { + delegate.preloadModelOptions() + advanceUntilIdle() + + delegate.openModelPicker() + assertTrue(uiState.value.showModelPicker) + assertEquals(1, uiState.value.modelPickerProviders.size) + assertFalse(uiState.value.modelPickerLoading) + } + + @Test + fun sendSlashModel_optimisticallyUpdatesModelAndDispatchesSlash() { + delegate.sendSlashModel("openai", "gpt-4o") + assertEquals("openai/gpt-4o", uiState.value.currentSessionModel) + assertFalse(uiState.value.showModelPicker) + assertEquals(listOf("/model gpt-4o --provider openai --session"), slashCommands) + assertEquals(1, contextRefetched) + } + + @Test + fun handleModelSwitch_stripsLeadingSlashModel_andHandlesConfirmation() = + testScope.runTest { + delegate.handleModelSwitch("/MODEL gpt-4o --provider openai --session") + advanceUntilIdle() + + assertEquals(listOf(WsMethods.CONFIG_SET), sentMethods) + assertEquals("gpt-4o --provider openai --session", sentParams.first()["value"]) + + delegate.handleConfigSetResult( + id = "req-1", + result = + mapOf( + "key" to "model", + "confirm_required" to true, + "confirm_message" to "High token cost ahead", + ), + ) + assertEquals("High token cost ahead", uiState.value.modelSwitchConfirmMessage) + + delegate.confirmModelSwitchExpensive() + advanceUntilIdle() + assertEquals(2, sentMethods.size) + assertEquals(true, sentParams[1]["confirm_expensive_model"]) + } + + @Test + fun dismissModelSwitchConfirm_revertsToPreviousModel() = + testScope.runTest { + delegate.sendSlashModel("opencode-free", "muse-spark-1.3-contributor-free") + advanceUntilIdle() + // In real ChatViewModel, handleSlashCommand routes to handleModelSwitch: + delegate.handleModelSwitch("/model muse-spark-1.3-contributor-free --provider opencode-free --session") + advanceUntilIdle() + delegate.handleConfigSetResult( + id = "req-1", + result = mapOf("key" to "model", "confirm_required" to true), + ) + + delegate.dismissModelSwitchConfirm() + assertNull(uiState.value.modelSwitchConfirmMessage) + assertEquals("anthropic/claude-3", uiState.value.currentSessionModel) + } +} From 3e012dbf3148e969e6d2a5177825114158c31fe0 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:03:54 +0300 Subject: [PATCH 4/7] refactor(chat): extract rpc request tracker with tests --- .../ui/chat/ChatRpcRequestTracker.kt | 111 ++++++++++++++++++ .../ui/chat/ChatRpcRequestTrackerTest.kt | 94 +++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt new file mode 100644 index 00000000..8492ab54 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt @@ -0,0 +1,111 @@ +package com.m57.hermescontrol.ui.chat + +import java.util.concurrent.ConcurrentHashMap + +/** + * Tracks an in-flight RPC request associated with a specific session generation + * and resume sequence to guard against stale results/errors from previous sessions. + */ +data class TrackedSessionRequest( + val generation: Long, + val resumeSequence: Long = 0L, + val sessionId: String? = null, +) + +/** + * Owns RPC id-to-method mapping, session request generations, hydration sequences, + * and staleness detection. + * + * Extracted behavior-preservingly from [ChatViewModel] to ensure deterministic + * RPC tracking across session creation, switches, resumes, and reconnects. + */ +class ChatRpcRequestTracker( + private val getCurrentSessionId: () -> String?, +) { + /** Maps an in-flight RPC id to its method for UI error labeling. */ + private val idToMethod = ConcurrentHashMap() + private val sessionRequestById = ConcurrentHashMap() + + var sessionGeneration: Long = 0L + private set + + var resumeRequestSequence: Long = 0L + private set + var activeResumeRequestSequence: Long = 0L + private set + + var hydrationRequestSequence: Long = 0L + private set + var activeHydrationRequestSequence: Long = 0L + private set + + var resumedGeneration: Long = -1L + var hydratedGeneration: Long = -1L + + fun nextSessionGeneration(): Long { + sessionGeneration++ + return sessionGeneration + } + + fun nextResumeSequence(): Long { + resumeRequestSequence++ + activeResumeRequestSequence = resumeRequestSequence + return resumeRequestSequence + } + + fun nextHydrationSequence(): Long { + hydrationRequestSequence++ + activeHydrationRequestSequence = hydrationRequestSequence + return hydrationRequestSequence + } + + fun trackRequest( + id: String, + method: String, + ) { + idToMethod[id] = method + } + + fun trackSessionRequest( + id: String, + method: String, + generation: Long, + resumeSequence: Long = 0L, + sessionId: String? = null, + ) { + sessionRequestById[id] = TrackedSessionRequest(generation, resumeSequence, sessionId) + trackRequest(id, method) + } + + fun isCurrentSessionRequest( + sessionId: String, + generation: Long, + ): Boolean = generation == sessionGeneration && sessionId == getCurrentSessionId() + + fun isCurrentHydration( + sessionId: String, + generation: Long, + requestSequence: Long, + ): Boolean = requestSequence == activeHydrationRequestSequence && isCurrentSessionRequest(sessionId, generation) + + fun isStaleSessionRequest(id: String): Boolean = sessionRequestById[id]?.let(::isStaleSessionRequest) == true + + fun isStaleSessionRequest(request: TrackedSessionRequest): Boolean = + request.generation != sessionGeneration || + (request.sessionId != null && request.sessionId != getCurrentSessionId()) || + (request.resumeSequence != 0L && request.resumeSequence != activeResumeRequestSequence) + + fun removeMethod(id: String): String? = idToMethod.remove(id) + + fun removeSessionRequest(id: String): TrackedSessionRequest? = sessionRequestById.remove(id) + + fun forgetRequest(id: String) { + idToMethod.remove(id) + sessionRequestById.remove(id) + } + + fun resetSessionStateTracking() { + resumedGeneration = -1L + hydratedGeneration = -1L + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt new file mode 100644 index 00000000..78eac772 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt @@ -0,0 +1,94 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatRpcRequestTrackerTest { + private var currentSessionId: String? = "session-a" + private val tracker = ChatRpcRequestTracker { currentSessionId } + + @Test + fun trackRequest_andRemoveMethod() { + tracker.trackRequest("req-1", "session.create") + assertEquals("session.create", tracker.removeMethod("req-1")) + assertNull(tracker.removeMethod("req-1")) + } + + @Test + fun generationIncrement_detectsStaleGenerations() { + val gen1 = tracker.nextSessionGeneration() + assertEquals(1L, gen1) + + tracker.trackSessionRequest( + id = "req-resume-1", + method = "session.resume", + generation = gen1, + sessionId = "session-a", + ) + assertFalse(tracker.isStaleSessionRequest("req-resume-1")) + + val gen2 = tracker.nextSessionGeneration() + assertEquals(2L, gen2) + assertTrue(tracker.isStaleSessionRequest("req-resume-1")) + } + + @Test + fun sessionMismatch_marksRequestStale() { + val gen = tracker.nextSessionGeneration() + tracker.trackSessionRequest( + id = "req-2", + method = "session.resume", + generation = gen, + sessionId = "session-a", + ) + assertFalse(tracker.isStaleSessionRequest("req-2")) + + currentSessionId = "session-b" + assertTrue(tracker.isStaleSessionRequest("req-2")) + } + + @Test + fun resumeSequence_marksSupersededResumeStale() { + val gen = tracker.nextSessionGeneration() + val seq1 = tracker.nextResumeSequence() + tracker.trackSessionRequest( + id = "req-resume-seq1", + method = "session.resume", + generation = gen, + resumeSequence = seq1, + sessionId = "session-a", + ) + assertFalse(tracker.isStaleSessionRequest("req-resume-seq1")) + + val seq2 = tracker.nextResumeSequence() + assertEquals(2L, seq2) + assertTrue(tracker.isStaleSessionRequest("req-resume-seq1")) + } + + @Test + fun hydrationSequence_guardsActiveHydration() { + val gen = tracker.nextSessionGeneration() + val hyd1 = tracker.nextHydrationSequence() + assertTrue(tracker.isCurrentHydration("session-a", gen, hyd1)) + + val hyd2 = tracker.nextHydrationSequence() + assertFalse(tracker.isCurrentHydration("session-a", gen, hyd1)) + assertTrue(tracker.isCurrentHydration("session-a", gen, hyd2)) + } + + @Test + fun forgetRequest_removesBothTrackingEntries() { + tracker.trackSessionRequest( + id = "req-forget", + method = "session.resume", + generation = tracker.sessionGeneration, + sessionId = "session-a", + ) + tracker.forgetRequest("req-forget") + assertNull(tracker.removeMethod("req-forget")) + assertNull(tracker.removeSessionRequest("req-forget")) + } +} From b9ee61d429c559bd4855019386fbb8ad95843fd2 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:03:58 +0300 Subject: [PATCH 5/7] refactor(remote): extract chat relogin authenticator --- .../data/remote/ChatReloginAuthenticator.kt | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/data/remote/ChatReloginAuthenticator.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ChatReloginAuthenticator.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ChatReloginAuthenticator.kt new file mode 100644 index 00000000..643fe1c7 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ChatReloginAuthenticator.kt @@ -0,0 +1,122 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.local.AuthManager +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import org.json.JSONException +import org.json.JSONObject +import java.io.IOException +import java.util.concurrent.TimeUnit + +/** + * Handles basic credential login and WS ticket minting for in-chat reconnect/re-authentication. + * + * Extracted from [com.m57.hermescontrol.ui.chat.ChatViewModel] to isolate raw OkHttp network calls. + */ +class ChatReloginAuthenticator( + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, + private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main, +) { + suspend fun relogin( + username: String, + password: String, + onSuccess: () -> Unit, + onFailure: (String) -> Unit, + ) { + withContext(ioDispatcher) { + val endpoint = AuthManager.endpointForBuild() + val jsonMediaType = "application/json; charset=utf-8".toMediaType() + val jsonBody = + JSONObject() + .put("provider", "basic") + .put("username", username) + .put("password", password) + .put("next", "") + .toString() + + try { + val loginClient = + OkHttpProvider.probe + .newBuilder() + .connectTimeout(10, TimeUnit.SECONDS) + .readTimeout(10, TimeUnit.SECONDS) + .build() + + val loginReq = + Request + .Builder() + .url(endpoint.resolve("auth/password-login").toString()) + .header("Content-Type", "application/json") + .post(jsonBody.toRequestBody(jsonMediaType)) + .build() + + loginClient.newCall(loginReq).execute().use { loginResp -> + if (!loginResp.isSuccessful) { + val msg = + when (loginResp.code) { + 401 -> "Invalid username or password (401)" + 403 -> "Forbidden (403)" + else -> "HTTP error code: ${loginResp.code}" + } + withContext(mainDispatcher) { + onFailure(msg) + } + return@withContext + } + } + + val ticketClient = + OkHttpProvider.base + .newBuilder() + .connectTimeout(10, TimeUnit.SECONDS) + .readTimeout(10, TimeUnit.SECONDS) + .build() + + val ticketReq = + Request + .Builder() + .url(endpoint.resolve("api/auth/ws-ticket").toString()) + .post("{}".toRequestBody(jsonMediaType)) + .build() + + ticketClient.newCall(ticketReq).execute().use { ticketResp -> + if (!ticketResp.isSuccessful) { + withContext(mainDispatcher) { + onFailure("Failed to mint WS ticket: HTTP ${ticketResp.code}") + } + return@withContext + } + + val body = ticketResp.body.string() + val ticket = JSONObject(body).optString("ticket").takeIf { it.isNotBlank() } + + if (ticket.isNullOrBlank()) { + withContext(mainDispatcher) { + onFailure("Invalid ticket returned from server") + } + return@withContext + } + + AuthManager.setWsAuthParam("ticket") + AuthManager.setToken(ticket) + + withContext(mainDispatcher) { + onSuccess() + } + } + } catch (e: IOException) { + withContext(mainDispatcher) { + onFailure("Connection failed: ${e.message}") + } + } catch (e: JSONException) { + withContext(mainDispatcher) { + onFailure("Connection failed: ${e.message}") + } + } + } + } +} From 0af4782da1f492a8531d7ef09290c16aff5821a9 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:04:09 +0300 Subject: [PATCH 6/7] refactor(chat): wire extracted delegates into chat viewmodel --- .../hermescontrol/ui/chat/ChatViewModel.kt | 820 ++---------------- 1 file changed, 87 insertions(+), 733 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 90910bc5..bdf18ffa 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -515,9 +515,6 @@ class ChatViewModel( val previousModel: String?, ) - private val pendingModelSwitchRequests = ConcurrentHashMap() - private var activeModelSwitchConfirmation: ActiveModelSwitch? = null - private var optimisticPreviousModel: String? = null private var resumeRequestSequence = 0L private var activeResumeRequestSequence = 0L private var hydrationRequestSequence = 0L @@ -615,12 +612,44 @@ class ChatViewModel( ioDispatcher = ioDispatcher, ) - /** - * Model options cached from GET /api/model/options so the in-session model - * picker (issue #589) opens instantly when the user types /model or taps the - * top-bar chip. Preloaded at GatewayReady; refreshed on open if empty. - */ - private var cachedModelOptions: List = emptyList() + private val reloginAuthenticator = + com.m57.hermescontrol.data.remote.ChatReloginAuthenticator( + ioDispatcher = ioDispatcher, + mainDispatcher = Dispatchers.Main, + ) + + private val modelSwitchDelegate = + ChatModelSwitchDelegate( + scope = viewModelScope, + ioDispatcher = ioDispatcher, + uiState = _uiState, + runtimeSessionId = { runtimeSessionId }, + wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, + trackRequest = { id, method -> trackRequest(id, method) }, + addAssistantMessage = { text -> addAssistantMessage(text) }, + handleSlashCommand = { cmd -> handleSlashCommand(cmd) }, + fetchContextUsage = { fetchContextUsage() }, + ) + + private val credentialPromptsDelegate = + ChatCredentialPromptsDelegate( + scope = viewModelScope, + ioDispatcher = ioDispatcher, + uiState = _uiState, + wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, + trackRequest = { id, method -> trackRequest(id, method) }, + ) + + private val approvalsDelegate = + ChatApprovalsDelegate( + scope = viewModelScope, + ioDispatcher = ioDispatcher, + uiState = _uiState, + runtimeSessionId = { runtimeSessionId }, + wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, + trackRequest = { id, method -> trackRequest(id, method) }, + addSystemMessage = { text -> addSystemMessage(text) }, + ) private val streamingController = ChatStreamingController( @@ -796,7 +825,7 @@ class ChatViewModel( addSystemMessage("Connected to Hermes") loadSessions() fetchCommandCatalog() - preloadModelOptions() + modelSwitchDelegate.preloadModelOptions() val currentId = _uiState.value.currentSessionId if (currentId != null) { if (sessionHasServerPresence) { @@ -944,7 +973,7 @@ class ChatViewModel( fullContextTokens = if (modelSwapped) null else state.fullContextTokens, ) } - syncCurrentModelCapabilities() + modelSwitchDelegate.syncCurrentModelCapabilities() if (modelSwapped) { // Issue #817: after a swap the REST model/info window is // PROFILE-scoped and may describe the old model (e.g. a @@ -957,18 +986,10 @@ class ChatViewModel( // reconciliation) — surface it unless already on screen. val pendingApproval = info["pending_approval"] as? Map<*, *> if (pendingApproval != null) { - val rid = pendingApproval["request_id"] as? String - val alreadyShown = - rid != null && - _uiState.value.messages.any { it.approvalInfo?.requestId == rid } - if (!alreadyShown) { - handleApprovalRequest( - parseApprovalMap( - pendingApproval, - runtimeSessionId ?: _uiState.value.currentSessionId, - ), - ) - } + approvalsDelegate.maybeSurfacePendingApproval( + pendingApproval, + runtimeSessionId ?: _uiState.value.currentSessionId, + ) } } } @@ -1034,23 +1055,23 @@ class ChatViewModel( } is WsEvent.ApprovalRequest -> { - handleApprovalRequest(event) + approvalsDelegate.handleApprovalRequest(event) } is WsEvent.SudoRequest -> { - handleSudoRequest(event) + credentialPromptsDelegate.handleSudoRequest(event) } is WsEvent.SudoExpire -> { - handleSudoExpire(event) + credentialPromptsDelegate.handleSudoExpire(event) } is WsEvent.SecretRequest -> { - handleSecretRequest(event) + credentialPromptsDelegate.handleSecretRequest(event) } is WsEvent.SecretExpire -> { - handleSecretExpire(event) + credentialPromptsDelegate.handleSecretExpire(event) } is WsEvent.GatewayError -> { @@ -1244,7 +1265,7 @@ class ChatViewModel( terminalBackend = terminalBackend ?: it.terminalBackend, ) } - syncCurrentModelCapabilities() + modelSwitchDelegate.syncCurrentModelCapabilities() // Mirror the active runtime session id app-wide (issue #532). ActiveSessionHolder.set(runtimeSessionId ?: sessionId, sessionId) addSystemMessage("Session resumed") @@ -1257,21 +1278,13 @@ class ChatViewModel( // the full queue in case more are parked. val pendingApproval = resultMap?.get("pending_approval") as? Map<*, *> if (pendingApproval != null) { - val rid = pendingApproval["request_id"] as? String - val alreadyShown = - rid != null && - _uiState.value.messages.any { it.approvalInfo?.requestId == rid } - if (!alreadyShown) { - handleApprovalRequest( - parseApprovalMap( - pendingApproval, - runtimeSessionId ?: sessionId, - ), - ) - } + approvalsDelegate.maybeSurfacePendingApproval( + pendingApproval, + runtimeSessionId ?: sessionId, + ) } val activeSessionId = runtimeSessionId ?: sessionId - if (activeSessionId != null) replayPendingApproval(activeSessionId) + if (activeSessionId != null) approvalsDelegate.replayPendingApproval(activeSessionId) } WsMethods.SESSION_INTERRUPT -> { @@ -1304,34 +1317,15 @@ class ChatViewModel( } WsMethods.APPROVAL_RESPOND -> { - val map = result as? Map<*, *> - val resolved = (map?.get("resolved") as? Number)?.toInt() ?: 0 - if (resolved > 0) { - addSystemMessage("Approval submitted") - } + approvalsDelegate.handleApprovalRespondResult(result) } WsMethods.APPROVAL_PENDING -> { - handleApprovalPendingResult(result) + approvalsDelegate.handleApprovalPendingResult(result) } WsMethods.CONFIG_SET -> { - val pending = pendingModelSwitchRequests.remove(id) - val map = result as? Map<*, *> ?: return - val key = map["key"] as? String - if (key == "model") { - val confirmRequired = map["confirm_required"] as? Boolean ?: false - if (confirmRequired) { - val confirmMessage = - (map["confirm_message"] as? String) - ?: (map["warning"] as? String) - ?: "This model requires confirmation to switch. Continue?" - activeModelSwitchConfirmation = pending - _uiState.update { - it.copy(modelSwitchConfirmMessage = confirmMessage) - } - } - } + modelSwitchDelegate.handleConfigSetResult(id, result) } } } @@ -1455,7 +1449,7 @@ class ChatViewModel( if (trimmed.startsWith("/", ignoreCase = true)) { // Issue #589: a bare "/model" (no argument) opens the picker instead // of requiring the user to hand-type the provider/model. - if (isModelPickerCommand(trimmed)) { + if (modelSwitchDelegate.isModelPickerCommand(trimmed)) { openModelPicker() return } @@ -1880,7 +1874,7 @@ class ChatViewModel( } is SlashResult.ModelSwitch -> { - handleModelSwitch(command) + modelSwitchDelegate.handleModelSwitch(command) } is SlashResult.Update -> { @@ -2123,68 +2117,6 @@ class ChatViewModel( } } - /** - * Hot-swap the current session's model via the backend's model-switch - * mechanism (issue #589). - * - * The TUI gateway's `prompt.submit` does NOT parse slash commands (it would - * make the LLM treat "/model ..." as a chat message), and `command.dispatch` - * only knows quick/plugin/bundle/skill commands (4018s on /model). The - * correct RPC is `config.set` with `key="model"` — the gateway (server.py - * `config.set`, L10253) routes `key=="model"` straight to `_apply_model_switch` - * using the same `_sessions.get(session_id)` lookup that the working - * `command.dispatch` uses. - * - * IMPORTANT: `config.set` key=model passes the value DIRECTLY to - * `parse_model_flags` (it does NOT strip a leading "/model" like slash.exec / - * prompt.submit do). So we strip the "/model" prefix here and send the bare - * spec `parse_model_flags` understands: - * ` --provider --session` - * (matching the TUI client's `modelValueForConfigSet`). - */ - private fun handleModelSwitch( - command: String, - confirmExpensive: Boolean = false, - ) { - val sessionId = runtimeSessionId - if (sessionId == null) { - addAssistantMessage("No active session. Use `/new` to create one.") - return - } - // Strip a leading "/model" (and any following whitespace) — config.set - // key=model expects the bare spec, not a slash command. Match the - // dispatcher's case-insensitive "/model" detection so a typed "/MODEL" - // (or any casing) doesn't forward the literal slash prefix to the - // backend, where parse_model_flags wouldn't recognize it. - val spec = - if (command.startsWith("/model", ignoreCase = true)) { - command.substring(6).trim() - } else { - command.trim() - } - val previousModel = optimisticPreviousModel ?: _uiState.value.currentSessionModel - optimisticPreviousModel = null - val params = - mutableMapOf( - "key" to "model", - "value" to spec, - "session_id" to sessionId, - ) - if (confirmExpensive) { - params["confirm_expensive_model"] = true - } - viewModelScope.launch(ioDispatcher) { - wsClient.send( - WsMethods.CONFIG_SET, - params, - onSent = { id -> - trackRequest(id, WsMethods.CONFIG_SET) - pendingModelSwitchRequests[id] = ActiveModelSwitch(spec, previousModel) - }, - ) - } - } - /** * Submits [text] as a prompt to the current session via WS, without * adding a duplicate user message. Used by [handleDispatchResult] when @@ -2470,220 +2402,34 @@ class ChatViewModel( // ── In-session model picker (issue #589) ───────────────────────────── - /** - * Whether [command] should open the in-session model picker instead of being - * dispatched as a normal slash command. True for a bare `/model` (with no - * trailing model argument) — the picker supplies the argument interactively. - * A fully-typed `/model provider/model` is forwarded straight to the backend. - */ - private fun isModelPickerCommand(command: String): Boolean { - val trimmed = command.trim() - if (!trimmed.startsWith("/", ignoreCase = true)) return false - val body = trimmed.removePrefix("/").trimStart() - // Must be exactly "model" with no argument (or just whitespace). - return body.equals("model", ignoreCase = true) || - ( - body.startsWith("model ", ignoreCase = true) && - body.substringAfter("model").trim().isEmpty() - ) - } + fun openModelPicker() = modelSwitchDelegate.openModelPicker() - /** Preload model options so the picker opens instantly (no spinner on /model). */ - private fun preloadModelOptions() { - viewModelScope.launch(ioDispatcher) { - val result = - safeApiCall { - ApiClient.hermesApi.getModelOptions(refresh = false) - } - if (result is NetworkResult.Success) { - cachedModelOptions = result.data.providers.orEmpty() - _uiState.update { it.copy(modelPickerPinned = AuthManager.getPinnedModels()) } - syncCurrentModelCapabilities() - } - } - } + fun refreshModelOptions() = modelSwitchDelegate.refreshModelOptions() - /** - * Open the in-session model picker. Uses the preloaded options if available - * (instant open); otherwise shows a loading state and fetches them. The - * `/model` slash command is the supported session hot-swap mechanism per the - * backend contract (issue #589). - */ - fun openModelPicker() { - val hasCached = cachedModelOptions.isNotEmpty() - _uiState.update { - it.copy( - showModelPicker = true, - modelPickerProviders = if (hasCached) cachedModelOptions else emptyList(), - modelPickerPinned = AuthManager.getPinnedModels(), - modelPickerLoading = !hasCached, - ) - } - if (!hasCached) { - refreshModelOptions() - } - } - - /** Re-fetch options (pull-to-refresh style) when the picker is already open. */ - fun refreshModelOptions() { - _uiState.update { it.copy(modelPickerLoading = true) } - viewModelScope.launch(ioDispatcher) { - val result = - safeApiCall { - ApiClient.hermesApi.getModelOptions(refresh = true) - } - when (result) { - is NetworkResult.Success -> { - cachedModelOptions = result.data.providers.orEmpty() - _uiState.update { - it.copy( - modelPickerProviders = cachedModelOptions, - modelPickerLoading = false, - ) - } - syncCurrentModelCapabilities() - } - - is NetworkResult.Failure -> { - _uiState.update { - it.copy( - modelPickerLoading = false, - errorMessage = "Failed to load models: ${result.error.message}", - ) - } - } - } - } - } - - fun closeModelPicker() { - _uiState.update { it.copy(showModelPicker = false, modelPickerLoading = false) } - } + fun closeModelPicker() = modelSwitchDelegate.closeModelPicker() fun togglePinModel( providerSlug: String, modelName: String, - ) { - val currentPinned = AuthManager.getPinnedModels().toMutableList() - val target = PinnedModel(providerSlug, modelName) - if (currentPinned.contains(target)) { - currentPinned.remove(target) - } else { - currentPinned.add(target) - } - AuthManager.savePinnedModels(currentPinned) - _uiState.update { it.copy(modelPickerPinned = currentPinned) } - } + ) = modelSwitchDelegate.togglePinModel(providerSlug, modelName) - /** - * Hot-swap the CURRENT session's model via the /model slash command. - * - * Builds the backend-valid command form `/model --provider - * --session`. The `--session` flag keeps the switch scoped to this chat - * only (it writes a per-session override and never touches the global - * model config), per the backend model-switch contract. - */ fun sendSlashModel( provider: String, model: String, - ) { - optimisticPreviousModel = _uiState.value.currentSessionModel - _uiState.update { - it.copy( - showModelPicker = false, - modelPickerLoading = false, - // Optimistic: reflect the chosen model in the top-bar chip until - // the next session sync confirms the backend hot-swap. - currentSessionModel = "$provider/$model", - ) - } - syncCurrentModelCapabilities() - // Model switch changes the context-window denominator — refetch it. - fetchContextUsage() - handleSlashCommand("/model $model --provider $provider --session") - } + ) = modelSwitchDelegate.sendSlashModel(provider, model) - fun dismissModelSwitchConfirm() { - val pending = activeModelSwitchConfirmation - activeModelSwitchConfirmation = null - _uiState.update { - it.copy( - modelSwitchConfirmMessage = null, - currentSessionModel = pending?.previousModel ?: it.currentSessionModel, - ) - } - syncCurrentModelCapabilities() - fetchContextUsage() - } + fun dismissModelSwitchConfirm() = modelSwitchDelegate.dismissModelSwitchConfirm() - fun confirmModelSwitchExpensive() { - val pending = activeModelSwitchConfirmation - activeModelSwitchConfirmation = null - _uiState.update { it.copy(modelSwitchConfirmMessage = null) } - if (pending != null) { - handleModelSwitch(pending.spec, confirmExpensive = true) - } - } + fun confirmModelSwitchExpensive() = modelSwitchDelegate.confirmModelSwitchExpensive() - /** - * Set the reasoning effort level for the current session. - * - * Updates the UI optimistically and sends a `config.set` RPC to the - * backend. The level applies per-session via the runtime session ID. - * If [level] is null it resets to the model's default. - * - * @param level One of "low", "medium", "high", or null for default. - */ - fun setReasoningLevel(level: String?) { - _uiState.update { it.copy(reasoningLevel = level) } - val sessionId = runtimeSessionId ?: return - if (level == null) return // null = model default, no need to send WS - viewModelScope.launch(ioDispatcher) { - wsClient.send( - WsMethods.CONFIG_SET, - mapOf( - "key" to "reasoning", - "value" to level, - "session_id" to sessionId, - ), - onSent = { id -> trackRequest(id, WsMethods.CONFIG_SET) }, - ) - } - } + fun setReasoningLevel(level: String?) = modelSwitchDelegate.setReasoningLevel(level) fun getModelCapabilities( providerSlug: String, modelName: String, - ): ModelCapabilities? = cachedModelOptions.find { it.slug == providerSlug }?.capabilities?.get(modelName) + ): ModelCapabilities? = modelSwitchDelegate.getModelCapabilities(providerSlug, modelName) - fun getCurrentModelCapabilities(): ModelCapabilities? { - val label = _uiState.value.currentSessionModel ?: return null - val idx = label.indexOf('/') - if (idx <= 0) return null - val provider = label.substring(0, idx) - val model = label.substring(idx + 1) - return getModelCapabilities(provider, model) - } - - private fun syncCurrentModelCapabilities() { - val label = _uiState.value.currentSessionModel - val caps = - if (label == null) { - null - } else { - val idx = label.indexOf('/') - if (idx <= 0) { - null - } else { - val provider = label.substring(0, idx) - val model = label.substring(idx + 1) - cachedModelOptions.find { it.slug == provider }?.capabilities?.get(model) - } - } - _uiState.update { state -> - if (state.currentModelCapabilities != caps) state.copy(currentModelCapabilities = caps) else state - } - } + fun getCurrentModelCapabilities(): ModelCapabilities? = modelSwitchDelegate.getCurrentModelCapabilities() fun switchSession(sessionId: String) { if (sessionId == _uiState.value.currentSessionId) return @@ -3762,332 +3508,17 @@ class ChatViewModel( // ── Approval flow ─────────────────────────────────────────────────── - private fun handleApprovalRequest(event: WsEvent.ApprovalRequest) { - val description = event.description ?: event.command ?: "Unknown command" - val content = "**Approval Required**\n$description" - val msg = - ChatMessage( - role = MessageRole.SYSTEM, - content = content, - approvalInfo = - ApprovalInfo( - command = event.command, - description = event.description, - patternKeys = event.patternKeys, - requestId = event.requestId, - choices = event.choices, - allowPermanent = event.allowPermanent, - smartDenied = event.smartDenied, - ), - ) - _uiState.update { state -> - state.copy( - messages = state.messages + msg, - isAgentTyping = false, - ) - } - // Desktop parity (`prompts.ts` receiveApprovalRequest): ack the render - // so the backend knows this client holds the prompt. Fire-and-forget. - val requestId = event.requestId - val sessionId = runtimeSessionId ?: event.sessionId ?: _uiState.value.currentSessionId - if (requestId != null && sessionId != null) { - viewModelScope.launch(ioDispatcher) { - runCatching { - wsClient.send( - method = WsMethods.APPROVAL_RECEIVED, - params = - mapOf( - "session_id" to sessionId, - "request_id" to requestId, - ), - ) - } - } - } - } - - /** - * Convert a backend approval map (snake_case, from `approval.pending` or - * `session.info` `pending_approval`) into a typed event. Mirrors - * [EventParser] defaults so replay and live paths agree. - */ - private fun parseApprovalMap( - map: Map<*, *>, - sessionId: String?, - ): WsEvent.ApprovalRequest { - @Suppress("UNCHECKED_CAST") - val patternKeys = (map["pattern_keys"] as? List<*>)?.filterIsInstance() - - @Suppress("UNCHECKED_CAST") - val rawChoices = (map["choices"] as? List<*>)?.filterIsInstance() - val allowPermanent = map["allow_permanent"] as? Boolean - val allowSession = map["allow_session"] as? Boolean - val smartDenied = map["smart_denied"] as? Boolean - val choices = - rawChoices ?: run { - if (smartDenied == true) { - listOf("once", "deny") - } else { - buildList { - add("once") - if (allowSession != false) { - add("session") - if (allowPermanent != false) add("always") - } - add("deny") - } - } - } - return WsEvent.ApprovalRequest( - command = map["command"] as? String, - description = map["description"] as? String, - patternKeys = patternKeys, - sessionId = sessionId, - requestId = map["request_id"] as? String, - choices = choices, - allowPermanent = allowPermanent, - smartDenied = smartDenied, - ) - } - - /** - * Reconnect replay (desktop `replayPendingApproval` parity): ask the - * gateway for unresolved approvals and surface the oldest one. Called - * after resume and after each respond (the queue can hold more). - */ - private fun replayPendingApproval(sessionId: String) { - val targetSessionId = runtimeSessionId ?: sessionId - viewModelScope.launch(ioDispatcher) { - wsClient.send( - method = WsMethods.APPROVAL_PENDING, - params = mapOf("session_id" to targetSessionId), - onSent = { id -> trackRequest(id, WsMethods.APPROVAL_PENDING) }, - ) - } - } - - private fun handleApprovalPendingResult(result: Any?) { - @Suppress("UNCHECKED_CAST") - val approvals = (result as? Map<*, *>)?.get("approvals") as? List<*> - val first = approvals?.filterIsInstance>()?.firstOrNull() ?: return - val requestId = first["request_id"] as? String ?: return - val sessionId = runtimeSessionId ?: _uiState.value.currentSessionId - // Don't duplicate a prompt already on screen. - val alreadyShown = - _uiState.value.messages.any { it.approvalInfo?.requestId == requestId } - if (alreadyShown) return - handleApprovalRequest(parseApprovalMap(first, sessionId)) - } - - fun respondToApproval(action: String) { - val state = _uiState.value - val approvalMsg = state.messages.lastOrNull { it.approvalInfo != null } ?: return - val sessionId = runtimeSessionId ?: state.currentSessionId ?: return - // Desktop sends `once` for a single run; legacy mobile sent `approve` - // (any non-deny still unblocks, but stay on-spec going forward). - val choice = if (action == "approve") "once" else action - val requestId = approvalMsg.approvalInfo?.requestId - - // Clear buttons immediately - _uiState.update { s -> - s.copy( - messages = - s.messages.map { - if (it.id == approvalMsg.id) { - it.copy(approvalInfo = null) - } else { - it - } - }, - ) - } - - viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "choice" to choice, - "all" to false, - ) - if (requestId != null) params["request_id"] = requestId - wsClient.send( - method = WsMethods.APPROVAL_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.APPROVAL_RESPOND) }, - ) - // The queue can hold more pendings — surface the next one. - replayPendingApproval(sessionId) - } - } + fun respondToApproval(action: String) = approvalsDelegate.respondToApproval(action) // ── Sudo / secret prompt flow (issue #524) ────────────────────────── - /** - * The agent needs the user's sudo password. Previously dropped → agent - * hung forever. Now we surface a secure dialog and reply via sudo.respond. - */ - private fun handleSudoRequest(event: WsEvent.SudoRequest) { - _uiState.update { - it.copy( - sudoPrompt = SudoPromptUi(event.requestId, event.sessionId), - isAgentTyping = false, - ) - } - } + fun dismissSudo() = credentialPromptsDelegate.dismissSudo() - /** - * Backend sudo timeout (120s) — clear only the matching dialog so a - * late expire for an old prompt never kills the current one. - * Desktop parity: `patchOverlayState(prev => prev.sudo?.requestId === id ? null : prev)`. - */ - private fun handleSudoExpire(event: WsEvent.SudoExpire) { - _uiState.update { state -> - val current = state.sudoPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { - return@update state - } - state.copy(sudoPrompt = null) - } - } + fun dismissSecret() = credentialPromptsDelegate.dismissSecret() - /** - * The agent needs a secret value (token/password). Previously dropped → - * agent hung forever. Now we surface a secure dialog and reply via - * secret.respond. - */ - private fun handleSecretRequest(event: WsEvent.SecretRequest) { - _uiState.update { - it.copy( - secretPrompt = - SecretPromptUi( - event.requestId, - event.sessionId, - event.envVar, - event.prompt, - ), - isAgentTyping = false, - ) - } - } + fun respondToSudo(password: String) = credentialPromptsDelegate.respondToSudo(password) - /** - * Backend secret timeout — match-only clear like [handleSudoExpire]. - */ - private fun handleSecretExpire(event: WsEvent.SecretExpire) { - _uiState.update { state -> - val current = state.secretPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { - return@update state - } - state.copy(secretPrompt = null) - } - } - - /** - * Cancel → send empty password (desktop parity: - * `prompt-overlays.tsx` `send('')`). Backend treats empty sudo as - * failed sudo (no command runs), so closing the dialog is a safe - * refusal that unblocks the turn instantly instead of hanging 120s - * until `sudo.expire`. - */ - fun dismissSudo() { - val prompt = _uiState.value.sudoPrompt ?: return - val sessionId = prompt.sessionId ?: _uiState.value.currentSessionId - _uiState.update { it.copy(sudoPrompt = null) } - if (sessionId == null) return - viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "password" to "", - ) - prompt.requestId?.let { id -> params["request_id"] = id } - wsClient.send( - method = WsMethods.SUDO_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.SUDO_RESPOND) }, - ) - } - } - - /** - * Cancel → send empty value (desktop parity). Backend `secret_cb` - * returns `skipped=True` on empty, unblocking the turn instantly. - */ - fun dismissSecret() { - val prompt = _uiState.value.secretPrompt ?: return - val sessionId = prompt.sessionId ?: _uiState.value.currentSessionId - _uiState.update { it.copy(secretPrompt = null) } - if (sessionId == null) return - viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "value" to "", - ) - prompt.requestId?.let { id -> params["request_id"] = id } - wsClient.send( - method = WsMethods.SECRET_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.SECRET_RESPOND) }, - ) - } - } - - /** - * Send the user's sudo password back to the gateway. Mirrors - * respondToApproval: clear the prompt immediately, then fire the RPC. - */ - fun respondToSudo(password: String) { - val prompt = _uiState.value.sudoPrompt ?: return - val sessionId = prompt.sessionId ?: _uiState.value.currentSessionId ?: return - if (password.isBlank()) return - - _uiState.update { it.copy(sudoPrompt = null) } - - viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "password" to password, - ) - prompt.requestId?.let { id -> params["request_id"] = id } - wsClient.send( - method = WsMethods.SUDO_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.SUDO_RESPOND) }, - ) - } - } - - /** - * Send the user's secret value back to the gateway. Mirrors respondToSudo. - */ - fun respondToSecret(value: String) { - val prompt = _uiState.value.secretPrompt ?: return - val sessionId = prompt.sessionId ?: _uiState.value.currentSessionId ?: return - if (value.isBlank()) return - - _uiState.update { it.copy(secretPrompt = null) } - - viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "value" to value, - ) - prompt.requestId?.let { id -> params["request_id"] = id } - wsClient.send( - method = WsMethods.SECRET_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.SECRET_RESPOND) }, - ) - } - } + fun respondToSecret(value: String) = credentialPromptsDelegate.respondToSecret(value) fun reconnect() { _uiState.update { @@ -4111,95 +3542,18 @@ class ChatViewModel( password: String, onResult: (Boolean, String?) -> Unit, ) { - viewModelScope.launch(ioDispatcher) { - val endpoint = AuthManager.endpointForBuild() - val jsonMediaType = "application/json; charset=utf-8".toMediaType() - val jsonBody = - JSONObject() - .put("provider", "basic") - .put("username", username) - .put("password", password) - .put("next", "") - .toString() - - try { - val loginClient = - com.m57.hermescontrol.data.remote.OkHttpProvider.probe - .newBuilder() - .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .build() - - val loginReq = - Request - .Builder() - .url(endpoint.resolve("auth/password-login").toString()) - .header("Content-Type", "application/json") - .post(jsonBody.toRequestBody(jsonMediaType)) - .build() - loginClient.newCall(loginReq).execute().use { loginResp -> - if (!loginResp.isSuccessful) { - val msg = - when (loginResp.code) { - 401 -> "Invalid username or password (401)" - 403 -> "Forbidden (403)" - else -> "HTTP error code: ${loginResp.code}" - } - withContext(Dispatchers.Main) { - onResult(false, msg) - } - return@launch - } - } - - val ticketClient = - com.m57.hermescontrol.data.remote.OkHttpProvider.base - .newBuilder() - .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .build() - - val ticketReq = - Request - .Builder() - .url(endpoint.resolve("api/auth/ws-ticket").toString()) - .post("{}".toRequestBody(jsonMediaType)) - .build() - ticketClient.newCall(ticketReq).execute().use { ticketResp -> - if (!ticketResp.isSuccessful) { - withContext(Dispatchers.Main) { - onResult(false, "Failed to mint WS ticket: HTTP ${ticketResp.code}") - } - return@launch - } - - val body = ticketResp.body.string() - val ticket = JSONObject(body).optString("ticket").takeIf { it.isNotBlank() } - - if (ticket.isNullOrBlank()) { - withContext(Dispatchers.Main) { - onResult(false, "Invalid ticket returned from server") - } - return@launch - } - - AuthManager.setWsAuthParam("ticket") - AuthManager.setToken(ticket) - - withContext(Dispatchers.Main) { - onResult(true, null) - reconnect() - } - } - } catch (e: java.io.IOException) { - withContext(Dispatchers.Main) { - onResult(false, "Connection failed: ${e.message}") - } - } catch (e: org.json.JSONException) { - withContext(Dispatchers.Main) { - onResult(false, "Connection failed: ${e.message}") - } - } + viewModelScope.launch { + reloginAuthenticator.relogin( + username = username, + password = password, + onSuccess = { + onResult(true, null) + reconnect() + }, + onFailure = { msg -> + onResult(false, msg) + }, + ) } } From 7b198fa6d61a9ab1b541a6ea5b949ff772082fa3 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 00:21:06 +0300 Subject: [PATCH 7/7] fix(chat): restore pending approval guard, tighten encapsulation, drop dormant tracker --- .../ui/chat/ChatApprovalsDelegate.kt | 6 +- .../ui/chat/ChatModelSwitchDelegate.kt | 10 +- .../ui/chat/ChatRpcRequestTracker.kt | 111 ------------------ .../hermescontrol/ui/chat/ChatViewModel.kt | 5 - .../ui/chat/ChatApprovalsDelegateTest.kt | 30 +++++ .../ui/chat/ChatRpcRequestTrackerTest.kt | 94 --------------- 6 files changed, 40 insertions(+), 216 deletions(-) delete mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt delete mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt index 99c3094c..0144d71b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt @@ -81,8 +81,12 @@ class ChatApprovalsDelegate( @Suppress("UNCHECKED_CAST") val approvals = (result as? Map<*, *>)?.get("approvals") as? List<*> val first = approvals?.filterIsInstance>()?.firstOrNull() ?: return + val requestId = first["request_id"] as? String ?: return val sessionId = runtimeSessionId() ?: uiState.value.currentSessionId - maybeSurfacePendingApproval(first, sessionId) + val alreadyShown = + uiState.value.messages.any { it.approvalInfo?.requestId == requestId } + if (alreadyShown) return + handleApprovalRequest(parseApprovalMap(first, sessionId)) } fun handleApprovalRespondResult(result: Any?) { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt index 44e2996c..b295f6ef 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt @@ -19,7 +19,7 @@ import java.util.concurrent.ConcurrentHashMap * Tracks an in-flight `config.set key=model` RPC and the model it would replace * so confirmation dialog dismiss can roll back optimistically. */ -data class ActiveModelSwitch( +private data class ActiveModelSwitch( val spec: String, val previousModel: String?, ) @@ -47,10 +47,10 @@ class ChatModelSwitchDelegate( private val getPinnedModels: () -> List = { AuthManager.getPinnedModels() }, private val savePinnedModels: (List) -> Unit = { AuthManager.savePinnedModels(it) }, ) { - val pendingModelSwitchRequests = ConcurrentHashMap() - var activeModelSwitchConfirmation: ActiveModelSwitch? = null - var optimisticPreviousModel: String? = null - var cachedModelOptions: List = emptyList() + private val pendingModelSwitchRequests = ConcurrentHashMap() + private var activeModelSwitchConfirmation: ActiveModelSwitch? = null + private var optimisticPreviousModel: String? = null + private var cachedModelOptions: List = emptyList() fun isModelPickerCommand(command: String): Boolean { val trimmed = command.trim() diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt deleted file mode 100644 index 8492ab54..00000000 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTracker.kt +++ /dev/null @@ -1,111 +0,0 @@ -package com.m57.hermescontrol.ui.chat - -import java.util.concurrent.ConcurrentHashMap - -/** - * Tracks an in-flight RPC request associated with a specific session generation - * and resume sequence to guard against stale results/errors from previous sessions. - */ -data class TrackedSessionRequest( - val generation: Long, - val resumeSequence: Long = 0L, - val sessionId: String? = null, -) - -/** - * Owns RPC id-to-method mapping, session request generations, hydration sequences, - * and staleness detection. - * - * Extracted behavior-preservingly from [ChatViewModel] to ensure deterministic - * RPC tracking across session creation, switches, resumes, and reconnects. - */ -class ChatRpcRequestTracker( - private val getCurrentSessionId: () -> String?, -) { - /** Maps an in-flight RPC id to its method for UI error labeling. */ - private val idToMethod = ConcurrentHashMap() - private val sessionRequestById = ConcurrentHashMap() - - var sessionGeneration: Long = 0L - private set - - var resumeRequestSequence: Long = 0L - private set - var activeResumeRequestSequence: Long = 0L - private set - - var hydrationRequestSequence: Long = 0L - private set - var activeHydrationRequestSequence: Long = 0L - private set - - var resumedGeneration: Long = -1L - var hydratedGeneration: Long = -1L - - fun nextSessionGeneration(): Long { - sessionGeneration++ - return sessionGeneration - } - - fun nextResumeSequence(): Long { - resumeRequestSequence++ - activeResumeRequestSequence = resumeRequestSequence - return resumeRequestSequence - } - - fun nextHydrationSequence(): Long { - hydrationRequestSequence++ - activeHydrationRequestSequence = hydrationRequestSequence - return hydrationRequestSequence - } - - fun trackRequest( - id: String, - method: String, - ) { - idToMethod[id] = method - } - - fun trackSessionRequest( - id: String, - method: String, - generation: Long, - resumeSequence: Long = 0L, - sessionId: String? = null, - ) { - sessionRequestById[id] = TrackedSessionRequest(generation, resumeSequence, sessionId) - trackRequest(id, method) - } - - fun isCurrentSessionRequest( - sessionId: String, - generation: Long, - ): Boolean = generation == sessionGeneration && sessionId == getCurrentSessionId() - - fun isCurrentHydration( - sessionId: String, - generation: Long, - requestSequence: Long, - ): Boolean = requestSequence == activeHydrationRequestSequence && isCurrentSessionRequest(sessionId, generation) - - fun isStaleSessionRequest(id: String): Boolean = sessionRequestById[id]?.let(::isStaleSessionRequest) == true - - fun isStaleSessionRequest(request: TrackedSessionRequest): Boolean = - request.generation != sessionGeneration || - (request.sessionId != null && request.sessionId != getCurrentSessionId()) || - (request.resumeSequence != 0L && request.resumeSequence != activeResumeRequestSequence) - - fun removeMethod(id: String): String? = idToMethod.remove(id) - - fun removeSessionRequest(id: String): TrackedSessionRequest? = sessionRequestById.remove(id) - - fun forgetRequest(id: String) { - idToMethod.remove(id) - sessionRequestById.remove(id) - } - - fun resetSessionStateTracking() { - resumedGeneration = -1L - hydratedGeneration = -1L - } -} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index bdf18ffa..828a53a9 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -510,11 +510,6 @@ class ChatViewModel( private val sessionRequestById = ConcurrentHashMap() private var sessionGeneration = 0L - private data class ActiveModelSwitch( - val spec: String, - val previousModel: String?, - ) - private var resumeRequestSequence = 0L private var activeResumeRequestSequence = 0L private var hydrationRequestSequence = 0L diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt index 4f3d298d..6623c4bf 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt @@ -161,6 +161,36 @@ class ChatApprovalsDelegateTest { ) } + @Test + fun handleApprovalPendingResult_dropsMissingRequestId_andDedupesAlreadyShown() { + val initialCount = uiState.value.messages.size + + // No request_id -> dropped entirely + delegate.handleApprovalPendingResult( + mapOf("approvals" to listOf(mapOf("command" to "rm -rf /"))), + ) + assertEquals(initialCount, uiState.value.messages.size) + + // Valid request_id -> surfaced + delegate.handleApprovalPendingResult( + mapOf("approvals" to listOf(mapOf("command" to "git pull", "request_id" to "r-valid"))), + ) + assertEquals(initialCount + 1, uiState.value.messages.size) + assertEquals( + "r-valid", + uiState.value.messages + .last() + .approvalInfo + ?.requestId, + ) + + // Duplicate request_id -> ignored + delegate.handleApprovalPendingResult( + mapOf("approvals" to listOf(mapOf("command" to "git pull", "request_id" to "r-valid"))), + ) + assertEquals(initialCount + 1, uiState.value.messages.size) + } + @Test fun handleApprovalRespondResult_appendsDesktopParitySystemMessage() { delegate.handleApprovalRespondResult(mapOf("resolved" to 1)) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt deleted file mode 100644 index 78eac772..00000000 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatRpcRequestTrackerTest.kt +++ /dev/null @@ -1,94 +0,0 @@ -package com.m57.hermescontrol.ui.chat - -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue -import org.junit.Test - -class ChatRpcRequestTrackerTest { - private var currentSessionId: String? = "session-a" - private val tracker = ChatRpcRequestTracker { currentSessionId } - - @Test - fun trackRequest_andRemoveMethod() { - tracker.trackRequest("req-1", "session.create") - assertEquals("session.create", tracker.removeMethod("req-1")) - assertNull(tracker.removeMethod("req-1")) - } - - @Test - fun generationIncrement_detectsStaleGenerations() { - val gen1 = tracker.nextSessionGeneration() - assertEquals(1L, gen1) - - tracker.trackSessionRequest( - id = "req-resume-1", - method = "session.resume", - generation = gen1, - sessionId = "session-a", - ) - assertFalse(tracker.isStaleSessionRequest("req-resume-1")) - - val gen2 = tracker.nextSessionGeneration() - assertEquals(2L, gen2) - assertTrue(tracker.isStaleSessionRequest("req-resume-1")) - } - - @Test - fun sessionMismatch_marksRequestStale() { - val gen = tracker.nextSessionGeneration() - tracker.trackSessionRequest( - id = "req-2", - method = "session.resume", - generation = gen, - sessionId = "session-a", - ) - assertFalse(tracker.isStaleSessionRequest("req-2")) - - currentSessionId = "session-b" - assertTrue(tracker.isStaleSessionRequest("req-2")) - } - - @Test - fun resumeSequence_marksSupersededResumeStale() { - val gen = tracker.nextSessionGeneration() - val seq1 = tracker.nextResumeSequence() - tracker.trackSessionRequest( - id = "req-resume-seq1", - method = "session.resume", - generation = gen, - resumeSequence = seq1, - sessionId = "session-a", - ) - assertFalse(tracker.isStaleSessionRequest("req-resume-seq1")) - - val seq2 = tracker.nextResumeSequence() - assertEquals(2L, seq2) - assertTrue(tracker.isStaleSessionRequest("req-resume-seq1")) - } - - @Test - fun hydrationSequence_guardsActiveHydration() { - val gen = tracker.nextSessionGeneration() - val hyd1 = tracker.nextHydrationSequence() - assertTrue(tracker.isCurrentHydration("session-a", gen, hyd1)) - - val hyd2 = tracker.nextHydrationSequence() - assertFalse(tracker.isCurrentHydration("session-a", gen, hyd1)) - assertTrue(tracker.isCurrentHydration("session-a", gen, hyd2)) - } - - @Test - fun forgetRequest_removesBothTrackingEntries() { - tracker.trackSessionRequest( - id = "req-forget", - method = "session.resume", - generation = tracker.sessionGeneration, - sessionId = "session-a", - ) - tracker.forgetRequest("req-forget") - assertNull(tracker.removeMethod("req-forget")) - assertNull(tracker.removeSessionRequest("req-forget")) - } -}