fix(consolidate): follow-ups from the #426 review (Refs #40)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Consolidation follow-ups (task #439): an accepted consolidation note is never a source of the next run (no recurring-fix or chain summaries of summaries; an accepted recurring-fix summary no longer pairs with its own sources), the report counts only newly queued or still-pending candidates and says how many were already reviewed, `amem consolidate --project` refuses a project other than the current one (its repository policy cannot be resolved from here), and acceptance reads one placement proof per locked section instead of four.
 - On-demand deterministic consolidation proposals and a bounded `amem_evidence` MCP bundle (agent-mem#40 F18/F19). Consolidation writes machine-local candidates, with person-only exact-revision acceptance; curated v1 ops, hashes and paths are unchanged. Candidate readers must understand the new consolidation payload before sharing a home that uses it; older candidate readers refuse the unknown kind. Evidence contains live project-curated citations only, makes no model call, and records only confirmed rendered output.
 
 ## 0.7.0

README.md

@@ -453,7 +453,7 @@ The library exports `buildIntentBrief`, `planGuardNodes`, `matchingPlanFails` an
 
 ## Consolidation and evidence
 
-Run `amem consolidate [--project <active-key>]` to queue deterministic proposals for `amem review`. It compares a bounded project-local window using indexed text overlap, shared paths and current supersession-chain metadata. It does not edit curated notes. The person reviews each merge, replacement or recurring-failure summary, and acceptance rechecks every source revision. A recurring-failure summary preserves its source failures and decisions. Large source quotations are counted as too large instead of silently shortened into a proposed note.
+Run `amem consolidate [--project <current-key>]` (another project is refused) to queue deterministic proposals for `amem review`. It compares a bounded project-local window using indexed text overlap, shared paths and current supersession-chain metadata. It does not edit curated notes. The person reviews each merge, replacement or recurring-failure summary, and acceptance rechecks every source revision. A recurring-failure summary preserves its source failures and decisions. Large source quotations are counted as too large instead of silently shortened into a proposed note.
 
 The CLI runs the deterministic pass only. A library host may explicitly enable an injected model callback for the person; no provider is built in. It has batch, byte, deadline and persistent daily limits. Model answers remain marked machine-local candidates that cannot be accepted; `amem note add` is the separate path for an independently authored note. A non-answer is never saved.
 

docs/contracts.md

@@ -416,7 +416,7 @@ agent-mem#40 F14 asks for query expansion from aliases and tags and for BM25 plu
 
 ## On-demand consolidation and evidence (F18/F19)
 
-`amem consolidate [--project <active-key>]` is a person-only candidate writer, never a curated-memory writer. It screens one project-local snapshot under one guarded policy revision, then uses at most 256 verified-synced live rows and emits at most 32 deterministic proposals. FTS5 overlap plus shared paths suggests a merge; identical text suggests a replacement; a current head's retained supersession metadata gives chain context. FTS unavailability uses literal token overlap and is reported. A pair of related fail notes can propose a recurring-failure summary only when a related actual decision or contract is present. The proposal asks the person to assess that relationship and never invents a fix. Every quoted source is preserved whole; a proposal that cannot fit the candidate/note bounds is counted as oversized. Truncation is explicit.
+`amem consolidate [--project <current-key>]` is a person-only candidate writer, never a curated-memory writer. It screens one project-local snapshot under one guarded policy revision, then uses at most 256 verified-synced live rows and emits at most 32 deterministic proposals. FTS5 overlap plus shared paths suggests a merge; identical text suggests a replacement; a current head's retained supersession metadata gives chain context. FTS unavailability uses literal token overlap and is reported. A pair of related fail notes can propose a recurring-failure summary only when a related actual decision or contract is present. The proposal asks the person to assess that relationship and never invents a fix. An accepted consolidation note (first line `Accepted consolidation candidate:`) is never a source: it neither seeds a fail or recurring-fix proposal nor a chain summary, so repeated runs do not summarize their own summaries. `--project` must be the current project (the repository policy that narrows screening is the working directory's). The report counts only candidates newly queued or still pending as queued; proposals whose candidate was already accepted or rejected are counted as `alreadyReviewed`. Every quoted source is preserved whole; a proposal that cannot fit the candidate/note bounds is counted as oversized. Truncation is explicit.
 
 Consolidation payloads live only in `candidates.db`, with the existing candidate hash/retention and person review flow. Curated shared-v1 schema, ops, hashes and paths do not change. A host with an older candidate reader must be upgraded before this payload is written into a shared home; older readers refuse an unknown candidate kind. Acceptance binds all exact source revisions, home/profile, project, placement and scope, and screens every source and the complete resulting note before appending. A stable host policy revision is required across all source verdicts and before effects. Transport placement comes from verified legacy segment locations (32 MiB read budget) or the generation index, never from tier or View.local. Unknown placement refuses the operation, local sources are excluded and counted, and local or mixed-placement acceptance refuses before any append. Merge and replacement acceptance supersede their sources; recurring-failure summaries keep their sources live. A rejected candidate remains rejected when the same proposal is generated again.
 

src/cli.ts

