Skip to content

fix(memory): make reconcileFacts safe under FORGE_LEDGER_ONLY (M2, partial)#105

Merged
CodeWithJuber merged 1 commit into
masterfrom
claude/loop-goal-worktree-issues-il3uk9
Jul 20, 2026
Merged

fix(memory): make reconcileFacts safe under FORGE_LEDGER_ONLY (M2, partial)#105
CodeWithJuber merged 1 commit into
masterfrom
claude/loop-goal-worktree-issues-il3uk9

Conversation

@CodeWithJuber

Copy link
Copy Markdown
Owner

What & why

Partial progress on M2 (finish the ledger-only memory migration) — specifically the
one destructive step, done safely and in isolation.

reconcileFacts (src/ledger_bridge.js) tombstones any author-owned fact claim that
has no backing file in the store. Under FORGE_LEDGER_ONLY there are no fact files, so it
would tombstone every fact — silently wiping the user's memory the first time
forge recall consolidate ran. It is now a guarded no-op when ledger-only is active: the
ledger IS the store then, so there is nothing to reconcile against. A new regression test
asserts two ledger-only facts survive a reconcile (no data loss).

This is the prerequisite the rest of M2 depends on — it makes FORGE_LEDGER_ONLY=1 safe
to opt into today.

Scope note — the default flip is intentionally NOT in this PR

Flipping the FORGE_LEDGER_ONLY default to on (and deleting the legacy stores) is
deliberately deferred: piloting the flip surfaced 31 failing tests, and crucially not
all are stale default-off assertions — some expose real ledger-only path gaps that
would need fixing first, e.g.:

  • brain.remember persists via a file but does not dual-write to the ledger, so under
    ledger-only the fact is lost (brain.buildIndex indexes 1 instead of 2).
  • the cortex learning loop's confirm-vs-create dedup reads localLessons, whose source
    changes under ledger-only — the candidate→active lifecycle needs re-verification.

Those are correctness-critical fixes across the memory subsystem and belong in their own
reviewed change, not rushed alongside a default flip. This PR lands only the safe,
self-contained destructive-heuristic fix.

Checklist

  • npm test passes (1070 pass / 0 fail / 2 gated skips — includes the new test)
  • npm run check passes (Biome — 0 errors)
  • New behavior has a test (no-data-loss regression)
  • Conventional commit message
  • CHANGELOG.md updated under ## [Unreleased]
  • No new runtime dependency

Risk & rollback

  • Risk level: low — narrows a destructive path to a no-op only under an off-by-default flag.
  • Rollback plan: revert the single commit.

Extra checks

  • npm run typecheck passes
  • node src/cli.js docs check green

🤖 Generated with Claude Code

https://claude.ai/code/session_01C7htTiesXKLvAUtv6ET2jz


Generated by Claude Code

…ge 1)

reconcileFacts tombstones any author-owned fact claim that has no backing file
in the store. Under ledger-only there are no fact files, so it would tombstone
EVERY fact -- silently wiping memory. It is now a guarded no-op when ledger-only
is active: the ledger is the store then, so there is nothing to reconcile against.

This is the destructive-heuristic fix that has to land BEFORE the default can be
flipped. Adds a no-data-loss regression test (two ledger-only facts survive a
reconcile). Full suite 1070 pass.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01C7htTiesXKLvAUtv6ET2jz
@CodeWithJuber
CodeWithJuber marked this pull request as ready for review July 20, 2026 15:47
@CodeWithJuber
CodeWithJuber merged commit 879e5ef into master Jul 20, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants