fix(phase2): never reset the baseline after a no-op consolidation

src/phase2.ts

@@ -8,6 +8,8 @@ import {
   writeWorkspaceDiff,
   validateConsolidationArtifactsForVersion,
   removeMemorySymlinks,
+  fingerprintConsolidationArtifacts,
+  diffCarriesNewLearning,
 } from "./workspace.js"
 import { ensureBaseline, captureWorkspaceDiff, resetBaseline, DIFF_ARTIFACT } from "./git-baseline.js"
 import {
@@ -254,6 +256,12 @@ async function runVersionPhase2(
 
       writeWorkspaceDiff(diff)
 
+      // Snapshot the artifacts the consolidator owns before handing it the diff.
+      // See fingerprintConsolidationArtifacts: a clean return from
+      // consolidateViaSubagent does not prove the agent wrote anything, and the
+      // contract lets it no-op. Compared after the helper closes.
+      const artifactsBeforeConsolidator = fingerprintConsolidationArtifacts(root)
+
       let heartbeatLost = false
       let heartbeatFailure: unknown = "ownership lost"
       const heartbeatOnce = (): boolean => {
@@ -345,6 +353,29 @@ async function runVersionPhase2(
         return { status: "failed_invalid_artifacts" }
       }
 
+      // The consolidation agent returned successfully but left MEMORY.md /
+      // memory_summary.md byte-identical while the diff carried new rollouts, so
+      // the new memories were never promoted. Validation cannot catch this: the
+      // stale files still exist and the summary header is still "v1".
+      //
+      // Resetting the baseline here would be the damaging part. It would fold the
+      // un-consolidated rollout summaries into the baseline, the next pass would
+      // then see zero changes, take the no_workspace_changes early return above,
+      // and never invoke the consolidator again — the new memories would be
+      // stranded in the workspace permanently and silently, with the job reported
+      // as succeeded. Keep the diff instead and let the normal failure backoff
+      // retry, so the run is observable and recoverable.
+      if (
+        fingerprintConsolidationArtifacts(root) === artifactsBeforeConsolidator &&
+        diffCarriesNewLearning(diff.changes)
+      ) {
+        store.markPhase2Failed(
+          claim.ownershipToken,
+          "consolidation agent made no artifact changes while the workspace diff carried new rollouts",
+        )
+        return { status: "no_artifact_changes" }
+      }
+
       if (!await withHostTimeout(resetBaseline(diff.extensionSnapshot), GIT_TIMEOUT_MS, "resetBaseline")) {
         store.markPhase2Failed(claim.ownershipToken, "baseline reset failed")
         return { status: "baseline_reset_failed" }

src/workspace.ts

@@ -106,6 +106,78 @@ export function isValidV2Summary(summary: string): boolean {
   return V2_SUMMARY_HEADINGS.every((heading) => lines.includes(heading))
 }
 
+/**
+ * Artifacts the consolidation agent owns. `raw_memories.md` and
+ * `rollout_summaries/` are phase-1 prep output, not consolidation output, so
+ * they are deliberately excluded: a change there says nothing about whether the
+ * agent promoted anything into the durable artifacts.
+ */
+const CONSOLIDATION_ARTIFACTS = ["MEMORY.md", "memory_summary.md"]
+
+/**
+ * Fingerprint of the consolidation artifacts, for detecting a no-op run.
+ *
+ * `consolidateViaSubagent` resolves as soon as the helper session closes and
+ * throws only on prompt/timeout/shutdown failure. The consolidation contract
+ * also explicitly permits a no-op ("No-op content updates are allowed and
+ * preferred when there is no meaningful, reusable learning worth saving"), so a
+ * clean return is NOT evidence that MEMORY.md / memory_summary.md were updated.
+ * `validateConsolidationArtifactsForVersion` cannot close that gap either: it
+ * only proves the files exist and the summary header is intact, which stays true
+ * for artifacts that are merely stale.
+ *
+ * Comparing this fingerprint before and after the helper runs is the only host
+ * signal that a consolidation actually happened. Uses size + mtime rather than
+ * content hashing: it runs once per phase-2 attempt over two small files, and
+ * mtime is what git's own status matrix keys on.
+ */
+export function fingerprintConsolidationArtifacts(root: string = memoryRoot()): string {
+  const parts: string[] = []
+  for (const name of CONSOLIDATION_ARTIFACTS) {
+    try {
+      const st = fs.lstatSync(path.join(root, name))
+      parts.push(st.isFile() ? `${name}:${st.size}:${st.mtimeMs}` : `${name}:not-a-file`)
+    } catch {
+      // v2 does not use MEMORY.md, so "missing" is a legitimate steady state.
+      parts.push(`${name}:missing`)
+    }
+  }
+  try {
+    const skills = fs
+      .readdirSync(path.join(root, SKILLS_DIR), { withFileTypes: true })
+      .map((entry) => entry.name)
+      .sort()
+    parts.push(`${SKILLS_DIR}:${skills.join(",")}`)
+  } catch {
+    parts.push(`${SKILLS_DIR}:none`)
+  }
+  return parts.join("|")
+}
+
+/**
+ * True when the workspace diff carries learning material the consolidator was
+ * required to propagate.
+ *
+ * Scoped to the two input families the consolidation contract names as
+ * consolidation triggers: added/modified/deleted files under
+ * `rollout_summaries/`, and resources under `extensions/<name>/resources/`.
+ * `raw_memories.md` is deliberately excluded even though it is derived from the
+ * same inputs, because `rebuildRawMemories([])` always writes a placeholder
+ * ("No raw memories yet."), so on a first INIT run it would register as new learning material on its own and turn a legitimately
+ * empty consolidation into a permanent retry loop. A new or updated stage-1
+ * output always changes the rollout_summaries set too (the file stem embeds
+ * source_updated_at), so nothing real is lost by keying on rollouts alone.
+ */
+export function diffCarriesNewLearning(changes: readonly { path: string }[]): boolean {
+  return changes.some(({ path: changed }) => {
+    if (changed.startsWith(`.${ROLLOUT_DIR}/`) || changed.startsWith(`${ROLLOUT_DIR}/.`)) return false
+    if (changed.startsWith(`${ROLLOUT_DIR}/`)) return true
+    // extensions/<name>/resources/<file>.md
+    const parts = changed.split("/")
+    return parts.length >= 3 && parts[0] === EXTENSIONS_DIR && parts[1] !== "." && parts[2] === "resources"
+  })
+}
+
 export function validateConsolidationArtifacts(root: string = memoryRoot()): { ok: true } | { ok: false; reason: string } {
   return validateConsolidationArtifactsForVersion(root, "v1")
 }

tests/phase2.test.ts

@@ -149,6 +149,82 @@
     expect(fs.existsSync(path.join(memoryRoot(), "phase2_workspace_diff.md"))).toBe(true)
   })
 
+  it("fails a successful-but-empty consolidation and keeps the diff so the next run retries", async () => {
+    // The consolidation contract explicitly permits a no-op: "No-op content
+    // updates are allowed and preferred when there is no meaningful, reusable
+    // learning worth saving." The helper can therefore read the diff, change
+    // nothing, and exit cleanly. That is not a consolidation, and it must not be
+    // recorded as one.
+    const store = new MemoryStore()
+    const ts = Date.now()
+    store.upsertStage1Output({
+      session_id: "ses_noop",
+      source_updated_at: ts,
+      raw_memory: "### Task 1: reproducible fact\n\n- the retry budget is per-job, not global",
+      rollout_summary: "# Reproducible fact\n\nThe stage-1 retry budget is per-job.",
+      rollout_slug: "reproducible-fact",
+      cwd: "/p",
+      generated_at: ts,
+    })
+    setPluginInput({
+      client: {
+        session: {
+          create: async () => ({ data: { id: "sub-phase2-noop" } }),
+          prompt: async () => ({ data: { info: {}, parts: [{ type: "text", text: "nothing worth saving" }] } }),
+          delete: async () => ({ data: {} }),
+          get: async (req: { path: { id: string } }) => ({ data: { id: req.path.id }, response: { status: 200 } }),
+        },
+        config: { get: async () => ({ data: {} }) },
+      },
+    } as any)
+
+    const result = await runPhase2(new MemoryStore())
+    expect(result.status).toBe("no_artifact_changes")
+
+    const job = openDb()
+      .prepare("SELECT status, lease_until, retry_at, last_error FROM memory_jobs WHERE kind='memory_consolidate_global'")
+      .get() as { status: string; lease_until: number | null; retry_at: number | null; last_error: string }
+    expect(job.status).toBe("failed")
+    expect(job.lease_until).toBeNull()
+    expect(job.last_error).toMatch(/no artifact changes/)
+    // Failure backoff, not a hot loop.
+    expect(job.retry_at).toBeGreaterThan(0)
+
+    // The load-bearing assertion. If the baseline had been reset here, the next
+    // pass would see zero changes, return no_workspace_changes, and never invoke
+    // the consolidator again — stranding this rollout in the workspace forever
+    // while the job reported success.
+    const pending = await captureWorkspaceDiff()
+    expect(pending.changes.some((change) => change.path.startsWith("rollout_summaries/"))).toBe(true)
+  })
+
+  it("accepts a no-op consolidation when the diff carries no new learning material", async () => {
+    // No stage-1 outputs: prep writes the raw_memories.md placeholder and no
+    // rollout summaries, so there is genuinely nothing to promote and a no-op is
+    // the correct outcome. Pre-seeding a valid baseline keeps the diff to the
+    // placeholder file, which must not be treated as learning material.
+    const { resetBaseline } = require("../src/git-baseline.js")
+    const root = memoryRoot()
+    fs.mkdirSync(root, { recursive: true })
+    fs.writeFileSync(path.join(root, "MEMORY.md"), "# MEMORY.md\n")
+    fs.writeFileSync(path.join(root, "memory_summary.md"), "v1\n\n## User Profile\n")
+    await resetBaseline(new Map())
+
+    setPluginInput({
+      client: {
+        session: {
+          create: async () => ({ data: { id: "sub-phase2-noop-clean" } }),
+          prompt: async () => ({ data: { info: {}, parts: [{ type: "text", text: "nothing to add" }] } }),
+          delete: async () => ({ data: {} }),
+        },
+        config: { get: async () => ({ data: {} }) },
+      },
+    } as any)
+
+    const result = await runPhase2(new MemoryStore())
+    expect(result.status).toBe("succeeded")
+  })
+
   it("v2 succeeds with only memory_summary.md and does not write raw_memories.md", async () => {
     const { applyPluginOptions } = require("../src/index.js")
     applyPluginOptions({ version: "v2" })

tests/workspace.test.ts

@@ -273,6 +273,78 @@ describe("validateConsolidationArtifacts", () => {
   })
 })
 
+describe("fingerprintConsolidationArtifacts", () => {
+  it("is stable when nothing is written and changes as soon as an artifact is touched", async () => {
+    const { ensureLayout, fingerprintConsolidationArtifacts } = require("../src/workspace.js")
+    const { memoryRoot } = require("../src/paths.js")
+    const root = memoryRoot()
+    ensureLayout()
+    fs.writeFileSync(path.join(root, "MEMORY.md"), "# MEMORY.md\n")
+    fs.writeFileSync(path.join(root, "memory_summary.md"), "v1\n\nprofile\n")
+
+    const before = fingerprintConsolidationArtifacts(root)
+    expect(fingerprintConsolidationArtifacts(root)).toBe(before)
+
+    // Stale-but-valid artifacts must still fingerprint as unchanged: this is
+    // exactly the state a no-op consolidation leaves behind, and detecting it is
+    // the whole point of the helper.
+    expect(validateStaleStillValid(root)).toBe(true)
+
+    // mtime granularity can hide a same-millisecond write, so bump it explicitly.
+    const future = new Date(Date.now() + 5000)
+    fs.utimesSync(path.join(root, "memory_summary.md"), future, future)
+    expect(fingerprintConsolidationArtifacts(root)).not.toBe(before)
+  })
+
+  it("reports a missing MEMORY.md as a steady state rather than throwing (v2)", () => {
+    const { fingerprintConsolidationArtifacts } = require("../src/workspace.js")
+    const { memoryRoot } = require("../src/paths.js")
+    const root = memoryRoot()
+    fs.mkdirSync(root, { recursive: true })
+    expect(fingerprintConsolidationArtifacts(root)).toContain("MEMORY.md:missing")
+  })
+
+  it("notices a newly created skill directory", () => {
+    const { fingerprintConsolidationArtifacts } = require("../src/workspace.js")
+    const { memoryRoot } = require("../src/paths.js")
+    const root = memoryRoot()
+    fs.mkdirSync(path.join(root, "skills"), { recursive: true })
+    const before = fingerprintConsolidationArtifacts(root)
+    fs.mkdirSync(path.join(root, "skills", "new-skill"), { recursive: true })
+    expect(fingerprintConsolidationArtifacts(root)).not.toBe(before)
+    expect(fingerprintConsolidationArtifacts(root)).toContain("skills:new-skill")
+  })
+})
+
+describe("diffCarriesNewLearning", () => {
+  it("flags rollout summary additions, modifications and deletions", () => {
+    const { diffCarriesNewLearning } = require("../src/workspace.js")
+    expect(diffCarriesNewLearning([{ path: "rollout_summaries/2026-07-03T05-11-22-abcd-fix.md" }])).toBe(true)
+    expect(diffCarriesNewLearning([{ path: "raw_memories.md" }])).toBe(false)
+    expect(diffCarriesNewLearning([{ path: "MEMORY.md" }])).toBe(false)
+    expect(diffCarriesNewLearning([])).toBe(false)
+  })
+
+  it("flags extension resources, which the contract also treats as consolidation input", () => {
+    const { diffCarriesNewLearning } = require("../src/workspace.js")
+    expect(diffCarriesNewLearning([{ path: "extensions/codex_memory_import/resources/a.md" }])).toBe(true)
+    // instructions.md is scaffolding, not learned content.
+    expect(diffCarriesNewLearning([{ path: "extensions/ad_hoc/instructions.md" }])).toBe(false)
+  })
+
+  it("ignores a raw_memories.md-only diff so a first INIT run cannot loop", () => {
+    // rebuildRawMemories([]) always writes the "No raw memories yet." placeholder,
+    // so on a fresh workspace that file alone must not count as new learning.
+    const { diffCarriesNewLearning } = require("../src/workspace.js")
+    expect(diffCarriesNewLearning([{ path: "raw_memories.md" }, { path: "skills/.keep" }])).toBe(false)
+  })
+})
+
+function validateStaleStillValid(root: string): boolean {
+  const { validateConsolidationArtifactsForVersion } = require("../src/workspace.js")
+  return validateConsolidationArtifactsForVersion(root, "v1").ok
+}
+
 describe("pruneExtensionResources", () => {
   it("prunes old timestamped resources but never notes or instructions.md", () => {
     const { ensureLayout, pruneExtensionResources } = require("../src/workspace.js")