@@ -232,6 +232,8 @@ export async function runCli(argv: readonly string[], context: CliContext = {}):
     const requested = typeof options["--project"] === "string" ? options["--project"] as string : boundProject();
     if (!requested.trim() || requested.length > 400 || requested.includes("\0")) throw Error("invalid consolidation project");
     const base = memoryHome(), project = resolveAlias(base, requested), aliases = aliasedKeys(base, project);
+    // The policy below is narrowed by the repository around the working directory, which is the bound project's. Another project's own repository policy cannot be resolved from here, so screening it with this one would apply the wrong narrowing.
+    if (project !== boundProject()) throw Error("consolidate --project must name the current project; run it from the other project's directory to consolidate that one");
     assertProjectActive(base, project);
     const store = curatedStore(project, { deferPromotions: true, screen: () => "keep" });
     let candidates: CandidateStore | undefined;
@@ -240,7 +242,7 @@ export async function runCli(argv: readonly string[], context: CliContext = {}):
       const assertCurrent = () => { binding(); assertProjectActive(base, project); if (resolveAlias(base, requested) !== project || aliasedKeys(base, project).join("\n") !== aliases.join("\n")) throw Error("consolidation project binding changed"); };
       candidates = new CandidateStore({ memoryDir: base, reconcile: { canonical: project, aliases } });
       const report = await proposeConsolidation(store, candidates, { project, actor: "person", policy: policyOf, policyVersion: policyKey, assertCurrent });
