`promoteSnapshotSections` accepted only ONE carrier spelling: `content: []`.
The plugin never emits that shape. `lib/index.js:932` always attaches exactly
one `{ type: 'text', text: injected }` block whose text IS `sections[0].text`,
so `promotedSection`'s `if (blocks.length > 0) return null` matched on every
real recall and the promotion was a no-op in production. Recall stayed in
user/message history — the exact defect 6d9ad62 claimed to fix.
6d9ad62's "surface recall as a system section instead of history" was never
true: the promotion it relies on was blocked by that content-non-empty check.
The unit tests could not see it because their `snapshotMessage()` helper built
`content: []` — a shape no producer emits — and the old P4 asserted "real
content blocks are NOT promoted", which is the opposite of reality.
It was confirmed today, from `lib/index.js:932`'s constant one-text-block
injection, and is fixed here.
The rule is now "move the text, don't rewrite it": a snapshot carrier with
content is promoted only when the rendered blocks equal the sections' text
under one of the two canonical joins (`''` glued, `'\n\n'` spaced), and the
promoted section text is the rendered content, so relocation is assertable.
Non-text blocks, mismatched text, and the stock runtime-context snapshot (whose
content carries a preamble the sections do not) pass through untouched.
Tests: S1-S7 plus real multi-block S2c/S2d pin the acceptance surface; every
negative case carries a positive control in the same decision, so the old
"content non-empty => reject" rule cannot satisfy it. P4 is split into P4a/b
(content == sections promotes; content != sections does not) with P4c proving
the split is decided by text, not by presence of content.
`test/loop-system-e2e.test.js` mounts the real HologramAgentLoop on real dsh
services, runs one turn, and asserts on the request the adapter received: the
recall is in the system prompt (E1) and in no non-system message body (E2),
carried by exactly one message (E2b).
174 lines
8.1 KiB
JavaScript
174 lines
8.1 KiB
JavaScript
/**
|
||
* HologramAgent loop: surface recall as a SYSTEM section instead of history.
|
||
*
|
||
* Why this exists
|
||
* ---------------
|
||
* `dsh-memory` injects per-turn recall through the `agent/pre-step` waterfall,
|
||
* and that hook can only return `messages`. Whatever it adds therefore lands in
|
||
* conversation history as a user message — the model reads its own memory as if
|
||
* a user had said it. `systemPrompt.context` cannot fix this either: that hook
|
||
* runs before the turn text is known (documented in dsh-memory's own module
|
||
* note), so it can only render input-INDEPENDENT text.
|
||
*
|
||
* The loop's own `preStep` is the first place that has BOTH the assembled
|
||
* prompt and the claimed messages for this turn. So the swap point is here:
|
||
* let the existing pre-step hooks run unchanged, then move the recall block out
|
||
* of `messages` and into `assembly.sections`. The memory becomes part of the
|
||
* system prompt for that turn, and the user message never enters history.
|
||
*
|
||
* Coupling
|
||
* --------
|
||
* None, deliberately. We do not import the memory plugin or match its name; we
|
||
* recognise the *shape* every snapshot-style injector already uses
|
||
* (`source.sections = [{ name, text }]`), which is the same shape
|
||
* `dsh-time-context` emits. Anything a plugin chose to label that way is
|
||
* promoted; everything else passes through untouched.
|
||
*/
|
||
|
||
import { AgentLoop } from '@deepseek-ai/dsh-agent-loop'
|
||
|
||
/** Suffix appended to the promoted section name, keeping provenance visible. */
|
||
const PROMOTED_SUFFIX = ':system'
|
||
|
||
/**
|
||
* Split one message's `source.sections` out of the message itself.
|
||
*
|
||
* A snapshot carrier is recognised by ONE of two canonical spellings, both of
|
||
* which real producers emit:
|
||
*
|
||
* 1. `content: []` — a bare carrier. The sections are the whole payload.
|
||
* 2. `content: [{ type: 'text', text }]` — what this plugin itself emits
|
||
* (`lib/index.js:932`): one text block whose text is the sections' text
|
||
* joined by `\n\n` (or, from other producers, concatenated with no
|
||
* separator). Both conventions are accepted because the joining choice
|
||
* belongs to the producer, not to this loop.
|
||
*
|
||
* ★ The invariant is "move the text, do not rewrite it". So when content is
|
||
* present, it is promoted ONLY if the rendered blocks equal the sections'
|
||
* text under one of those two conventions, and the promoted section text is
|
||
* the *rendered content*, not a re-join of the sections. That makes
|
||
* "relocation" assertable in a test (`promoted.text === original content
|
||
* text`) and leaves no room for a silent edit. Anything else — a differing
|
||
* text, a non-text block, a runtime-context snapshot whose content carries a
|
||
* fixed preamble the sections do not — stays in history untouched.
|
||
*
|
||
* History: the previous rule was "content non-empty ⇒ reject", derived from an
|
||
* assumption ("content means the plugin wants the user to see it") that is
|
||
* false for this plugin's own output, so on the real path the promotion could
|
||
* never fire. See `test/loop.test.js` S1–S7.
|
||
*
|
||
* @param {object} message a user message from a pre-step decision
|
||
* @returns {{text: string, name: string} | null}
|
||
*/
|
||
function promotedSection(message) {
|
||
const source = message === null || typeof message !== 'object' ? undefined : message.source
|
||
if (source === null || typeof source !== 'object') return null
|
||
if (source.form !== 'snapshot') return null
|
||
if (!Array.isArray(source.sections) || source.sections.length === 0) return null
|
||
const parts = []
|
||
for (const section of source.sections) {
|
||
if (section !== null && typeof section === 'object' && typeof section.text === 'string' && section.text !== '') {
|
||
parts.push(section.text)
|
||
}
|
||
else if (typeof section === 'string' && section !== '') parts.push(section)
|
||
}
|
||
if (parts.length === 0) return null
|
||
const name = typeof source.kind === 'string' && source.kind !== '' ? source.kind : 'memory'
|
||
|
||
const blocks = Array.isArray(message.content) ? message.content : []
|
||
if (blocks.length === 0) return { text: parts.join('\n\n'), name }
|
||
|
||
// Any non-text block means this is not a carrier of prompt text: an image
|
||
// (or a block whose text is not a string) would be silently destroyed by a
|
||
// text-only move, so refuse the whole message.
|
||
for (const block of blocks) {
|
||
if (block === null || typeof block !== 'object') return null
|
||
if (block.type !== 'text' || typeof block.text !== 'string') return null
|
||
}
|
||
|
||
// Both canonical joins are accepted; the promoted text is the RENDERED
|
||
// content, which is what the model would have read, byte for byte.
|
||
const rendered = blocks.map((block) => block.text).join('')
|
||
const glued = parts.join('')
|
||
const spaced = parts.join('\n\n')
|
||
if (rendered !== glued && rendered !== spaced) return null
|
||
return { text: rendered, name }
|
||
}
|
||
|
||
/**
|
||
* Rewrite one pre-step decision so snapshot sections live in the prompt.
|
||
*
|
||
* The returned object is a NEW decision: `super.step()` keeps the one it
|
||
* received, and the loop's phase bookkeeping reads `assembly` off whatever it
|
||
* is handed. Mutating in place would work today and would silently break the
|
||
* first time the loop reuses a decision.
|
||
*
|
||
* @param {object} decision the decision returned by the pre-step waterfall
|
||
* @returns {object} the same decision shape with promoted sections moved
|
||
*/
|
||
export function promoteSnapshotSections(decision) {
|
||
if (decision === null || typeof decision !== 'object') return decision
|
||
if (decision.kind === 'reject') return decision
|
||
const messages = Array.isArray(decision.messages) ? decision.messages : []
|
||
const assembly = decision.assembly
|
||
if (assembly === null || typeof assembly !== 'object' || !Array.isArray(assembly.sections)) return decision
|
||
|
||
const kept = []
|
||
const promoted = []
|
||
for (const message of messages) {
|
||
const section = promotedSection(message)
|
||
if (section === null) kept.push(message)
|
||
else promoted.push(section)
|
||
}
|
||
if (promoted.length === 0) return decision
|
||
|
||
const sections = [...assembly.sections]
|
||
const seen = new Set(sections.map((section) => section.name))
|
||
for (const section of promoted) {
|
||
// A name collision means some other producer already owns that slot. The
|
||
// prompt layer throws on duplicates, so suffix rather than overwrite:
|
||
// dropping the content would hide memory with no visible failure.
|
||
let name = `${section.name}${PROMOTED_SUFFIX}`
|
||
for (let n = 2; seen.has(name); n += 1) name = `${section.name}${PROMOTED_SUFFIX}${n}`
|
||
seen.add(name)
|
||
// `interpolate: false` is mandatory, not cosmetic: recalled notes are free
|
||
// text and may legitimately contain `{{...}}`, which the strict interpolator
|
||
// rejects with `unknown prompt variable` (measured — dsh-memory hit this).
|
||
sections.push({ name, text: section.text, interpolate: false })
|
||
}
|
||
|
||
return { ...decision, messages: kept, assembly: { ...assembly, sections } }
|
||
}
|
||
|
||
/**
|
||
* The headless AgentLoop, with snapshot sections promoted into the prompt.
|
||
*
|
||
* No constructor and no static `inject`: the parent already registers the
|
||
* turnBoundary + inbox projections, calls `agents.setFactory(this)`, and
|
||
* declares the `{{provider}}` / `{{model}}` variables. Re-declaring any of them
|
||
* here would be the bug — `setFactory` throws if a factory is already
|
||
* registered, so a second registration is exactly what breaks startup.
|
||
*/
|
||
export class HologramAgentLoop extends AgentLoop {
|
||
/**
|
||
* Prepare one agent, then wrap its pre-step seam.
|
||
*
|
||
* Patching the *instance* (not the prototype) is deliberate: the prototype is
|
||
* shared with every agent the base class creates, including ones this plugin
|
||
* must not touch. An own property shadows it for this agent only.
|
||
*
|
||
* @param {...unknown} args forwarded verbatim to the parent
|
||
* @returns {{agent: object, signal: AbortSignal, publish: Function}}
|
||
*/
|
||
prepare(...args) {
|
||
const prepared = super.prepare(...args)
|
||
const agent = prepared === null || typeof prepared !== 'object' ? undefined : prepared.agent
|
||
if (agent === undefined || typeof agent.preStep !== 'function') return prepared
|
||
const original = agent.preStep.bind(agent)
|
||
agent.preStep = async (target, position) => promoteSnapshotSections(await original(target, position))
|
||
return prepared
|
||
}
|
||
}
|
||
|
||
export default HologramAgentLoop
|