From 0306ec9fb43ddf32aee8d93e955e28aa44fa61f4 Mon Sep 17 00:00:00 2001 From: Finn763 <165816600+Finn763@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:59:47 +0800 Subject: [PATCH] fix(memory-core): read dreaming narrative from the terminal reply instead of racing the session store (#123360) (#127184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(memory-core): read dreaming narrative from the terminal reply instead of racing the session store (#123360) A completed run's terminal reply is captured by the run registry at completion time and survives the window where the session store lags the run. Prefer it over readSettledNarrativeText, whose 1,250ms settle budget expires while a first-finishing phase's cleanup keeps the store empty, discarding every other phase's diary entry on multi-phase nights. The settle loop remains the bounded fallback when the run result carries no visible terminal reply. * fix(memory-core): respect explicit non-visible terminal replies (#123360) Review feedback: absent, silent, and empty terminal results were all falling back to transcript polling. An explicit silent/empty terminal reply is authoritative no-text — polling could resurface an older narrative over it. Classify the terminal reply into visible / non-visible / absent and only poll the store for absent (legacy runtime) results. Also compresses the selection to net +13 production lines. * test(memory-core): lock the non-visible terminal reply contract (#123360) Adversarial review hardening: cover the message-tool-not-called variant, assert that text beside a non-visible disposition is ignored, cover whitespace-only visible text, and use fake timers so a regression fails fast instead of after the settle budget. --- .../src/dreaming-narrative.test.ts | 133 ++++++++++++++++++ .../memory-core/src/dreaming-narrative.ts | 63 +++++++-- 2 files changed, 188 insertions(+), 8 deletions(-) diff --git a/extensions/memory-core/src/dreaming-narrative.test.ts b/extensions/memory-core/src/dreaming-narrative.test.ts index ba039d81b722..4a7985b954a5 100644 --- a/extensions/memory-core/src/dreaming-narrative.test.ts +++ b/extensions/memory-core/src/dreaming-narrative.test.ts @@ -567,6 +567,139 @@ describe("runDreamNarrative", () => { } }); + it("writes the terminal reply text without polling the session store", async () => { + vi.useFakeTimers(); + try { + const workspaceDir = await createTempWorkspace("openclaw-dreaming-narrative-"); + const subagent = createMockSubagent(""); + subagent.waitForRun.mockResolvedValue({ + status: "ok", + terminalReply: { + disposition: "visible", + text: "The terminal reply carried the diary safely.", + }, + }); + // Simulate the sibling-cleanup race from #123360: the store never shows the + // completed run's text within the settle window. + subagent.getSessionMessages.mockResolvedValue({ messages: [] }); + const logger = createMockLogger(); + + const operation = runDreamNarrative({ + agentId: "main", + subagent, + workspaceDir, + data: { + phase: "light", + snippets: ["The narrative raced a sibling phase's cleanup."], + }, + nowMs: Date.parse("2026-04-05T03:00:00Z"), + timezone: "UTC", + logger, + }); + await flushNarrativeSettleTimers(operation); + + expect(subagent.getSessionMessages).not.toHaveBeenCalled(); + const content = await fs.readFile(path.join(workspaceDir, "DREAMS.md"), "utf-8"); + expect(content).toContain("The terminal reply carried the diary safely."); + expect(content).not.toContain("A memory trace surfaced"); + expectLogExcludes(logger.warn, "produced no text"); + } finally { + vi.useRealTimers(); + } + }); + + it.each([ + { label: "empty", reply: { disposition: "empty" } }, + { label: "silent", reply: { disposition: "silent" } }, + // `message-tool-not-called` is an explicit no-text fact too; history must + // not resurface a transcript for it. + { + label: "empty (message-tool-not-called)", + reply: { disposition: "empty", code: "message-tool-not-called" }, + }, + ] as const)( + "treats an explicit $label terminal reply as authoritative no-text and never reads history", + async ({ reply }) => { + vi.useFakeTimers(); + try { + const workspaceDir = await createTempWorkspace("openclaw-dreaming-narrative-"); + const subagent = createMockSubagent(""); + subagent.waitForRun.mockResolvedValue({ + status: "ok", + // Any `text` beside a non-visible disposition must be ignored. + terminalReply: { ...reply, text: "text that must be ignored" }, + }); + // The store holds an OLD narrative from a previous run; reading it would + // resurface stale text over the authoritative no-text result. + subagent.getSessionMessages.mockResolvedValue({ + messages: [ + { role: "user", content: "prompt" }, + { role: "assistant", content: "A stale narrative from a previous run." }, + ], + }); + const logger = createMockLogger(); + + const operation = runDreamNarrative({ + agentId: "main", + subagent, + workspaceDir, + data: { + phase: "rem", + snippets: ["The run was silent, so history must not speak for it."], + }, + nowMs: Date.parse("2026-04-05T03:00:00Z"), + timezone: "UTC", + logger, + }); + await flushNarrativeSettleTimers(operation); + + expect(subagent.getSessionMessages).not.toHaveBeenCalled(); + const content = await fs.readFile(path.join(workspaceDir, "DREAMS.md"), "utf-8"); + expect(content).toContain("A memory trace surfaced"); + expect(content).not.toContain("A stale narrative from a previous run."); + expect(content).not.toContain("text that must be ignored"); + expectLogIncludes(logger.warn, "produced no text"); + } finally { + vi.useRealTimers(); + } + }, + ); + + it("treats a visible terminal reply with only whitespace as no-text", async () => { + const workspaceDir = await createTempWorkspace("openclaw-dreaming-narrative-"); + const subagent = createMockSubagent(""); + subagent.waitForRun.mockResolvedValue({ + status: "ok", + terminalReply: { disposition: "visible", text: " \n " }, + }); + subagent.getSessionMessages.mockResolvedValue({ + messages: [ + { role: "user", content: "prompt" }, + { role: "assistant", content: "A stale narrative from a previous run." }, + ], + }); + const logger = createMockLogger(); + + await runDreamNarrative({ + agentId: "main", + subagent, + workspaceDir, + data: { + phase: "light", + snippets: ["A blank diary page must not borrow yesterday's words."], + }, + nowMs: Date.parse("2026-04-05T03:00:00Z"), + timezone: "UTC", + logger, + }); + + expect(subagent.getSessionMessages).not.toHaveBeenCalled(); + const content = await fs.readFile(path.join(workspaceDir, "DREAMS.md"), "utf-8"); + expect(content).toContain("A memory trace surfaced"); + expect(content).not.toContain("A stale narrative from a previous run."); + expectLogIncludes(logger.warn, "produced no text"); + }); + it("falls back after settled assistant text never appears", async () => { vi.useFakeTimers(); try { diff --git a/extensions/memory-core/src/dreaming-narrative.ts b/extensions/memory-core/src/dreaming-narrative.ts index 658bc5e49c84..62a30ee1f5fd 100644 --- a/extensions/memory-core/src/dreaming-narrative.ts +++ b/extensions/memory-core/src/dreaming-narrative.ts @@ -33,10 +33,20 @@ export type SubagentSurface = { lightContext?: boolean; deliver?: boolean; }) => Promise<{ runId: string }>; - waitForRun: (params: { - runId: string; - timeoutMs?: number; - }) => Promise<{ status: string; error?: string }>; + waitForRun: (params: { runId: string; timeoutMs?: number }) => Promise<{ + status: string; + error?: string; + /** + * Authoritative final assistant text captured by the run registry while the + * raw text was still available. Preferred over polling the session store, + * which lags a completed run (see readSettledNarrativeText) and can stay + * empty past the settle budget when a sibling phase's cleanup races it (#123360). + */ + terminalReply?: { + disposition: "visible" | "silent" | "empty"; + text?: string; + }; + }>; getSessionMessages: (params: { sessionKey: string; limit?: number; @@ -388,6 +398,31 @@ async function readSettledNarrativeText(params: { return null; } +/** + * Classifies the run result's terminal reply for diary use. The terminal + * reply is the authoritative completion-time fact, so an explicit + * non-visible disposition (silent/empty) means "this run produced no diary + * text" — the transcript must not be read for it, or an older narrative from + * a previous run could resurface (#127184 review). Only an absent terminal + * reply (legacy runtime) falls back to the transcript. + */ +type TerminalReplyNarrative = + | { kind: "visible"; text: string } + | { kind: "non-visible" } + | { kind: "absent" }; + +function classifyTerminalReplyNarrative( + result: Awaited>, +): TerminalReplyNarrative { + const reply = result.terminalReply; + if (!reply) { + return { kind: "absent" }; + } + const text = + reply.disposition === "visible" && typeof reply.text === "string" ? reply.text.trim() : ""; + return text ? { kind: "visible", text } : { kind: "non-visible" }; +} + // ── Date formatting ──────────────────────────────────────────────────── function formatNarrativeDate(epochMs: number, timezone?: string): string { @@ -812,6 +847,7 @@ async function generateAndAppendDreamNarrative( await withNarrativeSessionLock(sessionKey, async () => { const attempts: Array<{ sessionKey: string; runId: string | null }> = []; let successfulSessionKey: string | null = null; + let terminalReply: TerminalReplyNarrative | null = null; try { const attemptModels = params.model ? [params.model, undefined] : [undefined]; @@ -858,6 +894,7 @@ async function generateAndAppendDreamNarrative( if (result.status === "ok") { successfulSessionKey = attemptSessionKey; + terminalReply = classifyTerminalReplyNarrative(result); break; } @@ -908,10 +945,20 @@ async function generateAndAppendDreamNarrative( return; } - const narrative = await readSettledNarrativeText({ - subagent: params.subagent, - sessionKey: successfulSessionKey, - }); + // Prefer the terminal reply the run registry captured at completion time. + // A visible reply is immune to the sibling-cleanup race (#123360); an + // explicit non-visible reply is authoritative no-text and must not read + // history (an older narrative could resurface); only an absent reply + // (legacy runtime) falls back to polling the session store. + const narrative = + terminalReply?.kind === "visible" + ? terminalReply.text + : terminalReply?.kind === "absent" + ? await readSettledNarrativeText({ + subagent: params.subagent, + sessionKey: successfulSessionKey, + }) + : null; if (!narrative) { params.logger.warn( `memory-core: narrative generation produced no text for ${params.data.phase} phase; writing fallback diary entry.`,