From 717b5b0361224a0e7dd98b170cd9c0b7d4a057e3 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:51:02 +0300 Subject: [PATCH] refactor(#1375): typed params for approval.* and session.active_list (#1385) --- .../data/ws/SessionLiveStatusSource.kt | 8 +- .../data/ws/contract/Approvals.kt | 28 +++ .../data/ws/contract/RpcMethod.kt | 44 ++++ .../ui/chat/ChatApprovalsDelegate.kt | 42 ++-- .../hermescontrol/ui/chat/ChatViewModel.kt | 11 +- .../data/ws/HermesWsClientApprovalsTest.kt | 237 ++++++++++++++++++ .../data/ws/RawRpcParamsRatchetTest.kt | 4 - .../data/ws/SessionLiveStatusSourceTest.kt | 14 +- .../data/ws/contract/ApprovalsTypesTest.kt | 209 +++++++++++++++ .../ui/chat/ChatApprovalsDelegateTest.kt | 51 ++-- .../ui/chat/ChatViewModelTest.kt | 26 +- 11 files changed, 611 insertions(+), 63 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/data/ws/contract/Approvals.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientApprovalsTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/contract/ApprovalsTypesTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSource.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSource.kt index 31cc88b4..9d8964c7 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSource.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSource.kt @@ -3,6 +3,8 @@ package com.m57.hermescontrol.data.ws import com.m57.hermescontrol.data.model.ActiveSessionsResponse import com.m57.hermescontrol.data.model.LiveSessionSnapshot import com.m57.hermescontrol.data.model.SessionLiveStatus +import com.m57.hermescontrol.data.ws.contract.RpcMethods +import com.m57.hermescontrol.data.ws.contract.SessionActiveListParams import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow @@ -148,8 +150,8 @@ object SessionLiveStatusDecoder { * Production implementation of [SessionLiveStatusSource] backed by [HermesWsClient]. */ class HermesSessionLiveStatusSource( - private val rpcRequest: suspend (method: String, params: Map) -> Any? = { method, params -> - HermesWsClient.request(method, params).await() + private val rpcRequest: suspend () -> Any? = { + HermesWsClient.call(RpcMethods.SESSION_ACTIVE_LIST, SessionActiveListParams()) }, eventsProvider: () -> Flow = { HermesWsClient.events }, connectionStatusProvider: () -> StateFlow = { HermesWsClient.connectionStatus }, @@ -159,7 +161,7 @@ class HermesSessionLiveStatusSource( override suspend fun fetchActiveSessionsSnapshot(): LiveSessionSnapshot? = try { - val raw = rpcRequest(WsMethods.SESSION_ACTIVE_LIST, emptyMap()) + val raw = rpcRequest() SessionLiveStatusDecoder.decodeSnapshot(raw) } catch (e: CancellationException) { throw e diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/Approvals.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/Approvals.kt new file mode 100644 index 00000000..d66920eb --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/Approvals.kt @@ -0,0 +1,28 @@ +package com.m57.hermescontrol.data.ws.contract + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +data class ApprovalPendingParams( + @SerialName("session_id") val sessionId: String, +) + +@Serializable +data class ApprovalReceivedParams( + @SerialName("session_id") val sessionId: String, + @SerialName("request_id") val requestId: String, +) + +@Serializable +data class ApprovalRespondParams( + @SerialName("session_id") val sessionId: String, + @SerialName("choice") val choice: String? = null, + @SerialName("all") val all: Boolean? = null, + @SerialName("request_id") val requestId: String? = null, +) + +@Serializable +data class SessionActiveListParams( + @SerialName("current_session_id") val currentSessionId: String? = null, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt index 5b6c329d..3d817fce 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt @@ -10,6 +10,14 @@ class RpcMethod( val result: KSerializer, ) +interface TypedRpcSender { + fun

send( + method: RpcMethod, + params: P, + onSent: ((String) -> Unit)? = null, + ): String +} + object RpcMethods { val SESSION_EVENTS_SINCE: RpcMethod = RpcMethod( @@ -110,6 +118,38 @@ object RpcMethods { PromptBtwResult.serializer(), ) + // Passthrough JsonElement results: callers keep their existing legacy parsers. + val APPROVAL_PENDING: RpcMethod = + RpcMethod( + WsMethods.APPROVAL_PENDING, + ApprovalPendingParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy parsers. + val APPROVAL_RECEIVED: RpcMethod = + RpcMethod( + WsMethods.APPROVAL_RECEIVED, + ApprovalReceivedParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy parsers. + val APPROVAL_RESPOND: RpcMethod = + RpcMethod( + WsMethods.APPROVAL_RESPOND, + ApprovalRespondParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy parsers. + val SESSION_ACTIVE_LIST: RpcMethod = + RpcMethod( + WsMethods.SESSION_ACTIVE_LIST, + SessionActiveListParams.serializer(), + JsonElement.serializer(), + ) + /** Every typed method. GatewayContractTest iterates this; future migrations append here. */ val all: List> = listOf( @@ -127,5 +167,9 @@ object RpcMethods { SESSION_CONTEXT_BREAKDOWN, SESSION_USAGE, PROMPT_BTW, + APPROVAL_PENDING, + APPROVAL_RECEIVED, + APPROVAL_RESPOND, + SESSION_ACTIVE_LIST, ) } 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 32d68d66..313e57de 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 @@ -2,6 +2,11 @@ package com.m57.hermescontrol.ui.chat import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ApprovalPendingParams +import com.m57.hermescontrol.data.ws.contract.ApprovalReceivedParams +import com.m57.hermescontrol.data.ws.contract.ApprovalRespondParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods +import com.m57.hermescontrol.data.ws.contract.TypedRpcSender import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow @@ -22,7 +27,7 @@ class ChatApprovalsDelegate( 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 rpc: TypedRpcSender, private val trackRequest: (id: String, method: String) -> Unit, private val addSystemMessage: (text: String) -> Unit, private val respondToServerRequest: ((String, JsonElement) -> Unit)? = null, @@ -99,13 +104,12 @@ class ChatApprovalsDelegate( if (event.serverRequestId == null && requestId != null && sessionId != null) { scope.launch(ioDispatcher) { runCatching { - wsSend( - WsMethods.APPROVAL_RECEIVED, - mapOf( - "session_id" to sessionId, - "request_id" to requestId, + rpc.send( + RpcMethods.APPROVAL_RECEIVED, + ApprovalReceivedParams( + sessionId = sessionId, + requestId = requestId, ), - null, ) } } @@ -115,9 +119,9 @@ class ChatApprovalsDelegate( fun replayPendingApproval(sessionId: String) { val targetSessionId = runtimeSessionId() ?: sessionId scope.launch(ioDispatcher) { - wsSend( - WsMethods.APPROVAL_PENDING, - mapOf("session_id" to targetSessionId), + rpc.send( + RpcMethods.APPROVAL_PENDING, + ApprovalPendingParams(sessionId = targetSessionId), ) { id -> trackRequest(id, WsMethods.APPROVAL_PENDING) } } } @@ -190,16 +194,14 @@ class ChatApprovalsDelegate( ) return@launch } - 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, + rpc.send( + RpcMethods.APPROVAL_RESPOND, + ApprovalRespondParams( + sessionId = sessionId, + choice = choice, + all = false, + requestId = requestId, + ), ) { id -> trackRequest(id, WsMethods.APPROVAL_RESPOND) } // The queue can hold more pendings — surface the next one. replayPendingApproval(sessionId) 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 91eb50aa..87ae2a2b 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 @@ -38,6 +38,7 @@ import com.m57.hermescontrol.data.ws.JsonRpcError import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods import com.m57.hermescontrol.data.ws.contract.PromptBtwParams +import com.m57.hermescontrol.data.ws.contract.RpcMethod import com.m57.hermescontrol.data.ws.contract.RpcMethods import com.m57.hermescontrol.data.ws.contract.SessionBranchParams import com.m57.hermescontrol.data.ws.contract.SessionBranchWholeParams @@ -48,6 +49,7 @@ import com.m57.hermescontrol.data.ws.contract.SessionIdParams import com.m57.hermescontrol.data.ws.contract.SessionInterruptParams import com.m57.hermescontrol.data.ws.contract.SessionListParams import com.m57.hermescontrol.data.ws.contract.SessionResumeParams +import com.m57.hermescontrol.data.ws.contract.TypedRpcSender import com.m57.hermescontrol.data.ws.toAny import com.m57.hermescontrol.data.ws.toJsonElement import com.m57.hermescontrol.notification.ReplyNotificationTracker @@ -711,7 +713,14 @@ class ChatViewModel( ioDispatcher = ioDispatcher, uiState = _uiState, runtimeSessionId = { runtimeSessionId }, - wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, + rpc = + object : TypedRpcSender { + override fun

send( + method: RpcMethod, + params: P, + onSent: ((String) -> Unit)?, + ): String = wsClient.send(method, params, onSent) + }, trackRequest = { id, method -> trackRequest(id, method) }, addSystemMessage = { text -> addSystemMessage(text) }, respondToServerRequest = { id, result -> wsClient.respondToServerRequest(id, result) }, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientApprovalsTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientApprovalsTest.kt new file mode 100644 index 00000000..8c6fd063 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientApprovalsTest.kt @@ -0,0 +1,237 @@ +package com.m57.hermescontrol.data.ws + +import android.util.Log +import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.remote.CookieManager +import com.m57.hermescontrol.data.remote.DashboardSessionTokenRefresher +import com.m57.hermescontrol.data.remote.OkHttpProvider +import com.m57.hermescontrol.data.remote.buildFakePersistentCookieJar +import com.m57.hermescontrol.data.ws.contract.ApprovalRespondParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods +import com.m57.hermescontrol.data.ws.contract.SessionActiveListParams +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.withTimeoutOrNull +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.booleanOrNull +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +class HermesWsClientApprovalsTest { + private lateinit var mockWebServer: MockWebServer + + @Before + fun setUp() { + mockkStatic(Log::class) + every { Log.d(any(), any()) } returns 0 + every { Log.i(any(), any()) } returns 0 + every { Log.w(any(), any()) } returns 0 + every { Log.w(any(), any(), any()) } returns 0 + every { Log.e(any(), any(), any()) } returns 0 + + mockWebServer = MockWebServer() + mockWebServer.start() + mockkObject(AuthManager) + every { AuthManager.wsUrl() } returns mockWebServer.url("/").toString().replace("http://", "ws://") + every { AuthManager.isAutoReconnect() } returns false + every { AuthManager.getSessionCookie() } returns null + every { AuthManager.serverStore } returns + mockk().also { + every { it.getLatestState() } returns + com.m57.hermescontrol.data.config + .ServerStoreState() + } + CookieManager.setJarForTest(buildFakePersistentCookieJar()) + mockkObject(DashboardSessionTokenRefresher) + coEvery { DashboardSessionTokenRefresher.refreshAsync() } returns null + every { DashboardSessionTokenRefresher.refresh() } returns null + + HermesWsClient.disconnect(clearPendingMessages = true) + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.setAppForeground(true) + AuthManager.resetAuthStateForTest() + } + + @After + fun tearDown() { + AuthManager.resetAuthStateForTest() + HermesWsClient.releaseExternalActivityConnectionLease() + HermesWsClient.releaseBackgroundConnectionLease() + HermesWsClient.disconnect(clearPendingMessages = true) + runBlocking { + withTimeoutOrNull(5000) { + HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED } + } + } + runCatching { mockWebServer.shutdown() } + unmockkAll() + } + + private fun connectClient() { + HermesWsClient.connect() + runBlocking { + withTimeout(5000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } + } + } + + @Test + fun typedSendApprovalRespondPutsExactParamsOnWire() { + val opened = CountDownLatch(1) + val captured = CountDownLatch(1) + val frameRef = AtomicReference(null) + + mockWebServer.enqueue( + MockResponse().withWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: Response, + ) = opened.countDown() + + override fun onMessage( + webSocket: WebSocket, + text: String, + ) { + val json = OkHttpProvider.json.parseToJsonElement(text) as? JsonObject ?: return + val name = (json["method"] as? JsonPrimitive)?.content + if (name == "approval.respond") { + frameRef.set(json) + captured.countDown() + } + } + }, + ), + ) + + connectClient() + assertTrue(opened.await(5, TimeUnit.SECONDS)) + + var onSentId: String? = null + val returnedId = + HermesWsClient.send( + method = RpcMethods.APPROVAL_RESPOND, + params = + ApprovalRespondParams( + sessionId = "sess-appr-wire-1", + choice = "deny", + all = false, + requestId = "req-appr-wire-42", + ), + onSent = { id -> onSentId = id }, + ) + + assertTrue(returnedId.isNotBlank()) + assertEquals(returnedId, onSentId) + + assertTrue("Frame must reach the server", captured.await(5, TimeUnit.SECONDS)) + val frame = requireNotNull(frameRef.get()) + + assertFalse("Outgoing frames have NO jsonrpc field", frame.containsKey("jsonrpc")) + assertEquals("approval.respond", (frame["method"] as? JsonPrimitive)?.content) + assertEquals(returnedId, (frame["id"] as? JsonPrimitive)?.content) + + val params = frame["params"] as? JsonObject + val expectedParams = + buildJsonObject { + put("session_id", "sess-appr-wire-1") + put("choice", "deny") + put("all", false) + put("request_id", "req-appr-wire-42") + } + assertEquals(expectedParams, params) + assertEquals(false, (params?.get("all") as? JsonPrimitive)?.booleanOrNull) + } + + @Test + fun typedCallSessionActiveListReturnsRawServerJsonUntouched() { + val opened = CountDownLatch(1) + val captured = CountDownLatch(1) + val frameRef = AtomicReference(null) + + val serverResultJson = + buildJsonObject { + put("active_count", 2) + put( + "sessions", + buildJsonObject { + put("primary", "sess-alpha") + put("secondary", "sess-beta") + }, + ) + } + + mockWebServer.enqueue( + MockResponse().withWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: Response, + ) = opened.countDown() + + override fun onMessage( + webSocket: WebSocket, + text: String, + ) { + val json = OkHttpProvider.json.parseToJsonElement(text) as? JsonObject ?: return + val name = (json["method"] as? JsonPrimitive)?.content + if (name == "session.active_list") { + frameRef.set(json) + captured.countDown() + val id = (json["id"] as? JsonPrimitive)?.content ?: "1" + webSocket.send( + """{"jsonrpc":"2.0","id":"$id","result":$serverResultJson}""", + ) + } + } + }, + ), + ) + + connectClient() + assertTrue(opened.await(5, TimeUnit.SECONDS)) + + val result: JsonElement = + runBlocking { + HermesWsClient.call( + RpcMethods.SESSION_ACTIVE_LIST, + SessionActiveListParams(), + ) + } + + assertTrue("Frame must reach the server", captured.await(5, TimeUnit.SECONDS)) + val frame = requireNotNull(frameRef.get()) + assertEquals("session.active_list", (frame["method"] as? JsonPrimitive)?.content) + + // An empty params object is omitted on wire or empty + val params = frame["params"] as? JsonObject + assertTrue( + "session.active_list with null currentSessionId carries no params", + params == null || params.isEmpty(), + ) + assertEquals(serverResultJson, result) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt index 37c39ff9..9e7a02d1 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt @@ -339,7 +339,6 @@ class RawRpcParamsRatchetTest { "data/ws/PluginManageRepository.kt::PLUGINS_MANAGE", "data/ws/PluginRemovalRepository.kt::PLUGINS_MANAGE", "data/ws/ProjectsSource.kt::PROJECTS_LIST", - "data/ws/SessionLiveStatusSource.kt::SESSION_ACTIVE_LIST", "data/ws/SubagentRepository.kt::SUBAGENT_LIST", "data/ws/SubagentRepository.kt::SUBAGENT_TAIL", "data/ws/VaultRepository.kt::VAULT_LIST", @@ -355,9 +354,6 @@ class RawRpcParamsRatchetTest { "ui/bots/group/GroupChatViewModel.kt::PROFILES_CONFIGURE", "ui/bots/group/GroupChatViewModel.kt::PROFILES_CONFIGURE", "ui/bots/group/GroupChatViewModel.kt::PROFILES_LIST", - "ui/chat/ChatApprovalsDelegate.kt::APPROVAL_PENDING", - "ui/chat/ChatApprovalsDelegate.kt::APPROVAL_RECEIVED", - "ui/chat/ChatApprovalsDelegate.kt::APPROVAL_RESPOND", "ui/chat/ChatConnectionOperationDelegate.kt::CONNECTION_RESPOND", "ui/chat/ChatConnectionOperationDelegate.kt::CONNECTION_RESPOND", "ui/chat/ChatConnectionOperationDelegate.kt::CONNECTORS_OPERATION_WAKE", diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSourceTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSourceTest.kt index 4a32ef87..a10d23dd 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSourceTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/SessionLiveStatusSourceTest.kt @@ -121,14 +121,12 @@ class SessionLiveStatusSourceTest { @Test fun `HermesSessionLiveStatusSource fetches and decodes snapshot via RPC`() = runBlocking { - var capturedMethod: String? = null - var capturedParams: Map? = null + var rpcCalls = 0 val source = HermesSessionLiveStatusSource( - rpcRequest = { method, params -> - capturedMethod = method - capturedParams = params + rpcRequest = { + rpcCalls++ mapOf( "sessions" to listOf( @@ -146,8 +144,8 @@ class SessionLiveStatusSourceTest { val snapshot = source.fetchActiveSessionsSnapshot() - assertEquals(WsMethods.SESSION_ACTIVE_LIST, capturedMethod) - assertTrue(capturedParams!!.isEmpty()) + // Method name and empty params are pinned on the wire in HermesWsClientApprovalsTest. + assertEquals(1, rpcCalls) assertNotNull(snapshot) assertEquals(SessionLiveStatus.WORKING, snapshot!!.statusByStoredId["st-1"]) } @@ -157,7 +155,7 @@ class SessionLiveStatusSourceTest { runBlocking { val source = HermesSessionLiveStatusSource( - rpcRequest = { _, _ -> throw RuntimeException("RPC error") }, + rpcRequest = { throw RuntimeException("RPC error") }, eventsProvider = { MutableSharedFlow() }, connectionStatusProvider = { MutableStateFlow(ConnectionStatus.CONNECTED) }, ) diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ApprovalsTypesTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ApprovalsTypesTest.kt new file mode 100644 index 00000000..2b419264 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ApprovalsTypesTest.kt @@ -0,0 +1,209 @@ +package com.m57.hermescontrol.data.ws.contract + +import com.m57.hermescontrol.data.remote.OkHttpProvider +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.decodeFromJsonElement +import kotlinx.serialization.json.encodeToJsonElement +import kotlinx.serialization.json.put +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class ApprovalsTypesTest { + @Test + fun approvalPendingParamsEncodesSessionIdExactly() { + val params = ApprovalPendingParams(sessionId = "sess-appr-pending-1") + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalPendingParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-appr-pending-1") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id"), (encoded as JsonObject).keys) + } + + @Test + fun approvalReceivedParamsEncodesSessionIdAndRequestId() { + val params = + ApprovalReceivedParams( + sessionId = "sess-appr-rec-1", + requestId = "req-appr-rec-99", + ) + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalReceivedParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-appr-rec-1") + put("request_id", "req-appr-rec-99") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id", "request_id"), (encoded as JsonObject).keys) + } + + @Test + fun approvalRespondParamsEncodesOnlySessionIdWhenOptionalsNull() { + val params = ApprovalRespondParams(sessionId = "sess-respond-1") + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalRespondParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-respond-1") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id"), (encoded as JsonObject).keys) + } + + @Test + fun approvalRespondParamsEncodesAllFieldsWhenPresent() { + val params = + ApprovalRespondParams( + sessionId = "sess-respond-2", + choice = "allow", + all = true, + requestId = "req-respond-2", + ) + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalRespondParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-respond-2") + put("choice", "allow") + put("all", true) + put("request_id", "req-respond-2") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id", "choice", "all", "request_id"), (encoded as JsonObject).keys) + } + + @Test + fun approvalRespondParamsEncodesExplicitFalseForAll() { + val params = + ApprovalRespondParams( + sessionId = "sess-respond-3", + choice = "deny", + all = false, + requestId = "req-respond-3", + ) + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalRespondParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-respond-3") + put("choice", "deny") + put("all", false) + put("request_id", "req-respond-3") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id", "choice", "all", "request_id"), (encoded as JsonObject).keys) + } + + @Test + fun approvalRespondParamsOmitsOnlyNullFields() { + val params = + ApprovalRespondParams( + sessionId = "sess-respond-4", + choice = "allow", + all = null, + requestId = null, + ) + val encoded = OkHttpProvider.json.encodeToJsonElement(ApprovalRespondParams.serializer(), params) + + val expected = + buildJsonObject { + put("session_id", "sess-respond-4") + put("choice", "allow") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("session_id", "choice"), (encoded as JsonObject).keys) + } + + @Test + fun sessionActiveListParamsEncodesToEmptyJsonObjectWhenCurrentSessionIdNull() { + val params = SessionActiveListParams() + val encoded = OkHttpProvider.json.encodeToJsonElement(SessionActiveListParams.serializer(), params) + + val expected = buildJsonObject {} + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertTrue((encoded as JsonObject).isEmpty()) + } + + @Test + fun sessionActiveListParamsEncodesCurrentSessionIdWhenSet() { + val params = SessionActiveListParams(currentSessionId = "sess-active-curr-1") + val encoded = OkHttpProvider.json.encodeToJsonElement(SessionActiveListParams.serializer(), params) + + val expected = + buildJsonObject { + put("current_session_id", "sess-active-curr-1") + } + + assertEquals(expected, encoded) + assertTrue(encoded is JsonObject) + assertEquals(setOf("current_session_id"), (encoded as JsonObject).keys) + } + + @Test + fun rpcMethodsRegistrationMatchesContract() { + assertEquals("approval.pending", RpcMethods.APPROVAL_PENDING.name) + assertEquals("approval.received", RpcMethods.APPROVAL_RECEIVED.name) + assertEquals("approval.respond", RpcMethods.APPROVAL_RESPOND.name) + assertEquals("session.active_list", RpcMethods.SESSION_ACTIVE_LIST.name) + + assertTrue(RpcMethods.all.contains(RpcMethods.APPROVAL_PENDING)) + assertTrue(RpcMethods.all.contains(RpcMethods.APPROVAL_RECEIVED)) + assertTrue(RpcMethods.all.contains(RpcMethods.APPROVAL_RESPOND)) + assertTrue(RpcMethods.all.contains(RpcMethods.SESSION_ACTIVE_LIST)) + } + + @Test + fun jsonElementPassthroughMethodsDecodeResultUntouched() { + val arbitraryPayload = + buildJsonObject { + put("status", "pending") + put("count", 3) + put("nested", buildJsonObject { put("detail", "value") }) + } + + val decodedPending: JsonElement = + OkHttpProvider.json.decodeFromJsonElement( + RpcMethods.APPROVAL_PENDING.result, + arbitraryPayload, + ) + val decodedReceived: JsonElement = + OkHttpProvider.json.decodeFromJsonElement( + RpcMethods.APPROVAL_RECEIVED.result, + arbitraryPayload, + ) + val decodedRespond: JsonElement = + OkHttpProvider.json.decodeFromJsonElement( + RpcMethods.APPROVAL_RESPOND.result, + arbitraryPayload, + ) + val decodedActiveList: JsonElement = + OkHttpProvider.json.decodeFromJsonElement( + RpcMethods.SESSION_ACTIVE_LIST.result, + arbitraryPayload, + ) + + assertEquals(arbitraryPayload, decodedPending) + assertEquals(arbitraryPayload, decodedReceived) + assertEquals(arbitraryPayload, decodedRespond) + assertEquals(arbitraryPayload, decodedActiveList) + } +} 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 bc797046..ec98fa19 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 @@ -1,7 +1,10 @@ package com.m57.hermescontrol.ui.chat +import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.RpcMethod +import com.m57.hermescontrol.data.ws.contract.TypedRpcSender import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.StandardTestDispatcher @@ -9,6 +12,8 @@ import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive import org.junit.Assert.assertEquals @@ -25,7 +30,7 @@ class ChatApprovalsDelegateTest { private val uiState = MutableStateFlow(ChatUiState(currentSessionId = "storage-session-1")) private var runtimeId: String? = null private val sentMethods = mutableListOf() - private val sentParams = mutableListOf>() + private val sentParams = mutableListOf() private val trackedRequests = mutableListOf>() private val systemMessages = mutableListOf() @@ -35,11 +40,22 @@ class ChatApprovalsDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { method, params, onSent -> - sentMethods.add(method) - sentParams.add(params) - onSent?.invoke("req-id-${sentMethods.size}") - }, + rpc = + object : TypedRpcSender { + override fun

