feat: add sync command to bridge extracted memories into MEMORY.md - #3
feat: add sync command to bridge extracted memories into MEMORY.md#3anupamchugh wants to merge 1 commit into
Conversation
Closes the feedback loop between post-session extraction and in-session memory. Extracted insights from ~/.claude/memories/extracted/ are filtered by confidence, deduplicated, and synced into Claude Code's MEMORY.md files — respecting the 200-line limit. - New `claude-memory sync` CLI command with dry-run, stats, and list-projects subcommands - ID-based + fuzzy title deduplication prevents double-syncing - Backup MEMORY.md before modification - 24 passing tests covering parser, dedup, and sync engine Co-Authored-By: Claude Opus 4.6 <[email protected]>
📝 WalkthroughWalkthroughThis PR introduces a complete memory synchronization system, integrating a CLI sync command that orchestrates the extraction, deduplication, and merging of memory entries from markdown files into MEMORY.md files across Claude projects. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant SyncCommand
participant MemoryParser
participant Deduplicator
participant SyncEngine
participant MEMORY.md
User->>CLI: Run sync command
CLI->>SyncCommand: new SyncCommand(options)
SyncCommand->>SyncCommand: execute()
alt listProjects mode
SyncCommand->>MemoryParser: loadExtractedMemories()
MemoryParser-->>SyncCommand: ExtractedMemoryFile[]
SyncCommand-->>User: Display project list
else showStats mode
SyncCommand->>MemoryParser: loadExtractedMemories()
MemoryParser-->>SyncCommand: ExtractedMemoryFile[]
SyncCommand-->>User: Display statistics
else runSync mode
SyncCommand->>MemoryParser: loadExtractedMemories()
MemoryParser-->>SyncCommand: ExtractedMemoryFile[]
SyncCommand->>SyncEngine: syncMemories(options)
SyncEngine->>SyncEngine: findProjectMemoryPaths()
loop For each target MEMORY.md
SyncEngine->>Deduplicator: loadSyncState()
Deduplicator-->>SyncEngine: SyncState
SyncEngine->>Deduplicator: isDuplicate() for each memory
Deduplicator-->>SyncEngine: boolean
SyncEngine->>MEMORY.md: Append new section (or dry-run)
SyncEngine->>Deduplicator: saveSyncState()
end
SyncEngine-->>SyncCommand: SyncResult[]
SyncCommand-->>User: Display sync results
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 106-108: The parsed numeric CLI options minConfidence and maxLines
can become NaN (via parseFloat/parseInt) which silently disables guards; update
the parsing logic in src/cli.ts where minConfidence and maxLines are set to
validate the parsed values (use Number.isFinite or isNaN checks) and either (a)
throw a user-friendly error via the CLI parser or process.exit when input is
invalid, or (b) replace invalid values with safe defaults and log a warning;
ensure you reference the parsed results of parseFloat(options.minConfidence) and
parseInt(options.maxLines, 10) and apply the check before using them in any
confidence or line-limit comparisons.
In `@src/sync/deduplicator.ts`:
- Around line 22-31: loadSyncState currently returns parsed JSON without
validating shape, so a valid JSON file missing syncedIds (e.g., {}) leads to
syncState.syncedIds being undefined and later throwing when spread; update
loadSyncState to validate and normalize the parsed object: after JSON.parse
check that the result is an object and that result.syncedIds is an Array (use
Array.isArray) and result.lastSync is a string, otherwise return the safe
default { syncedIds: [], lastSync: '' } (or coerce fields to those types), and
reference the loadSyncState function and the syncState.syncedIds usage so
callers like the spread in sync-engine.ts operate on a guaranteed array.
In `@src/sync/memory-parser.ts`:
- Line 45: The filter `l => l !== '' || l === ''` is a tautology and does
nothing; update the usage in formatMemoryEntry (in src/sync/sync-engine.ts) and
the similar code in src/sync/memory-parser.ts to either remove the redundant
filter and call .join('\n') directly, or change the predicate to `l => l !== ''`
if the intent is to strip blank lines; locate the join usage in the
formatMemoryEntry function and replace the filter accordingly so blank-line
behavior matches the intended output.
- Line 83: The check in parseMemoryFile currently uses a falsy test (if (!fm.id
|| !fm.confidence) return null;) which drops valid memories with confidence ===
0; change the condition to explicitly reject missing values only (e.g., check
fm.id == null or fm.confidence == null/undefined) or otherwise document intent —
update the condition that references fm.id and fm.confidence to use strict
null/undefined checks (or add a comment if zero confidence should be treated as
invalid) so zero is preserved or explicitly rejected as intended.
In `@src/sync/sync-engine.ts`:
- Around line 44-46: The filter `l => l !== '' || l === ''` is tautological and
does nothing; remove the entire .filter(...) so the code simply returns
lines.join('\n') if you intend to preserve blank lines as spacers (or replace
the filter with `.filter(Boolean)` if you want to remove blank lines). If you
choose to keep the trailing blank line behavior used in the function that
constructs `lines`, also update the line-count logic in `syncToTarget` (the
`countLines` usage) to account for the extra trailing blank per entry so the
200-line budget is computed correctly.
🧹 Nitpick comments (5)
src/sync/memory-parser.ts (1)
35-39: Dead condition:trimmed.startsWith(' ')is always false aftertrim().
trimmedis the result ofline.trim(), so it can never start with a space. This is harmless but misleading.♻️ Suggested fix
- if (!trimmed || trimmed.startsWith('#') || trimmed.startsWith('-') || trimmed.startsWith(' ')) { + if (!trimmed || trimmed.startsWith('#') || trimmed.startsWith('-')) {src/sync/sync-engine.ts (3)
56-58: Prefer top-level import over inlinerequire('os').
osis already imported at the top level in other files (e.g.,src/cli.ts). Usingrequire('os')inline is inconsistent and prevents tree-shaking / static analysis.♻️ Suggested fix
Add to the existing imports at the top of the file:
import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os';Then update the usage:
- const projectsDir = path.join(require('os').homedir(), '.claude', 'projects'); + const projectsDir = path.join(os.homedir(), '.claude', 'projects');
109-112:statePathderivation assumes the path ends withMEMORY.md.
memoryMdPath.replace(/MEMORY\.md$/, '.memory-sync-state.json')silently falls through if the path doesn't match the regex, resulting in the state file being written to the same path as the MEMORY.md (clobbering it). This only works correctly because all current callers guarantee the filename. A defensive guard would prevent silent data corruption if that assumption is ever violated.🛡️ Suggested defensive check
const statePath = memoryMdPath.replace(/MEMORY\.md$/, '.memory-sync-state.json'); + if (statePath === memoryMdPath) { + throw new Error(`Expected memoryMdPath to end with MEMORY.md, got: ${memoryMdPath}`); + }
128-144: Line budget slightly underestimates due to inter-entry separators.When multiple entries are joined via
entries.join('\n')(line 157), each join inserts a\nbetween entries. Since each formatted entry already ends with a trailing newline, this produces a blank line between entries — adding approximately(N-1)lines not captured inaddedLines. Given the 10-line buffer (190 of 200), this is unlikely to cause a real overshoot, but worth being aware of if the buffer is tightened.tests/unit/sync.test.ts (1)
150-154: Date filter test implicitly relies on lexicographic string comparison.The test at line 151 passes
since: '2026-01-16'(a date-only string) whilemem.createdvalues are full ISO timestamps like'2026-01-16T10:00:00Z'. This works because of JS lexicographic comparison, but it's a subtle detail. Consider adding a brief comment in the test or inloadExtractedMemoriesnoting that the comparison relies on ISO 8601's lexicographic ordering property.
| minConfidence: parseFloat(options.minConfidence), | ||
| maxLines: parseInt(options.maxLines, 10), | ||
| since: options.since, |
There was a problem hiding this comment.
NaN from invalid numeric CLI input silently disables confidence and line-limit guards.
parseFloat('abc') and parseInt('abc', 10) both return NaN. Since NaN comparisons always return false, passing a non-numeric --min-confidence would skip all confidence filtering, and a non-numeric --max-lines would skip line-limit enforcement.
Consider adding a guard after parsing:
🛡️ Proposed fix
- const command = new SyncCommand({
- extractedDir: options.extractedDir,
- memoryMd: options.memoryMd,
- minConfidence: parseFloat(options.minConfidence),
- maxLines: parseInt(options.maxLines, 10),
+ const minConfidence = parseFloat(options.minConfidence);
+ const maxLines = parseInt(options.maxLines, 10);
+ if (isNaN(minConfidence) || isNaN(maxLines)) {
+ console.error('Error: --min-confidence and --max-lines must be valid numbers.');
+ process.exit(1);
+ }
+
+ const command = new SyncCommand({
+ extractedDir: options.extractedDir,
+ memoryMd: options.memoryMd,
+ minConfidence,
+ maxLines,🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 106 - 108, The parsed numeric CLI options
minConfidence and maxLines can become NaN (via parseFloat/parseInt) which
silently disables guards; update the parsing logic in src/cli.ts where
minConfidence and maxLines are set to validate the parsed values (use
Number.isFinite or isNaN checks) and either (a) throw a user-friendly error via
the CLI parser or process.exit when input is invalid, or (b) replace invalid
values with safe defaults and log a warning; ensure you reference the parsed
results of parseFloat(options.minConfidence) and parseInt(options.maxLines, 10)
and apply the check before using them in any confidence or line-limit
comparisons.
| export function loadSyncState(statePath: string): SyncState { | ||
| if (!fs.existsSync(statePath)) { | ||
| return { syncedIds: [], lastSync: '' }; | ||
| } | ||
| try { | ||
| return JSON.parse(fs.readFileSync(statePath, 'utf-8')); | ||
| } catch { | ||
| return { syncedIds: [], lastSync: '' }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Corrupted state file with valid JSON but wrong shape can crash the sync.
loadSyncState handles parse errors but not schema mismatches. If the file contains valid JSON without a syncedIds array (e.g., {}), then syncState.syncedIds is undefined, and the spread in sync-engine.ts line 180 ([...syncState.syncedIds, ...]) will throw TypeError: not iterable.
🛡️ Proposed fix — add minimal shape validation
export function loadSyncState(statePath: string): SyncState {
if (!fs.existsSync(statePath)) {
return { syncedIds: [], lastSync: '' };
}
try {
- return JSON.parse(fs.readFileSync(statePath, 'utf-8'));
+ const parsed = JSON.parse(fs.readFileSync(statePath, 'utf-8'));
+ return {
+ syncedIds: Array.isArray(parsed.syncedIds) ? parsed.syncedIds : [],
+ lastSync: typeof parsed.lastSync === 'string' ? parsed.lastSync : '',
+ };
} catch {
return { syncedIds: [], lastSync: '' };
}
}🤖 Prompt for AI Agents
In `@src/sync/deduplicator.ts` around lines 22 - 31, loadSyncState currently
returns parsed JSON without validating shape, so a valid JSON file missing
syncedIds (e.g., {}) leads to syncState.syncedIds being undefined and later
throwing when spread; update loadSyncState to validate and normalize the parsed
object: after JSON.parse check that the result is an object and that
result.syncedIds is an Array (use Array.isArray) and result.lastSync is a
string, otherwise return the safe default { syncedIds: [], lastSync: '' } (or
coerce fields to those types), and reference the loadSyncState function and the
syncState.syncedIds usage so callers like the spread in sync-engine.ts operate
on a guaranteed array.
| if (colonIdx === -1) continue; | ||
|
|
||
| const key = trimmed.slice(0, colonIdx).trim(); | ||
| const rawVal = trimmed.slice(colonIdx + 1).trim(); |
There was a problem hiding this comment.
The filter in lines.filter(l => l !== '' || l === '') is a no-op — it always returns true.
Looking at how formatMemoryEntry in src/sync/sync-engine.ts (line 45) uses a similar pattern — lines.filter(l => l !== '' || l === '').join('\n') — this condition is tautological: every string is either not-empty or empty. All lines pass through unchanged. If the intent was to strip blank lines, the filter should be l => l !== ''. If the intent was to keep everything (including blank lines as spacers), just use .join('\n') directly.
I raise this here because the formatMemoryEntry in sync-engine.ts (line 45) is the actual usage, but the root issue is the logical expression itself.
🤖 Prompt for AI Agents
In `@src/sync/memory-parser.ts` at line 45, The filter `l => l !== '' || l === ''`
is a tautology and does nothing; update the usage in formatMemoryEntry (in
src/sync/sync-engine.ts) and the similar code in src/sync/memory-parser.ts to
either remove the redundant filter and call .join('\n') directly, or change the
predicate to `l => l !== ''` if the intent is to strip blank lines; locate the
join usage in the formatMemoryEntry function and replace the filter accordingly
so blank-line behavior matches the intended output.
| } | ||
|
|
||
| const fm = parseFrontmatter(text); | ||
| if (!fm.id || !fm.confidence) return null; |
There was a problem hiding this comment.
confidence: 0 is falsy — memories with zero confidence are silently dropped.
!fm.confidence is true when confidence is 0, so parseMemoryFile returns null for a valid-but-zero confidence entry. If this is intentional (zero confidence is meaningless), consider making it explicit with a comment or using a stricter check.
♻️ Suggested clarification
- if (!fm.id || !fm.confidence) return null;
+ if (!fm.id || fm.confidence === undefined || fm.confidence === null) return null;Or, if zero confidence should indeed be rejected:
- if (!fm.id || !fm.confidence) return null;
+ // Note: confidence of 0 is intentionally rejected as meaningless
+ if (!fm.id || !fm.confidence) return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!fm.id || !fm.confidence) return null; | |
| if (!fm.id || fm.confidence === undefined || fm.confidence === null) return null; |
🤖 Prompt for AI Agents
In `@src/sync/memory-parser.ts` at line 83, The check in parseMemoryFile currently
uses a falsy test (if (!fm.id || !fm.confidence) return null;) which drops valid
memories with confidence === 0; change the condition to explicitly reject
missing values only (e.g., check fm.id == null or fm.confidence ==
null/undefined) or otherwise document intent — update the condition that
references fm.id and fm.confidence to use strict null/undefined checks (or add a
comment if zero confidence should be treated as invalid) so zero is preserved or
explicitly rejected as intended.
|
|
||
| return lines.filter(l => l !== '' || l === '').join('\n'); | ||
| } |
There was a problem hiding this comment.
Tautological filter — l !== '' || l === '' is always true.
Every string is either not-empty or empty, so this filter passes all elements unchanged. If the intent is to keep blank lines as spacers (the trailing '' on line 42), just drop the filter:
♻️ Suggested fix
- return lines.filter(l => l !== '' || l === '').join('\n');
+ return lines.filter(Boolean).join('\n');Or if the trailing blank line is intentional (to separate entries):
- return lines.filter(l => l !== '' || l === '').join('\n');
+ return lines.join('\n');Note: If the trailing blank line is intentional, the line-count budget in syncToTarget (line 139) undercounts by one line per entry, since countLines trims before counting — but the blank line is present in the appended text. With a 10-line buffer (190 of 200), this is unlikely to matter in practice.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return lines.filter(l => l !== '' || l === '').join('\n'); | |
| } | |
| return lines.filter(Boolean).join('\n'); | |
| } |
| return lines.filter(l => l !== '' || l === '').join('\n'); | |
| } | |
| return lines.join('\n'); | |
| } |
🤖 Prompt for AI Agents
In `@src/sync/sync-engine.ts` around lines 44 - 46, The filter `l => l !== '' || l
=== ''` is tautological and does nothing; remove the entire .filter(...) so the
code simply returns lines.join('\n') if you intend to preserve blank lines as
spacers (or replace the filter with `.filter(Boolean)` if you want to remove
blank lines). If you choose to keep the trailing blank line behavior used in the
function that constructs `lines`, also update the line-count logic in
`syncToTarget` (the `countLines` usage) to account for the extra trailing blank
per entry so the 200-line budget is computed correctly.
1 participant
Summary
Closes the feedback loop between
claude-memory extractand Claude Code's built-in MEMORY.md.Today the pipeline is:
This PR adds
claude-memory syncto bridge that gap:What it does
~/.claude/memories/extracted/New files
src/commands/sync.tssrc/sync/memory-parser.tssrc/sync/deduplicator.tssrc/sync/sync-engine.tstests/unit/sync.test.tsUsage
Hook integration
Design decisions
.memory-sync-state.jsonalongside MEMORY.md. Fuzzy title matching is a fallback for memories synced before tracking was added.MEMORY.md.bakbefore any modification. Cheap insurance.fs,path, andosfrom Node stdlib.parser.test.ts.Test plan
npx jest tests/unit/sync.test.ts)memory-writer,vector-store,llm-clientare upstream issues)npx tsc)Context
I built a Python prototype of this concept first, then ported to TypeScript to fit the existing codebase. The Python version works as a standalone zero-dependency alternative.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features