-      return options["--json"] ? JSON.stringify(report, null, 2) : `${report.candidates.length} consolidation candidates queued for amem review; no curated notes changed${report.truncated ? "; scan truncated" : ""}${report.oversized ? `; ${report.oversized} proposals too large` : ""}${report.localExcluded ? `; ${report.localExcluded} local sources excluded` : ""}`;
+      return options["--json"] ? JSON.stringify(report, null, 2) : `${report.candidates.length} consolidation candidates queued for amem review; no curated notes changed${report.alreadyReviewed ? `; ${report.alreadyReviewed} already reviewed` : ""}${report.truncated ? "; scan truncated" : ""}${report.oversized ? `; ${report.oversized} proposals too large` : ""}${report.localExcluded ? `; ${report.localExcluded} local sources excluded` : ""}`;
     } finally { candidates?.close(); store.close(); }
   }
   if (sub === "brief") {

src/commands-table.ts

@@ -24,7 +24,7 @@ const filters = { ...json, "--tier": { description: "Memory tier", value: ["shor
 export const GLOBAL_OPTIONS = { "--help": flag("Show help") };
 export const ROOT_OPTIONS = { "--version": flag("Show version"), ...json };
 export const COMMANDS: Record<string, CommandDefinition> = {
-  consolidate: { description: "Queue deterministic project-local consolidation candidates; curated notes change only after person review", mutates: true, options: { ...json, "--project": text("Explicit active project key (default current project)") } },
+  consolidate: { description: "Queue deterministic project-local consolidation candidates; curated notes change only after person review", mutates: true, options: { ...json, "--project": text("The current project's key; another project is refused (run from its directory)") } },
   setup: { description: "Preview reversible MCP and hook setup for native coding CLIs; apply and undo are person-only", options: { "--apply": flag("Apply after terminal confirmation"), "--undo": flag("Restore original configuration after confirmation") }, args: [{ name: "target", values: ["claude", "codex", "kimi", "pi", "all"] }] },
   mcp: { description: "Run the project-bound MCP stdio server for this session" },
   hook: { description: "Run a fail-open, context-only Claude Code hook", args: [{ name: "event", values: [...HOOK_EVENTS] }] },

src/memory/candidates.ts

@@ -10,6 +10,8 @@ import { stripPrivateDeep } from "./private-spans.ts";
 import { MemoryStore, type GoalView, type NoteInput, type View } from "./store.ts";
 
 export const CANDIDATE_DB_BUSY_TIMEOUT_MS = 3000;
+/** First line of every note written by accepting a consolidation candidate; `consolidationClusters` reads it to avoid proposing summaries of its own summaries. */
+export const ACCEPTED_CONSOLIDATION_PREFIX = "Accepted consolidation candidate: ";
 export const CANDIDATE_RETENTION = { count: 500, ageMs: 30 * 86400_000 } as const;
 export const COUNTER_RETENTION = { count: 10_000, ageMs: 90 * 86400_000 } as const;
 export type CandidateVerdict = "public" | "private" | "pii" | "unknown";
@@ -284,27 +286,29 @@ export class CandidateStore {
         if (context.actor !== "person" || context.store !== boundStore || context.project !== boundProject || context.policy !== boundPolicy) throw Error("consolidation acceptance context changed");
         if (context.policyVersion!() !== policyVersion) throw Error("consolidation policy changed");
       };
-      const check = () => {
+      // One placement proof per locked section: `sourcePlacements` (read once inside withCurrentEntries, whose fences stay held) serves every check before the append section;
+      // the append section reads one fresh proof. Each proof re-reads the legacy snapshot (up to 32 MiB), so four per acceptance was the cost this removes.
+      const check = (fresh = false) => {
         policyCurrent();
         const current = data.targets.map(target => {
           const row = context.store.get(target.id, context.project);
           if (!row || row.id !== target.id || row.project !== context.project || row.supersededBy || candidateTargetRevision(row) !== target.revision) throw Error("consolidation source revision is stale");
           return row;
         });
-        const placements = context.store.entryPlacements(data.targets.map(target => target.id), context.project);
+        const placements = fresh ? context.store.entryPlacements(data.targets.map(target => target.id), context.project) : sourcePlacements;
         if (current.some(row => row.scope !== current[0]!.scope || placements.get(row.id) !== placements.get(current[0]!.id))) throw Error("consolidation sources must have the same placement and scope");
         if (data.targets.some(target => placements.get(target.id) !== sourcePlacements.get(target.id))) throw Error("consolidation source placement changed");
         policyCurrent(); // The proof scan can be long; fence policy changes after it, not only before it.
         return current;
       };
       const sources = check(), first = sources[0]!;
       const note: NoteInput = { op: "note", project: context.project, scope: first.scope, tier: "long", kind: data.noteKind, peer: "person", source: "person", about: [...data.about],
-        text: `Accepted consolidation candidate: ${candidate.id}\nAction: ${data.action}\nSources: ${data.targets.map(target => `mem:${target.id}`).join(", ")}\n\n${data.text}` };
+        text: `${ACCEPTED_CONSOLIDATION_PREFIX}${candidate.id}\nAction: ${data.action}\nSources: ${data.targets.map(target => `mem:${target.id}`).join(", ")}\n\n${data.text}` };
       if (note.text.length > 4000) throw Error("accepted consolidation exceeds the v1 note limit");
       for (const source of sources) if (context.policy(structuredClone(candidate), structuredClone(source), structuredClone(note)) !== true) throw Error("consolidation refused by current host policy");
       check();
       return context.store.withAcceptedNote(note, append => {
-        check();
+        check(true);
         const replay = this.beginAcceptance(candidate.id, context.project, policyCurrent); if (replay) return { candidateId: candidate.id, noteId: replay, alreadyAccepted: true };
         const noteId = append(), applied: string[] = [];
         try {

src/memory/consolidate.ts

@@ -1,4 +1,4 @@
-import { CandidateStore, candidateTargetRevision, type CandidateInput } from "./candidates.ts";
+import { CandidateStore, candidateInputId, candidateTargetRevision, type CandidateInput } from "./candidates.ts";
 import { consolidationClusters } from "./consolidation-clusters.ts";
 import { memoryPolicySnapshot, type StandalonePolicy } from "./policy.ts";
 import { recallViewIsLive } from "./recall.ts";
@@ -15,6 +15,8 @@ export interface ConsolidationOptions {
 export interface ConsolidationReport {
   schema: "amem.consolidation/v1"; project: string; candidates: { id: string; action: string; sources: string[] }[];
   engine: "fts5" | "substring"; truncated: boolean; oversized: number; localExcluded: number;
+  /** Proposals whose candidate already exists as accepted, accepting or rejected: they are not queued again and are not in `candidates`. */
+  alreadyReviewed: number;
   model?: ModelConsolidationReport;
 }
 export class ConsolidationProposalError extends Error {
@@ -58,11 +60,18 @@ export async function proposeConsolidation(store: MemoryStore, candidates: Candi
     check(); if (await policy.screen(input)) prepared.push(input);
   }
   check();
-  const report: ConsolidationReport = { schema: "amem.consolidation/v1", project, candidates: [], engine: clustered.engine, truncated: raw.length === 257 || clustered.truncated, oversized, localExcluded: rows.slice(0, 256).length - synced.length };
+  const report: ConsolidationReport = { schema: "amem.consolidation/v1", project, candidates: [], engine: clustered.engine, truncated: raw.length === 257 || clustered.truncated, oversized, localExcluded: rows.slice(0, 256).length - synced.length, alreadyReviewed: 0 };
   try {
     // All asynchronous host work finishes before the first candidate write. The ordinary candidate
     // queue cap still applies; a full queue reports the ids already enqueued, never a silent batch.
-    for (const input of prepared) { check(); const id = candidates.put(input, "public"); report.candidates.push({ id, action: input.action, sources: input.targets.map(target => `mem:${target.id}`) }); }
+    // put() is INSERT OR IGNORE: a candidate the person already accepted or rejected keeps its status, so it is neither written again nor reported as queued.
+    for (const input of prepared) {
+      check();
+      const known = candidates.get(candidateInputId(input), project);
+      if (known && known.status !== "pending") { report.alreadyReviewed++; continue; }
+      const id = candidates.put(input, "public");
+      report.candidates.push({ id, action: input.action, sources: input.targets.map(target => `mem:${target.id}`) });
+    }
     if (options.model !== undefined) report.model = await proposeModelConsolidations(store, candidates, synced, options.model, { project, actor: options.actor, check, policy });
   } catch { throw new ConsolidationProposalError(report.candidates.map(row => row.id)); }
   return report;