From 2abc722f9d7721392ce3c5f54371f2af299fa96a Mon Sep 17 00:00:00 2001 From: Opencode Date: Tue, 7 Apr 2026 10:15:17 +0000 Subject: [PATCH] fix: merge all system messages into one for llama.cpp compatibility - Merge memory blocks, provider system content, and journal instructions into a single system message at index 0 - Fixes 'System message must be at the beginning' error with Qwen 3.5 when using llama.cpp/Jinja templates - Add unit tests for system message transformation --- src/plugin.test.ts | 200 +++++++++++++++++++++++++++++++++++++++++++++ src/plugin.ts | 18 ++-- 2 files changed, 210 insertions(+), 8 deletions(-) create mode 100644 src/plugin.test.ts diff --git a/src/plugin.test.ts b/src/plugin.test.ts new file mode 100644 index 0000000..7dc103c --- /dev/null +++ b/src/plugin.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, test, mock } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; + +import { MemoryPlugin } from "./plugin"; +import { renderMemoryBlocks } from "./prompt"; + +async function mkTmpDir(): Promise { + const root = await fs.mkdtemp(path.join("/tmp/", "opencode-plugin-")); + return root; +} + +interface SystemMessage { + content: string; +} + +interface SystemTransformOutput { + system: SystemMessage[]; +} + +describe("MemoryPlugin system message transformation", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkTmpDir(); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + test("should merge all system messages into one when multiple exist", async () => { + const plugin = await MemoryPlugin({ directory: tmpDir }); + const transformHook = plugin["experimental.chat.system.transform"]; + + const output: SystemTransformOutput = { + system: [ + { content: "Provider system header" }, + { content: "Additional system instruction" }, + ], + }; + + // @ts-ignore - testing internal hook + await transformHook({}, output); + + // Should merge all into exactly one system message + expect(output.system.length).toBe(1); + expect(output.system[0].content).toContain(""); + expect(output.system[0].content).toContain("Provider system header"); + expect(output.system[0].content).toContain("Additional system instruction"); + }); + + test("should handle empty system array", async () => { + const plugin = await MemoryPlugin({ directory: tmpDir }); + const transformHook = plugin["experimental.chat.system.transform"]; + + const output: SystemTransformOutput = { system: [] }; + + // @ts-ignore - testing internal hook + await transformHook({}, output); + + // Should create one system message with just the memory blocks + expect(output.system.length).toBe(1); + expect(output.system[0].content).toContain(""); + }); + + test("should handle single existing system message", async () => { + const plugin = await MemoryPlugin({ directory: tmpDir }); + const transformHook = plugin["experimental.chat.system.transform"]; + + const output: SystemTransformOutput = { + system: [{ content: "Existing system content" }], + }; + + // @ts-ignore - testing internal hook + await transformHook({}, output); + + // Should still have exactly one system message + expect(output.system.length).toBe(1); + expect(output.system[0].content).toContain(""); + expect(output.system[0].content).toContain("Existing system content"); + }); + + test("should prepend memory blocks before provider system content", async () => { + const plugin = await MemoryPlugin({ directory: tmpDir }); + const transformHook = plugin["experimental.chat.system.transform"]; + + const output: SystemTransformOutput = { + system: [{ content: "Provider system header" }], + }; + + // @ts-ignore - testing internal hook + await transformHook({}, output); + + // Memory blocks should come first + const content = output.system[0].content; + const memoryIndex = content.indexOf(""); + const headerIndex = content.indexOf("Provider system header"); + + expect(memoryIndex).toBeLessThan(headerIndex); + }); + + test("should handle string-typed system messages", async () => { + // Some providers may use string instead of { content: string } + const plugin = await MemoryPlugin({ directory: tmpDir }); + const transformHook = plugin["experimental.chat.system.transform"]; + + const output: SystemTransformOutput = { + system: [ + { content: "String content" }, + { content: "More content" }, + ], + }; + + // @ts-ignore - testing internal hook + await transformHook({}, output); + + expect(output.system.length).toBe(1); + expect(output.system[0].content).toContain("String content"); + expect(output.system[0].content).toContain("More content"); + }); +}); + +describe("renderMemoryBlocks output structure", () => { + test("renders valid memory blocks xml", () => { + const xml = renderMemoryBlocks([ + { + scope: "global", + label: "persona", + description: "Your persona", + limit: 5000, + readOnly: false, + value: "I am an AI assistant", + filePath: "/tmp/persona.md", + lastModified: new Date("2025-01-15T08:00:00Z"), + }, + ]); + + expect(xml).toContain(""); + expect(xml).toContain(""); + expect(xml).toContain(""); + expect(xml).toContain(""); + expect(xml).toContain(""); + }); + + test("includes line numbers in memory block values", () => { + const xml = renderMemoryBlocks([ + { + scope: "global", + label: "notes", + description: "Notes", + limit: 1000, + readOnly: false, + value: "line one\nline two\nline three", + filePath: "/tmp/notes.md", + lastModified: new Date("2025-01-15T08:00:00Z"), + }, + ]); + + expect(xml).toContain("1→ line one"); + expect(xml).toContain("2→ line two"); + expect(xml).toContain("3→ line three"); + }); + + test("handles empty memory block values", () => { + const xml = renderMemoryBlocks([ + { + scope: "project", + label: "empty", + description: "Empty block", + limit: 1000, + readOnly: false, + value: "", + filePath: "/tmp/empty.md", + lastModified: new Date("2025-01-15T08:00:00Z"), + }, + ]); + + expect(xml).toContain(""); + expect(xml).toContain("\n\n"); + }); + + test("marks read-only blocks correctly", () => { + const xml = renderMemoryBlocks([ + { + scope: "project", + label: "readonly", + description: "Read only", + limit: 1000, + readOnly: true, + value: "content", + filePath: "/tmp/readonly.md", + lastModified: new Date("2025-01-15T08:00:00Z"), + }, + ]); + + expect(xml).toContain("read_only=true"); + expect(xml).toContain("DO NOT MODIFY"); + }); +}); diff --git a/src/plugin.ts b/src/plugin.ts index ac4a747..8a71941 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -58,15 +58,17 @@ export const MemoryPlugin: Plugin = async ({ directory }) => { const xml = renderMemoryBlocks(blocks); if (!xml) return; - // Insert early (right after provider header) for salience. - // OpenCode will re-join system chunks to preserve caching. - const insertAt = output.system.length > 0 ? 1 : 0; - output.system.splice(insertAt, 0, xml); + // Merge all system messages into one to satisfy llama.cpp's Jinja template + // which requires exactly one system message at the beginning. + const systemContents = output.system.length > 0 + ? output.system.map(s => typeof s === "string" ? s : s.content || "") + : []; - // Append journal instructions at the end (preserves memory block cache) - if (journalSystemNote) { - output.system.push(journalSystemNote); - } + const mergedContent = [xml, ...systemContents, journalSystemNote] + .filter(Boolean) + .join("\n\n"); + + output.system = [mergedContent]; }, tool: {