diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/CredentialPoolAddRequest.kt b/app/src/main/java/com/m57/hermescontrol/data/model/CredentialPoolAddRequest.kt new file mode 100644 index 00000000..ec4f9fb9 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/model/CredentialPoolAddRequest.kt @@ -0,0 +1,11 @@ +package com.m57.hermescontrol.data.model + +import kotlinx.serialization.Serializable + +/** Body for POST api/credentials/pool. `apiKey` serializes as `api_key`. */ +@Serializable +data class CredentialPoolAddRequest( + val provider: String, + val apiKey: String, + val label: String? = null, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/DebugShareResponse.kt b/app/src/main/java/com/m57/hermescontrol/data/model/DebugShareResponse.kt index 14be3d2d..705c5212 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/DebugShareResponse.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/DebugShareResponse.kt @@ -9,3 +9,8 @@ data class DebugShareResponse( val redacted: Boolean? = null, val auto_delete_seconds: Long? = null, ) + +@Serializable +data class DebugShareRequest( + val redact: Boolean? = null, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/HookResponse.kt b/app/src/main/java/com/m57/hermescontrol/data/model/HookResponse.kt index 5f4f4b31..46ed5b8c 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/HookResponse.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/HookResponse.kt @@ -17,3 +17,19 @@ data class HookEntry( val approved_at: String? = null, val executable: Boolean? = null, ) + +/** Body for POST api/ops/hooks. The backend field is `approve` (writes the consent allowlist entry). */ +@Serializable +data class HookCreateRequest( + val event: String, + val command: String, + val matcher: String? = null, + val timeout: Int? = null, + val approve: Boolean? = null, +) + +@Serializable +data class HookDeleteRequest( + val event: String, + val command: String, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/McpServer.kt b/app/src/main/java/com/m57/hermescontrol/data/model/McpServer.kt index 9b671430..2e856c48 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/McpServer.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/McpServer.kt @@ -96,3 +96,9 @@ data class McpOAuthFlowResponse( val authorizationUrl: String? = null, val error: String? = null, ) + +/** Body for PUT api/mcp/servers/{name}; only the fields being changed are sent. */ +@Serializable +data class McpServerUpdateRequest( + val env: Map? = null, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/HermesApiService.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/HermesApiService.kt index a2a486c6..3e15cf62 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/HermesApiService.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/HermesApiService.kt @@ -20,10 +20,12 @@ import com.m57.hermescontrol.data.model.CreateCronJobRequest import com.m57.hermescontrol.data.model.CreateProfileRequest import com.m57.hermescontrol.data.model.CreateTaskBody import com.m57.hermescontrol.data.model.CreateWebhookRequest +import com.m57.hermescontrol.data.model.CredentialPoolAddRequest import com.m57.hermescontrol.data.model.CredentialPoolResponse import com.m57.hermescontrol.data.model.CronBlueprintListResponse import com.m57.hermescontrol.data.model.CronJob import com.m57.hermescontrol.data.model.CuratorResponse +import com.m57.hermescontrol.data.model.DebugShareRequest import com.m57.hermescontrol.data.model.DebugShareResponse import com.m57.hermescontrol.data.model.DeleteWebhookResponse import com.m57.hermescontrol.data.model.DeliveryTargetsResponse @@ -38,6 +40,8 @@ import com.m57.hermescontrol.data.model.EnvVarUpdate import com.m57.hermescontrol.data.model.GatewayMigrationPlan import com.m57.hermescontrol.data.model.GatewayMigrationStartResponse import com.m57.hermescontrol.data.model.HealthStatus +import com.m57.hermescontrol.data.model.HookCreateRequest +import com.m57.hermescontrol.data.model.HookDeleteRequest import com.m57.hermescontrol.data.model.HookResponse import com.m57.hermescontrol.data.model.InstantiateBlueprintRequest import com.m57.hermescontrol.data.model.KanbanBoardResponse @@ -57,6 +61,7 @@ import com.m57.hermescontrol.data.model.McpOAuthFlowResponse import com.m57.hermescontrol.data.model.McpServer import com.m57.hermescontrol.data.model.McpServerTestResponse import com.m57.hermescontrol.data.model.McpServerToggleRequest +import com.m57.hermescontrol.data.model.McpServerUpdateRequest import com.m57.hermescontrol.data.model.McpServersResponse import com.m57.hermescontrol.data.model.MemoryProviderConfigResponse import com.m57.hermescontrol.data.model.MemoryProviderConfigUpdateRequest @@ -583,7 +588,7 @@ interface HermesApiService : KanbanApiService { @PUT("api/mcp/servers/{name}") suspend fun updateMcpServer( @Path("name") name: String, - @Body body: Map, + @Body body: McpServerUpdateRequest, ): Response @POST("api/mcp/servers/{name}/auth") @@ -602,7 +607,7 @@ interface HermesApiService : KanbanApiService { @POST("api/mcp/catalog/install") suspend fun installMcpCatalogEntry( @Body body: McpCatalogInstallRequest, - ): Response> + ): Response @GET("api/webhooks") suspend fun getWebhooks(): Response @@ -881,7 +886,7 @@ interface HermesApiService : KanbanApiService { @PUT("api/curator/paused") suspend fun setCuratorPaused( @Body body: Map, - ): Response> + ): Response @POST("api/curator/run") suspend fun runCurator(): Response @@ -918,14 +923,14 @@ interface HermesApiService : KanbanApiService { @POST("api/credentials/pool") suspend fun addCredentialPoolEntry( - @Body body: Map, - ): Response> + @Body body: CredentialPoolAddRequest, + ): Response @DELETE("api/credentials/pool/{provider}/{index}") suspend fun removeCredentialPoolEntry( @Path("provider") provider: String, @Path("index") index: Int, - ): Response> + ): Response // ── Admin: Operations ───────────────────────────────────────────── @POST("api/ops/security-audit") @@ -968,7 +973,7 @@ interface HermesApiService : KanbanApiService { // ── Admin: Debug share ──────────────────────────────────────────── @POST("api/ops/debug-share") suspend fun runDebugShare( - @Body body: Map, + @Body body: DebugShareRequest, ): Response // ── Admin: Checkpoints ──────────────────────────────────────────── @@ -984,13 +989,13 @@ interface HermesApiService : KanbanApiService { @POST("api/ops/hooks") suspend fun createHook( - @Body body: Map, - ): Response> + @Body body: HookCreateRequest, + ): Response - @DELETE("api/ops/hooks") + @HTTP(method = "DELETE", path = "api/ops/hooks", hasBody = true) suspend fun deleteHook( - @Body body: Map, - ): Response> + @Body body: HookDeleteRequest, + ): Response // ── Admin: Action status (log viewer) ──────────────────────────── @GET("api/actions/{name}/status") diff --git a/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt index 97ac71bf..a83e0421 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt @@ -10,6 +10,7 @@ import com.m57.hermescontrol.data.model.McpOAuthFlowResponse import com.m57.hermescontrol.data.model.McpServer import com.m57.hermescontrol.data.model.McpServerTestResponse import com.m57.hermescontrol.data.model.McpServerToggleRequest +import com.m57.hermescontrol.data.model.McpServerUpdateRequest import com.m57.hermescontrol.data.model.McpServersResponse import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.NetworkError @@ -458,7 +459,12 @@ class McpServersViewModel( val updatedEnv = existingEnv + (key to value) val result = withContext(ioDispatcher) { - safeApiCall { ApiClient.hermesApi.updateMcpServer(serverName, mapOf("env" to updatedEnv)) } + safeApiCall { + ApiClient.hermesApi.updateMcpServer( + serverName, + McpServerUpdateRequest(env = updatedEnv), + ) + } } when (result) { is NetworkResult.Success -> { @@ -490,7 +496,12 @@ class McpServersViewModel( val updatedEnv = existingEnv - key val result = withContext(ioDispatcher) { - safeApiCall { ApiClient.hermesApi.updateMcpServer(serverName, mapOf("env" to updatedEnv)) } + safeApiCall { + ApiClient.hermesApi.updateMcpServer( + serverName, + McpServerUpdateRequest(env = updatedEnv), + ) + } } when (result) { is NetworkResult.Success -> { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt index 88b8b69e..2de21c08 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt @@ -11,11 +11,15 @@ import com.m57.hermescontrol.data.model.ActionResponse import com.m57.hermescontrol.data.model.ActionStatusResponse import com.m57.hermescontrol.data.model.BackupTriggerRequest import com.m57.hermescontrol.data.model.CheckpointsResponse +import com.m57.hermescontrol.data.model.CredentialPoolAddRequest import com.m57.hermescontrol.data.model.CredentialPoolProvider import com.m57.hermescontrol.data.model.CuratorResponse +import com.m57.hermescontrol.data.model.DebugShareRequest import com.m57.hermescontrol.data.model.DebugShareResponse import com.m57.hermescontrol.data.model.DoctorResponse import com.m57.hermescontrol.data.model.GatewayMigrationPlan +import com.m57.hermescontrol.data.model.HookCreateRequest +import com.m57.hermescontrol.data.model.HookDeleteRequest import com.m57.hermescontrol.data.model.HookResponse import com.m57.hermescontrol.data.model.PortalResponse import com.m57.hermescontrol.data.model.StatusResponse @@ -531,11 +535,11 @@ class SystemViewModel( _uiState.update { it.copy(addingCred = true) } viewModelScope.launch { val body = - buildMap { - put("provider", state.credProvider) - put("token", state.credKey) - if (state.credLabel.isNotBlank()) put("label", state.credLabel) - } + CredentialPoolAddRequest( + provider = state.credProvider, + apiKey = state.credKey, + label = state.credLabel.ifBlank { null }, + ) val result = withContext(Dispatchers.IO) { safeApiCall { ApiClient.hermesApi.addCredentialPoolEntry(body) } @@ -780,7 +784,7 @@ class SystemViewModel( viewModelScope.launch { val result = withContext(Dispatchers.IO) { - safeApiCall { ApiClient.hermesApi.runDebugShare(mapOf("redact" to shareRedact)) } + safeApiCall { ApiClient.hermesApi.runDebugShare(DebugShareRequest(redact = shareRedact)) } } when (result) { is NetworkResult.Success -> { @@ -849,13 +853,13 @@ class SystemViewModel( _uiState.update { it.copy(creatingHook = true) } viewModelScope.launch { val body = - buildMap { - put("event", state.hookEvent) - put("command", state.hookCommand) - if (state.hookMatcher.isNotBlank()) put("matcher", state.hookMatcher) - if (state.hookTimeout.isNotBlank()) put("timeout", state.hookTimeout.toIntOrNull() ?: 30) - put("allowed", state.hookApprove) - } + HookCreateRequest( + event = state.hookEvent, + command = state.hookCommand, + matcher = state.hookMatcher.ifBlank { null }, + timeout = if (state.hookTimeout.isBlank()) null else state.hookTimeout.toIntOrNull() ?: 30, + approve = state.hookApprove, + ) val result = withContext(Dispatchers.IO) { safeApiCall { ApiClient.hermesApi.createHook(body) } @@ -894,7 +898,7 @@ class SystemViewModel( viewModelScope.launch { val result = withContext(Dispatchers.IO) { - safeApiCall { ApiClient.hermesApi.deleteHook(mapOf("event" to event, "command" to command)) } + safeApiCall { ApiClient.hermesApi.deleteHook(HookDeleteRequest(event, command)) } } when (result) { is NetworkResult.Success -> { diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/HermesApiServiceConverterTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/HermesApiServiceConverterTest.kt new file mode 100644 index 00000000..0cb63108 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/HermesApiServiceConverterTest.kt @@ -0,0 +1,114 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.model.CredentialPoolAddRequest +import com.m57.hermescontrol.data.model.DebugShareRequest +import com.m57.hermescontrol.data.model.HookCreateRequest +import com.m57.hermescontrol.data.model.HookDeleteRequest +import com.m57.hermescontrol.data.model.McpCatalogInstallRequest +import com.m57.hermescontrol.data.model.McpServerUpdateRequest +import kotlinx.coroutines.runBlocking +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import retrofit2.Retrofit +import retrofit2.converter.kotlinx.serialization.asConverterFactory + +/** + * Regression for #1402: `Map` bodies/results have no kotlinx serializer, so Retrofit failed to + * create the converter before any request was sent. These call the real service methods so converter creation + * (which JSON round-trip tests never exercise) is covered. + */ +class HermesApiServiceConverterTest { + private lateinit var server: MockWebServer + private lateinit var api: HermesApiService + + @Before + fun setUp() { + server = MockWebServer().also { it.start() } + api = + Retrofit + .Builder() + .baseUrl(server.url("/")) + .addConverterFactory(OkHttpProvider.json.asConverterFactory("application/json".toMediaType())) + .build() + .create(HermesApiService::class.java) + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun ok(body: String = """{"ok":true}""") = + server.enqueue(MockResponse().setHeader("Content-Type", "application/json").setBody(body)) + + @Test + fun catalogInstall_readsResponse() = + runBlocking { + ok("""{"ok":true,"name":"example","background":false}""") + val r = api.installMcpCatalogEntry(McpCatalogInstallRequest(name = "example")) + assertTrue(r.isSuccessful) + assertEquals("example", r.body()?.name) + assertEquals("/api/mcp/catalog/install", server.takeRequest().path) + } + + @Test + fun updateMcpServer_sendsEnvBody() = + runBlocking { + ok("""{"name":"s","enabled":true}""") + api.updateMcpServer("s", McpServerUpdateRequest(env = mapOf("K" to "v"))) + val req = server.takeRequest() + assertEquals("PUT", req.method) + assertEquals("""{"env":{"K":"v"}}""", req.body.readUtf8()) + } + + @Test + fun curatorPaused_readsResponse() = + runBlocking { + ok("""{"ok":true,"paused":true}""") + assertTrue(api.setCuratorPaused(mapOf("paused" to true)).isSuccessful) + assertEquals("""{"paused":true}""", server.takeRequest().body.readUtf8()) + } + + @Test + fun credentialPool_addSendsApiKeyField_removeReadsResponse() = + runBlocking { + ok("""{"ok":true,"provider":"openrouter","count":1}""") + assertTrue(api.addCredentialPoolEntry(CredentialPoolAddRequest("openrouter", "sk", "lbl")).isSuccessful) + assertEquals( + """{"provider":"openrouter","api_key":"sk","label":"lbl"}""", + server.takeRequest().body.readUtf8(), + ) + ok("""{"ok":true,"provider":"openrouter","count":0,"cleaned":[],"hints":[]}""") + assertTrue(api.removeCredentialPoolEntry("openrouter", 1).isSuccessful) + } + + @Test + fun hooks_createSendsApprove_deleteSendsBody() = + runBlocking { + ok("""{"ok":true,"event":"e","command":"c","approved":true}""") + assertTrue(api.createHook(HookCreateRequest("e", "c", timeout = 5, approve = true)).isSuccessful) + assertEquals( + """{"event":"e","command":"c","timeout":5,"approve":true}""", + server.takeRequest().body.readUtf8(), + ) + ok() + assertTrue(api.deleteHook(HookDeleteRequest("e", "c")).isSuccessful) + val del = server.takeRequest() + assertEquals("DELETE", del.method) + assertEquals("""{"event":"e","command":"c"}""", del.body.readUtf8()) + } + + @Test + fun debugShare_sendsRedact() = + runBlocking { + ok("""{"ok":true,"urls":{"report":"u"}}""") + assertTrue(api.runDebugShare(DebugShareRequest(redact = false)).isSuccessful) + assertEquals("""{"redact":false}""", server.takeRequest().body.readUtf8()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpOwnershipTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpOwnershipTest.kt index 81c7c7a1..383898b8 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpOwnershipTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpOwnershipTest.kt @@ -3,6 +3,7 @@ package com.m57.hermescontrol.ui.mcp import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.model.McpServer import com.m57.hermescontrol.data.model.McpServerTestResponse +import com.m57.hermescontrol.data.model.McpServerUpdateRequest import com.m57.hermescontrol.data.model.McpServersResponse import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.HermesApiService @@ -122,10 +123,10 @@ class McpOwnershipTest { vm.updateEnvValue("value") vm.addEnvVar(config.name) dispatcher.scheduler.advanceUntilIdle() - coVerify { api.updateMcpServer(config.name, mapOf("env" to mapOf("KEY" to "value"))) } + coVerify { api.updateMcpServer(config.name, McpServerUpdateRequest(env = mapOf("KEY" to "value"))) } vm.removeEnvVar(config.name, "KEY") dispatcher.scheduler.advanceUntilIdle() - coVerify { api.updateMcpServer(config.name, mapOf("env" to emptyMap())) } + coVerify { api.updateMcpServer(config.name, McpServerUpdateRequest(env = emptyMap())) } coEvery { api.deleteMcpServer(config.name) } returns Response.success(Unit) vm.deleteServer(config.name) dispatcher.scheduler.advanceUntilIdle()