diff --git a/detekt-baseline.xml b/detekt-baseline.xml index f7e7108..7124ece 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -649,6 +649,7 @@ PackageNaming:SkillSelectionTest.kt$package agents_engine.core PackageNaming:SkillsEncapsulationTest.kt$package agents_engine.core PackageNaming:Snapshot.kt$package agents_engine.core + PackageNaming:SnapshotImagesRoundTripTest.kt$package agents_engine.core PackageNaming:SnapshotManifestGuardTest.kt$package agents_engine.core PackageNaming:SnapshotResumeTest.kt$package agents_engine.model PackageNaming:StdioMcpTransport.kt$package agents_engine.mcp diff --git a/src/main/kotlin/agents_engine/core/Snapshot.kt b/src/main/kotlin/agents_engine/core/Snapshot.kt index bd3fdef..661c4c5 100644 --- a/src/main/kotlin/agents_engine/core/Snapshot.kt +++ b/src/main/kotlin/agents_engine/core/Snapshot.kt @@ -1,6 +1,7 @@ package agents_engine.core import agents_engine.generation.LenientJsonParser +import agents_engine.model.ImagePart import agents_engine.model.InlineToolCallParser import agents_engine.model.LlmMessage import agents_engine.model.TokenUsage @@ -164,6 +165,19 @@ internal object SnapshotJson { }) append("]") } + // #2866 — persist vision attachments so file-backed resume can rehydrate + // the LlmMessage byte-for-byte. Pre-#2866 the encoder silently dropped + // `images`, breaking SSE-style apps that snapshot mid-conversation. + // Base64 directly inside the snapshot (rather than ContentRef.hash with + // a re-fetch on resume) keeps the snapshot self-contained — no + // BlobStore dependency at restore time. + m.images?.let { imgs -> + append(""","images":[""") + append(imgs.joinToString(",") { part -> + """{"base64":${part.base64.toJsonString()},"mime":${part.wireMime.value.toJsonString()}}""" + }) + append("]") + } append("}") } @@ -194,13 +208,33 @@ internal object SnapshotJson { .entries.associate { (k, v) -> k.toString() to v } as Map ToolCall(name = name, arguments = args) } + // #2866 — rehydrate vision attachments. Mime strings that don't match + // a known [ImagePart.WireMime] variant skip with a null mapping + // (defensive — the encoder only writes the closed set, but a + // hand-edited or future-extended snapshot shouldn't crash resume). + val images = (m["images"] as? List<*>)?.mapNotNull { imgRaw -> + val img = imgRaw as? Map<*, *> ?: return@mapNotNull null + val base64 = img["base64"] as? String ?: return@mapNotNull null + val mimeStr = img["mime"] as? String ?: return@mapNotNull null + val wireMime = decodeWireMime(mimeStr) ?: return@mapNotNull null + ImagePart(base64 = base64, wireMime = wireMime) + } return LlmMessage( role = m["role"]?.toString() ?: "user", content = m["content"]?.toString() ?: "", toolCalls = toolCalls, + images = images, ) } + private fun decodeWireMime(value: String): ImagePart.WireMime? = when (value) { + ImagePart.WireMime.Png.value -> ImagePart.WireMime.Png + ImagePart.WireMime.Jpeg.value -> ImagePart.WireMime.Jpeg + ImagePart.WireMime.Gif.value -> ImagePart.WireMime.Gif + ImagePart.WireMime.Webp.value -> ImagePart.WireMime.Webp + else -> null + } + private fun decodeTokens(t: Map<*, *>?): TokenUsage? { if (t == null) return null val prompt = (t["prompt"] as? Number)?.toInt() ?: return null diff --git a/src/test/kotlin/agents_engine/core/SnapshotImagesRoundTripTest.kt b/src/test/kotlin/agents_engine/core/SnapshotImagesRoundTripTest.kt new file mode 100644 index 0000000..3c6f17e --- /dev/null +++ b/src/test/kotlin/agents_engine/core/SnapshotImagesRoundTripTest.kt @@ -0,0 +1,163 @@ +package agents_engine.core + +import agents_engine.model.ImagePart +import agents_engine.model.LlmMessage +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * #2866 — regression coverage for the SnapshotJson encoder + decoder + * round-tripping `LlmMessage.images`. + * + * Pre-#2866 the encoder serialised only `role / content / toolCalls` + * per message. File-backed `snapshot/resume` silently lost any vision + * attachments that were on the conversation when the snapshot was + * taken. SSE-style apps mid-conversation lost the image context on + * resume. + * + * The fix encodes each `ImagePart` as `{base64, mime}` directly inside + * the message (rather than ContentRef + BlobStore re-fetch) so the + * snapshot stays self-contained — no BlobStore dependency at resume. + */ +class SnapshotImagesRoundTripTest { + + @Test + fun `single image round-trips through encode-decode`() { + val original = SessionSnapshot( + messages = listOf( + LlmMessage( + role = "user", + content = "What's in this image?", + images = listOf( + ImagePart(base64 = "iVBORw0KGgoAAAANSUhEUg==", wireMime = ImagePart.WireMime.Png), + ), + ), + ), + turns = 1, + toolCalls = 0, + toolCallLimit = 0, + tokensUsed = null, + memory = emptyMap(), + requestId = "req-1", sessionId = null, manifestHash = null, + ) + + val encoded = SnapshotJson.encode(original) + val decoded = SnapshotJson.decode(encoded) + + assertEquals(1, decoded.messages.size) + val msg = decoded.messages.single() + assertEquals("user", msg.role) + assertEquals("What's in this image?", msg.content) + val images = msg.images + assertNotNull(images, "images must rehydrate") + assertEquals(1, images.size) + assertEquals("iVBORw0KGgoAAAANSUhEUg==", images.single().base64) + assertEquals(ImagePart.WireMime.Png, images.single().wireMime) + } + + @Test + fun `multiple images of mixed mimes round-trip in order`() { + val original = SessionSnapshot( + messages = listOf( + LlmMessage( + role = "user", + content = "Compare these.", + images = listOf( + ImagePart(base64 = "AAA1", wireMime = ImagePart.WireMime.Png), + ImagePart(base64 = "BBB2", wireMime = ImagePart.WireMime.Jpeg), + ImagePart(base64 = "CCC3", wireMime = ImagePart.WireMime.Webp), + ImagePart(base64 = "DDD4", wireMime = ImagePart.WireMime.Gif), + ), + ), + ), + turns = 1, + toolCalls = 0, + toolCallLimit = 0, + tokensUsed = null, + memory = emptyMap(), + requestId = "req-2", sessionId = null, manifestHash = null, + ) + + val decoded = SnapshotJson.decode(SnapshotJson.encode(original)) + val images = decoded.messages.single().images!! + assertEquals(listOf("AAA1", "BBB2", "CCC3", "DDD4"), images.map { it.base64 }) + assertEquals( + listOf( + ImagePart.WireMime.Png, + ImagePart.WireMime.Jpeg, + ImagePart.WireMime.Webp, + ImagePart.WireMime.Gif, + ), + images.map { it.wireMime }, + ) + } + + @Test + fun `message with no images round-trips with null images field — back-compat`() { + val original = SessionSnapshot( + messages = listOf(LlmMessage(role = "user", content = "no vision")), + turns = 1, + toolCalls = 0, + toolCallLimit = 0, + tokensUsed = null, + memory = emptyMap(), + requestId = "req-3", sessionId = null, manifestHash = null, + ) + + val encoded = SnapshotJson.encode(original) + // Wire shape: when images is null, the `images` key must be omitted — + // this preserves byte-identity with pre-#2866 snapshots. + assertTrue( + !encoded.contains("\"images\""), + "no images → no `images` key in the JSON (back-compat): $encoded", + ) + + val decoded = SnapshotJson.decode(encoded) + assertNull(decoded.messages.single().images) + } + + @Test + fun `pre-2866 snapshots without an images key decode with null images`() { + // Hand-crafted legacy snapshot shape — older clients that saved + // before #2866 should still load cleanly. + val legacy = """{ + "messages":[{"role":"user","content":"hi"}], + "turns":0, + "toolCalls":0, + "toolCallLimit":0, + "memory":{}, + "requestId":"legacy-1" + }""".trimIndent() + + val decoded = SnapshotJson.decode(legacy) + assertEquals(1, decoded.messages.size) + assertNull(decoded.messages.single().images, "legacy snapshots decode with null images") + } + + @Test + fun `unknown image mime values are skipped on decode — defensive`() { + // A future-extended or hand-edited snapshot with an unknown mime + // should NOT crash resume — the unknown part is dropped, the rest + // of the message rehydrates fine. + val custom = """{ + "messages":[{ + "role":"user", + "content":"mixed-mime", + "images":[ + {"base64":"OK1","mime":"image/png"}, + {"base64":"OK2","mime":"image/unknown-future-format"}, + {"base64":"OK3","mime":"image/jpeg"} + ] + }], + "turns":0,"toolCalls":0,"toolCallLimit":0,"memory":{},"requestId":"r" + }""".trimIndent() + + val decoded = SnapshotJson.decode(custom) + val images = decoded.messages.single().images!! + assertEquals(2, images.size, "the unknown-mime entry should be skipped") + assertEquals(listOf("OK1", "OK3"), images.map { it.base64 }) + } +}