From c446715608ec38aa2c2c8e457a28e13ab938b7be Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Mon, 13 Jul 2026 22:40:16 +0300 Subject: [PATCH] fix(#587): forward session_id on image.attach_bytes and await the RPC (#588) * fix(#587): forward session_id on image.attach_bytes and await the RPC The mobile image-send path called image.attach_bytes without the required session_id field, so the gateway resolver (_sess) returned JsonRpcError(code=4001, message=session not found). - Add "session_id" to agentSessionId to the image.attach_bytes params (desktop reference client already does this; mobile was the outlier). - Switch the image branch from fire-and-forget wsClient.send to the awaited sendRpcAndAwait() layer (same awaited-RPC path used for slash commands, #526). This also removes the race where prompt.submit could fire before the image was staged, and surfaces RPC errors instead of silently swallowing them. Root cause verified against tui_gateway/server.py _sess() and the desktop use-prompt-actions reference. ChatViewModel.kt passes ktlintCheck. Closes #587 * fix(#587): also forward session_id on file.attach (same 4001 bug) The image branch was fixed in the prior commit, but the file branch (sendMessage else -> file.attach) omitted session_id too, so non-image file uploads also hit JsonRpcError(code=4001, "session not found"). The backend file.attach handler (tui_gateway/server.py _sess) requires session_id exactly like image.attach_bytes. Add "session_id" to agentSessionId to the file.attach params, and refresh the sendMessage flow comment (no longer fire-and-forget). Verified by source: both image.attach_bytes and file.attach call the same _sess resolver. Camera capture is unaffected (it produces an image/jpeg attachment, which already routes through the fixed image.attach_bytes branch). ChatViewModel.kt passes ktlintCheck. * test(#587): regression for image.attach_bytes session_id Adds testSendMessage_imageAttachment_sendsSessionId, which proves the image-attach RPC is issued with session_id (the missing field that caused JsonRpcError 4001 'session not found'). Root blocker found: readContentUriBase64 constructs android.util.Base64OutputStream, which throws MockK's 'Stub!' on the JVM test, making the read return null and the image branch never run. Mocking the Base64OutputStream constructor lets the read succeed so the request(any(), capture(paramsSlot), any()) stub captures the params and the session_id assertion holds. Full ChatViewModelTest suite passes; ktlint-clean. --------- Co-authored-by: M57 --- .../hermescontrol/ui/chat/ChatViewModel.kt | 43 ++++++++---- .../ui/chat/ChatViewModelTest.kt | 67 +++++++++++++++++++ 2 files changed, 96 insertions(+), 14 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 8b7ca524..fb176dde 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 @@ -642,8 +642,8 @@ class ChatViewModel( * Flow: * 1. Snapshot pending attachments (then clear them from UI) * 2. Add user message to UI + DB immediately (optimistic UX) - * 3. For each image → fire-and-forget `image.attach_bytes` (queues into session) - * 4. For each file → await `file.attach`, collect @file: refs + * 3. For each image → await `image.attach_bytes` (requires session_id) + * 4. For each file → await `file.attach` (requires session_id), collect @file: refs * 5. Send `prompt.submit` with text + @file: refs — images auto-picked up by backend */ fun sendMessage(text: String) { @@ -694,23 +694,38 @@ class ChatViewModel( try { if (attachment.isImage) { - // Fire-and-forget — backend queues into session["attached_images"], - // auto-picked by the subsequent prompt.submit - wsClient.send( - method = WsMethods.IMAGE_ATTACH_BYTES, - params = - mapOf( - "content_base64" to "data:${attachment.mimeType};base64,$b64", - "filename" to attachment.name, - "ext" to attachment.fileExtension, - ), - ) + // Await so the backend stages the image into + // session["attached_images"] BEFORE prompt.submit runs + // (a fire-and-forget send raced prompt.submit and the + // image was dropped). Requires session_id or the gateway + // 4001s "session not found" (desktop passes it too). + val result = + sendRpcAndAwait( + method = WsMethods.IMAGE_ATTACH_BYTES, + params = + mapOf( + "session_id" to agentSessionId, + "content_base64" to "data:${attachment.mimeType};base64,$b64", + "filename" to attachment.name, + "ext" to attachment.fileExtension, + ), + ) + if (result != null) { + @Suppress("UNCHECKED_CAST") + val ok = (result as? Map)?.get("attached") as? Boolean + if (ok != true) { + Log.w(TAG, "Image attach for ${attachment.name} returned non-ok: $result") + } + } } else { - // Await the @file: ref text so we can embed it in the prompt + // Await the @file: ref text so we can embed it in the prompt. + // file.attach also requires session_id or the gateway 4001s + // "session not found" (same resolver as image.attach_bytes). sendRpcAndAwait( method = WsMethods.FILE_ATTACH, params = mapOf( + "session_id" to agentSessionId, "data_url" to "data:${attachment.mimeType};base64,$b64", "name" to attachment.name, ), 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 40ac4940..b60670f5 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 @@ -17,8 +17,10 @@ import io.mockk.every import io.mockk.mockk import io.mockk.mockkObject import io.mockk.mockkStatic +import io.mockk.slot import io.mockk.unmockkAll import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow @@ -1396,6 +1398,71 @@ class ChatViewModelTest { assertTrue("Message should still be sent despite attachment read failure", sent) } + /** + * Regression for the "session not found" (code 4001) error when sending an + * image: the mobile image-attach path must pass `session_id` to + * `image.attach_bytes` (the gateway resolves the session from it; desktop + * does the same). Without it the backend 4001s and the image is dropped. + * Also asserts the image attach is AWAITED (staged before prompt.submit), + * not fire-and-forget. + */ + @Test + fun testSendMessage_imageAttachment_sendsSessionId() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + + // sendRpcAndAwait calls HermesWsClient.request(method, params) — the + // 2-arg form (Kotlin synthesizes request(String, Map)). Stub that + // exact signature, capture params, and return a completed deferred + // so the await resolves without hanging. + val paramsSlot = slot>() + every { + HermesWsClient.request(any(), capture(paramsSlot), any()) + } returns CompletableDeferred(mapOf("attached" to true)) + + // Mock Android's Base64OutputStream constructor to avoid "Stub!" exception in JVM tests + io.mockk.mockkConstructor(android.util.Base64OutputStream::class) + every { + anyConstructed().write( + any(), + any(), + any(), + ) + } returns Unit + every { anyConstructed().close() } returns Unit + + // The image bytes read via ContentResolver must succeed. + mockkStatic(Uri::class) + val mockUri = mockk() + every { Uri.parse("content://dummy") } returns mockUri + val contentResolver = mockk() + every { app.contentResolver } returns contentResolver + every { contentResolver.openInputStream(any()) } returns + java.io.ByteArrayInputStream(byteArrayOf(1, 2, 3, 4)) + + viewModel.addAttachment("content://dummy", "test.png", "image/png", 1000) + advanceUntilIdle() + assertTrue( + "pendingAttachments must contain the image before send", + viewModel.uiState.value.pendingAttachments.isNotEmpty(), + ) + assertTrue( + "attached png must have isImage=true", + viewModel.uiState.value.pendingAttachments.first().isImage, + ) + + viewModel.sendMessage("Here is an image") + advanceUntilIdle() + + // Verify the image attach RPC was issued with session_id. + verify { HermesWsClient.request(WsMethods.IMAGE_ATTACH_BYTES, any()) } + assertEquals( + "session_id must be forwarded to image.attach_bytes", + sessionId, + paramsSlot.captured["session_id"], + ) + } + // ── Pending request timeout + rejectAllPending (issue #526) ─────────── /**