send( + method: RpcMethod, + params: P, + onSent: ((String) -> Unit)?, + ): String { + sentMethods.add(method.name) + sentParams.add( + OkHttpProvider.json.encodeToJsonElement(method.params, params) as JsonObject, + ) + val id = "req-id-${sentMethods.size}" + onSent?.invoke(id) + return id + } + }, trackRequest = { id, method -> trackedRequests.add(id to method) }, addSystemMessage = { text -> systemMessages.add(text) }, ) @@ -98,8 +114,8 @@ class ChatApprovalsDelegateTest { 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"]) + assertEquals(JsonPrimitive("runtime-session-xyz"), sentParams.first()["session_id"]) + assertEquals(JsonPrimitive("req-123"), sentParams.first()["request_id"]) } @Test @@ -125,11 +141,11 @@ class ChatApprovalsDelegateTest { 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"]) + assertEquals(JsonPrimitive("once"), sentParams[0]["choice"]) + assertEquals(JsonPrimitive("req-999"), sentParams[0]["request_id"]) + assertEquals(JsonPrimitive(false), sentParams[0]["all"]) + assertEquals(JsonPrimitive("runtime-session-xyz"), sentParams[0]["session_id"]) + assertEquals(JsonPrimitive("runtime-session-xyz"), sentParams[1]["session_id"]) } @Test @@ -270,7 +286,14 @@ class ChatApprovalsDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { "session-1" }, - wsSend = { _, _, _ -> error("new approval must not use legacy response") }, + rpc = + object : TypedRpcSender { + override fun

send( + method: RpcMethod, + params: P, + onSent: ((String) -> Unit)?, + ): String = error("new approval must not use legacy response") + }, trackRequest = { _, _ -> }, addSystemMessage = { }, respondToServerRequest = { id, result -> responses += id to result }, diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index e69f29a0..a2a0be4b 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -6619,9 +6619,9 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_RESPOND, withArg { params -> - assertEquals(sessionId, params["session_id"]) - assertEquals("once", params["choice"]) - assertEquals("req-42", params["request_id"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) + assertEquals(JsonPrimitive("once"), params["choice"]) + assertEquals(JsonPrimitive("req-42"), params["request_id"]) }, any(), ) @@ -6653,8 +6653,8 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_RESPOND, withArg { params -> - assertEquals("session", params["choice"]) - assertEquals("req-7", params["request_id"]) + assertEquals(JsonPrimitive("session"), params["choice"]) + assertEquals(JsonPrimitive("req-7"), params["request_id"]) }, any(), ) @@ -6684,7 +6684,7 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_RESPOND, withArg { params -> - assertEquals("deny", params["choice"]) + assertEquals(JsonPrimitive("deny"), params["choice"]) assertFalse(params.containsKey("request_id")) }, any(), @@ -6717,7 +6717,7 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_PENDING, withArg { params -> - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) }, any(), ) @@ -6763,8 +6763,8 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_RECEIVED, withArg { params -> - assertEquals("runtime-sid-999", params["session_id"]) - assertEquals("req-runtime-test", params["request_id"]) + assertEquals(JsonPrimitive("runtime-sid-999"), params["session_id"]) + assertEquals(JsonPrimitive("req-runtime-test"), params["request_id"]) }, any(), ) @@ -6777,16 +6777,16 @@ class ChatViewModelTest { HermesWsClient.send( WsMethods.APPROVAL_RESPOND, withArg { params -> - assertEquals("runtime-sid-999", params["session_id"]) - assertEquals("once", params["choice"]) - assertEquals("req-runtime-test", params["request_id"]) + assertEquals(JsonPrimitive("runtime-sid-999"), params["session_id"]) + assertEquals(JsonPrimitive("once"), params["choice"]) + assertEquals(JsonPrimitive("req-runtime-test"), params["request_id"]) }, any(), ) HermesWsClient.send( WsMethods.APPROVAL_PENDING, withArg { params -> - assertEquals("runtime-sid-999", params["session_id"]) + assertEquals(JsonPrimitive("runtime-sid-999"), params["session_id"]) }, any(), )