diff --git a/dist/index.js b/dist/index.js index 29914e43..8327d088 100644 --- a/dist/index.js +++ b/dist/index.js @@ -1015,14 +1015,24 @@ async function ensureDailyLogFile(dailyPath, dateStr) { await writeFile(dailyPath, `# ${dateStr}\n\n`, "utf-8"); } } -export function buildReflectionPrompt(conversation, maxInputChars, toolErrorSignals = []) { +// Slot split: system = identity + every static block (task, headings +// contract, hard rules, section/governance rules, notes, output template); +// user = only the dynamically generated content (tool error signals + the +// INPUT transcript fence). Both runners deliver the combined single-slot +// form today (the embedded raw run does not honor a separate system slot, +// and the CLI fallback carries a single user message); the parts keep the +// seam explicit so a host whose runner honors a system slot can deliver +// them split without changing the prompt text: system + blank line + user +// reproduces the single-slot form exactly, and the prompt hash is computed +// over that combined form. +export function buildReflectionPromptParts(conversation, maxInputChars, toolErrorSignals = []) { const clipped = conversation.slice(-maxInputChars); const errorHints = toolErrorSignals.length > 0 ? toolErrorSignals .map((e, i) => `${i + 1}. [${e.toolName}] ${e.summary} (sig:${e.signatureHash.slice(0, 8)})`) .join("\n") : "- (none)"; - return [ + const system = [ "You are generating a durable MEMORY REFLECTION entry for an AI assistant system.", "", "Output Markdown only. No intro text. No outro text. No extra headings.", @@ -1119,7 +1129,8 @@ export function buildReflectionPrompt(conversation, maxInputChars, toolErrorSign "", "## Derived", "- This run showed ...", - "", + ].join("\n"); + const user = [ "Recent tool error signals:", errorHints, "", @@ -1128,6 +1139,13 @@ export function buildReflectionPrompt(conversation, maxInputChars, toolErrorSign clipped, "```", ].join("\n"); + return { system, user }; +} +// Combined single-slot form of the distiller prompt: what both runners +// send today, and what prompt-content tests assert on. +export function buildReflectionPrompt(conversation, maxInputChars, toolErrorSignals = []) { + const parts = buildReflectionPromptParts(conversation, maxInputChars, toolErrorSignals); + return `${parts.system}\n\n${parts.user}`; } function buildReflectionFallbackText() { return [ @@ -1203,7 +1221,10 @@ export async function generateReflectionText(params) { } } async function generateReflectionTextUnbounded(params) { - const prompt = buildReflectionPrompt(params.conversation, params.maxInputChars, params.toolErrorSignals ?? []); + const promptParts = buildReflectionPromptParts(params.conversation, params.maxInputChars, params.toolErrorSignals ?? []); + // The combined single-slot form: hashed for change detection and sent + // as one prompt by both runners (neither has an honored system slot). + const prompt = `${promptParts.system}\n\n${promptParts.user}`; const promptHash = sha256Hex(prompt); const tempSessionFile = join(tmpdir(), `memory-reflection-${Date.now()}-${Math.random().toString(36).slice(2)}.jsonl`); let reflectionText = null; diff --git a/index.ts b/index.ts index 6166bcd1..46377de1 100644 --- a/index.ts +++ b/index.ts @@ -1462,18 +1462,30 @@ async function ensureDailyLogFile(dailyPath: string, dateStr: string): Promise 0 ? toolErrorSignals .map((e, i) => `${i + 1}. [${e.toolName}] ${e.summary} (sig:${e.signatureHash.slice(0, 8)})`) .join("\n") : "- (none)"; - return [ + const system = [ "You are generating a durable MEMORY REFLECTION entry for an AI assistant system.", "", "Output Markdown only. No intro text. No outro text. No extra headings.", @@ -1570,7 +1582,8 @@ export function buildReflectionPrompt( "", "## Derived", "- This run showed ...", - "", + ].join("\n"); + const user = [ "Recent tool error signals:", errorHints, "", @@ -1579,6 +1592,18 @@ export function buildReflectionPrompt( clipped, "```", ].join("\n"); + return { system, user }; +} + +// Combined single-slot form of the distiller prompt: what both runners +// send today, and what prompt-content tests assert on. +export function buildReflectionPrompt( + conversation: string, + maxInputChars: number, + toolErrorSignals: ReflectionErrorSignal[] = [] +): string { + const parts = buildReflectionPromptParts(conversation, maxInputChars, toolErrorSignals); + return `${parts.system}\n\n${parts.user}`; } function buildReflectionFallbackText(): string { @@ -1682,11 +1707,14 @@ export async function generateReflectionText( async function generateReflectionTextUnbounded( params: GenerateReflectionTextParams ): Promise { - const prompt = buildReflectionPrompt( + const promptParts = buildReflectionPromptParts( params.conversation, params.maxInputChars, params.toolErrorSignals ?? [] ); + // The combined single-slot form: hashed for change detection and sent + // as one prompt by both runners (neither has an honored system slot). + const prompt = `${promptParts.system}\n\n${promptParts.user}`; const promptHash = sha256Hex(prompt); const tempSessionFile = join( tmpdir(), diff --git a/package.json b/package.json index 8260b2e8..ca7bdcdd 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "skills/**/*.md" ], "scripts": { - "test": "node test/embedder-error-hints.test.mjs && node --test test/embedder-max-input-chars.test.mjs && node test/cjk-recursion-regression.test.mjs && node test/extraction-prompt-structural-noise.test.mjs && node test/i18n-memory-triggers.test.mjs && node test/migrate-legacy-schema.test.mjs && node --test test/config-session-strategy-migration.test.mjs && node --test test/scope-access-undefined.test.mjs && node --test test/reflection-bypass-hook.test.mjs && node --test test/smart-extractor-scope-filter.test.mjs && node --test test/store-empty-scope-filter.test.mjs && node --test test/recall-text-cleanup.test.mjs && node test/update-consistency-lancedb.test.mjs && node --test test/strip-envelope-metadata.test.mjs && node test/cli-smoke.mjs && node test/functional-e2e.mjs && node --test test/per-agent-auto-recall.test.mjs && node test/retriever-rerank-regression.mjs && node test/smart-memory-lifecycle.mjs && node test/smart-extractor-branches.mjs && node --test test/smart-extractor-noise-gating.test.mjs && node test/memory-capability-runtime.test.mjs && node --test test/startup-health-diagnostics.test.mjs && node test/corpus-indexer.test.mjs && node --test test/regex-fallback-bulk-store.test.mjs && node test/plugin-manifest-regression.mjs && node --test test/dreaming-engine.test.mjs && node --test test/session-summary-before-reset.test.mjs && node --test test/sync-plugin-version.test.mjs && node test/smart-metadata-v2.mjs && node test/vector-search-cosine.test.mjs && node test/context-support-e2e.mjs && node test/temporal-facts.test.mjs && node test/memory-update-supersede.test.mjs && node test/memory-update-metadata-refresh.test.mjs && node test/memory-upgrader-diagnostics.test.mjs && node --test test/llm-api-key-client.test.mjs && node --test test/llm-oauth-client.test.mjs && node --test test/cli-oauth-login.test.mjs && node --test test/workflow-fork-guards.test.mjs && node --test test/clawteam-scope.test.mjs && node --test test/cross-process-lock.test.mjs && node --test test/preference-slots.test.mjs && node test/is-latest-auto-supersede.test.mjs && node --test test/temporal-awareness.test.mjs && node --test test/command-reflection-guard.test.mjs && node --test test/tier1-counters.test.mjs && node --test test/startup-check-timeout.test.mjs && node --test test/memory-subsession-prompt-hooks.test.mjs && node --test test/read-consistency-interval.test.mjs && node --test test/reflection-distiller-hook-skip.test.mjs && node --test test/register-scope-dedup.test.mjs && node --test test/raw-run-distiller-hooks.test.mjs && node --test test/autocapture-watermark-reset.test.mjs && node --test test/autocapture-internal-session-guard.test.mjs && node --test test/memory-categories-storage-map.test.mjs && node --test test/delete-invalidate-reflection-caches.test.mjs && node --test test/reflection-mapped-rows-admission.test.mjs", "test:cli-smoke": "node scripts/run-ci-tests.mjs --group cli-smoke", + "test": "node test/embedder-error-hints.test.mjs && node --test test/embedder-max-input-chars.test.mjs && node test/cjk-recursion-regression.test.mjs && node test/extraction-prompt-structural-noise.test.mjs && node test/i18n-memory-triggers.test.mjs && node test/migrate-legacy-schema.test.mjs && node --test test/config-session-strategy-migration.test.mjs && node --test test/scope-access-undefined.test.mjs && node --test test/reflection-bypass-hook.test.mjs && node --test test/smart-extractor-scope-filter.test.mjs && node --test test/store-empty-scope-filter.test.mjs && node --test test/recall-text-cleanup.test.mjs && node test/update-consistency-lancedb.test.mjs && node --test test/strip-envelope-metadata.test.mjs && node test/cli-smoke.mjs && node test/functional-e2e.mjs && node --test test/per-agent-auto-recall.test.mjs && node test/retriever-rerank-regression.mjs && node test/smart-memory-lifecycle.mjs && node test/smart-extractor-branches.mjs && node --test test/smart-extractor-noise-gating.test.mjs && node test/memory-capability-runtime.test.mjs && node --test test/startup-health-diagnostics.test.mjs && node test/corpus-indexer.test.mjs && node --test test/regex-fallback-bulk-store.test.mjs && node test/plugin-manifest-regression.mjs && node --test test/dreaming-engine.test.mjs && node --test test/session-summary-before-reset.test.mjs && node --test test/sync-plugin-version.test.mjs && node test/smart-metadata-v2.mjs && node test/vector-search-cosine.test.mjs && node test/context-support-e2e.mjs && node test/temporal-facts.test.mjs && node test/memory-update-supersede.test.mjs && node test/memory-update-metadata-refresh.test.mjs && node test/memory-upgrader-diagnostics.test.mjs && node --test test/llm-api-key-client.test.mjs && node --test test/llm-oauth-client.test.mjs && node --test test/cli-oauth-login.test.mjs && node --test test/workflow-fork-guards.test.mjs && node --test test/clawteam-scope.test.mjs && node --test test/cross-process-lock.test.mjs && node --test test/preference-slots.test.mjs && node test/is-latest-auto-supersede.test.mjs && node --test test/temporal-awareness.test.mjs && node --test test/command-reflection-guard.test.mjs && node --test test/tier1-counters.test.mjs && node --test test/startup-check-timeout.test.mjs && node --test test/memory-subsession-prompt-hooks.test.mjs && node --test test/read-consistency-interval.test.mjs && node --test test/reflection-distiller-hook-skip.test.mjs && node --test test/register-scope-dedup.test.mjs && node --test test/raw-run-distiller-hooks.test.mjs && node --test test/autocapture-watermark-reset.test.mjs && node --test test/autocapture-internal-session-guard.test.mjs && node --test test/memory-categories-storage-map.test.mjs && node --test test/delete-invalidate-reflection-caches.test.mjs && node --test test/reflection-mapped-rows-admission.test.mjs && node --test test/reflection-prompt-split.test.mjs", "test:cli-smoke": "node scripts/run-ci-tests.mjs --group cli-smoke", "test:core-regression": "node scripts/run-ci-tests.mjs --group core-regression", "test:storage-and-schema": "node scripts/run-ci-tests.mjs --group storage-and-schema", "test:llm-clients-and-auth": "node scripts/run-ci-tests.mjs --group llm-clients-and-auth", diff --git a/scripts/ci-test-manifest.mjs b/scripts/ci-test-manifest.mjs index dce4388f..066e99c4 100644 --- a/scripts/ci-test-manifest.mjs +++ b/scripts/ci-test-manifest.mjs @@ -110,6 +110,7 @@ export const CI_TEST_MANIFEST = [ // Delete/delete-bulk must synchronously invalidate in-process reflection read caches { group: "core-regression", runner: "node", file: "test/delete-invalidate-reflection-caches.test.mjs", args: ["--test"] }, { group: "core-regression", runner: "node", file: "test/reflection-mapped-rows-admission.test.mjs", args: ["--test"] }, + { group: "core-regression", runner: "node", file: "test/reflection-prompt-split.test.mjs", args: ["--test"] }, ]; export function getEntriesForGroup(group) { diff --git a/test/reflection-prompt-split.test.mjs b/test/reflection-prompt-split.test.mjs new file mode 100644 index 00000000..eb89f518 --- /dev/null +++ b/test/reflection-prompt-split.test.mjs @@ -0,0 +1,195 @@ +/** + * Distiller prompt slot-placement tests. + * + * The reflection distiller's prompt is built as two parts: system = identity + * + every static block (task, headings contract, hard rules, + * section/governance rules, notes, output template); user = ONLY the + * dynamically generated content (tool error signals + the INPUT transcript + * fence). Both runners deliver the combined single-slot form today (the + * embedded raw run does not honor a separate system slot, and the CLI + * fallback carries a single user message); the parts keep the seam explicit + * and tested so a host whose runner honors a system slot can deliver them + * split without changing one byte of prompt text. + * + * Fixtures are entirely synthetic; no real fleet data. + * + * Run: node --test test/reflection-prompt-split.test.mjs + */ + +import { describe, it, after } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync, readFileSync, rmSync, chmodSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import jitiFactory from "jiti"; + +const testDir = path.dirname(fileURLToPath(import.meta.url)); +const pluginSdkStubPath = path.resolve(testDir, "helpers", "openclaw-plugin-sdk-stub.mjs"); +const jiti = jitiFactory(import.meta.url, { + interopDefault: true, + alias: { + "openclaw/plugin-sdk": pluginSdkStubPath, + }, +}); + +const { + generateReflectionText, + buildReflectionPromptParts, +} = jiti("../index.ts"); + +// Every static block of the distiller prompt, by its opening sentinel. These +// must ALL live in the system part and NONE may leak into the user part. +const STATIC_SENTINELS = [ + "You are generating a durable MEMORY REFLECTION entry for an AI assistant system.", + "Use these headings exactly once, in this exact order, with exact spelling:", + "Hard rules:", + "Section rules:", + "Governance section rules:", + "Notes:", + "OUTPUT TEMPLATE (copy this structure exactly):", + "- This run showed ...", +]; + +const CONVERSATION = "user: hello there\nassistant: hi, noted"; + +// loadEmbeddedPiRunner caches the first Layer-1 runner it resolves +// module-wide, so every test in this file shares ONE fake api and swaps +// behavior via this dispatcher (same pattern as raw-run-distiller-hooks). +let currentRunnerImpl = async () => ({ payloads: [{ text: "noop" }] }); +const fakeApi = { + runtime: { + agent: { + runEmbeddedPiAgent: (params) => currentRunnerImpl(params), + }, + }, +}; + +const baseParams = { + conversation: CONVERSATION, + maxInputChars: 1000, + cfg: {}, + agentId: "agent-one", + workspaceDir: "/tmp", + timeoutMs: 2000, + thinkLevel: "off", + api: fakeApi, +}; + +describe("buildReflectionPromptParts slot placement", () => { + it("puts the identity opener and every static block in system, none of them in user", () => { + const parts = buildReflectionPromptParts(CONVERSATION, 1000, []); + assert.ok( + parts.system.startsWith("You are generating a durable MEMORY REFLECTION entry"), + "system must open with the distiller identity line", + ); + for (const sentinel of STATIC_SENTINELS) { + assert.ok(parts.system.includes(sentinel), `system must contain static block sentinel: ${sentinel}`); + assert.ok(!parts.user.includes(sentinel), `user must NOT contain static block sentinel: ${sentinel}`); + } + }); + + it("user carries ONLY the dynamic content: tool error signals then the INPUT fence", () => { + const parts = buildReflectionPromptParts(CONVERSATION, 1000, []); + assert.ok(parts.user.startsWith("Recent tool error signals:"), "user must open with the tool-error block"); + assert.ok(parts.user.includes("- (none)"), "empty signals render the placeholder"); + assert.ok(parts.user.includes(`INPUT:\n\`\`\`\n${CONVERSATION}\n\`\`\``), "user must carry the fenced transcript"); + assert.ok(!parts.user.includes("## Context (session background)"), "the headings contract must not leak into user"); + assert.ok(!parts.system.includes(CONVERSATION), "the transcript must not leak into system"); + }); + + it("renders tool error signals dynamically in user, leaving system untouched", () => { + const signals = [{ toolName: "exec", summary: "exit 1 on build", signatureHash: "abcdef1234567890" }]; + const withSignals = buildReflectionPromptParts(CONVERSATION, 1000, signals); + const without = buildReflectionPromptParts(CONVERSATION, 1000, []); + assert.ok(withSignals.user.includes("1. [exec] exit 1 on build (sig:abcdef12)")); + assert.equal(withSignals.system, without.system, "signals are dynamic content; system must be static"); + }); + + it("keeps the prompt text verbatim: system + blank line + user reproduces the single-slot form", () => { + const parts = buildReflectionPromptParts(CONVERSATION, 1000, []); + const combined = `${parts.system}\n\n${parts.user}`; + assert.ok( + combined.includes("- This run showed ...\n\nRecent tool error signals:"), + "the static tail must flow into the dynamic block exactly as the legacy single-slot prompt did", + ); + assert.ok(combined.endsWith("```"), "the combined form still ends with the INPUT fence close"); + }); + + it("clips the conversation to maxInputChars from the tail, in user only", () => { + const long = "x".repeat(50) + "TAIL-MARKER"; + const parts = buildReflectionPromptParts(long, 20, []); + assert.ok(parts.user.includes("TAIL-MARKER")); + assert.ok(!parts.user.includes("x".repeat(30)), "only the last maxInputChars survive"); + }); +}); + +describe("embedded distiller run sends the combined single-slot form", () => { + it("passes system + user as one prompt and no system-slot param", async () => { + let seenParams = null; + currentRunnerImpl = async (params) => { + seenParams = params; + return { payloads: [{ text: "reflection text" }] }; + }; + + const result = await generateReflectionText(baseParams); + assert.equal(result.runner, "embedded"); + assert.ok(seenParams, "the embedded runner must have been invoked"); + + const parts = buildReflectionPromptParts(CONVERSATION, baseParams.maxInputChars, []); + assert.equal( + seenParams.prompt, + `${parts.system}\n\n${parts.user}`, + "the embedded raw run has no honored system slot, so the prompt must be the combined single-slot form", + ); + assert.equal( + seenParams.systemPromptOverride, + undefined, + "no system-slot param may be passed; a host whose runner honors one can deliver the parts split", + ); + assert.equal(seenParams.modelRun, true, "raw-run semantics must survive the parts refactor"); + assert.equal(seenParams.promptMode, "minimal"); + }); +}); + +describe("CLI fallback keeps the single-slot combined form (cannot set a system prompt)", () => { + const stubDir = mkdtempSync(path.join(tmpdir(), "reflection-cli-stub-")); + const dumpPath = path.join(stubDir, "argv.dump"); + const stubPath = path.join(stubDir, "fake-openclaw.sh"); + writeFileSync( + stubPath, + `#!/bin/sh\nprintf '%s\\0' "$@" > "${dumpPath}"\necho '{"payloads":[{"text":"cli reflection text"}]}'\n`, + "utf-8", + ); + chmodSync(stubPath, 0o755); + + after(() => rmSync(stubDir, { recursive: true, force: true })); + + it("sends system + user as one --message payload", async () => { + currentRunnerImpl = async () => { + throw new Error("forced embedded failure"); + }; + const originalCliBin = process.env.OPENCLAW_CLI_BIN; + process.env.OPENCLAW_CLI_BIN = stubPath; + try { + const result = await generateReflectionText(baseParams); + assert.equal(result.runner, "cli"); + assert.equal(result.text, "cli reflection text"); + } finally { + if (originalCliBin === undefined) delete process.env.OPENCLAW_CLI_BIN; + else process.env.OPENCLAW_CLI_BIN = originalCliBin; + } + + assert.ok(existsSync(dumpPath), "the CLI stub must have captured argv"); + const argv = readFileSync(dumpPath, "utf-8").split("\0"); + const messageIdx = argv.indexOf("--message"); + assert.ok(messageIdx > 0, "--message flag expected in CLI argv"); + const message = argv[messageIdx + 1]; + const parts = buildReflectionPromptParts(CONVERSATION, baseParams.maxInputChars, []); + assert.equal( + message, + `${parts.system}\n\n${parts.user}`, + "the CLI fallback has no system slot, so it must carry the combined single-slot prompt", + ); + }); +});