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 <m57@hermes-control.app>
This commit is contained in:
M57andM57 authored and GitHub committed 2026-07-13 22:40:16 +03:00
1 parent 173820c304
commit c446715608
2 files changed
+96 -14

No files matched your search

@@ -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<Map<String, Any>>()
every {
HermesWsClient.request(any(), capture(paramsSlot), any())
} returns CompletableDeferred<Any?>(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<android.util.Base64OutputStream>().write(
any<ByteArray>(),
any(),
any(),
)
} returns Unit
every { anyConstructed<android.util.Base64OutputStream>().close() } returns Unit
// The image bytes read via ContentResolver must succeed.
mockkStatic(Uri::class)
val mockUri = mockk<Uri>()
every { Uri.parse("content://dummy") } returns mockUri
val contentResolver = mockk<ContentResolver>()
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) ───────────
/**