← Back to PRs

#4386: fix(memory): persist dirty flag to prevent false positive on status

by Iamadig open 2026-01-30 05:10 View on GitHub →
## Problem The memory index `dirty` flag was showing a false positive after a clean sync. When running `clawdbot memory status`, it would always report `Dirty: yes` even after a successful `clawdbot memory sync`, causing confusion about whether the index actually needed reindexing. ### Root Cause Each `clawdbot memory` command creates a fresh `MemoryIndexManager` instance. The issue occurred because: 1. Constructor unconditionally set `this.dirty = this.sources.has("memory")` (line 248) 2. `sync()` would set `this.dirty = false` after successful indexing 3. `close()` removes the manager from cache 4. Next command creates a new instance -> dirty resets to `true` again The actual indexing logic was working correctly (using file hashes to skip unchanged files), but the dirty flag was purely a UX issue showing incorrect state. ## Solution Persist the `dirty` flag to the database metadata so it survives across manager instances: 1. **Extended `MemoryIndexMeta` type** to include optional `dirty?: boolean` field 2. **Modified constructor** to restore dirty state from metadata: `this.dirty = meta?.dirty ?? this.sources.has("memory")` 3. **Added `persistDirtyState()` helper** that updates the dirty flag in metadata 4. **Persist after sync** completes (when setting `dirty = false`) 5. **Persist when watcher detects changes** (when setting `dirty = true`) ## Testing Added comprehensive test suite in `manager-dirty-flag.test.ts` that verifies: - After sync, dirty is set to false and persisted - New manager instances restore the dirty state from metadata - The flag doesn't incorrectly reset to true on each new instance ## Impact - **User-facing:** Status now accurately reflects whether the index needs reindexing - **Backward compatible:** Existing indexes without the dirty field default to `true` (safe fallback) - **No data changes:** Only affects the status reporting, not the actual indexing logic <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR aims to fix a UX issue where `clawdbot memory status` would always show `Dirty: yes` after a clean sync by persisting the `dirty` flag into the memory index metadata and restoring it in new `MemoryIndexManager` instances. It updates `src/memory/manager.ts` to (a) add an optional `dirty` field to `MemoryIndexMeta`, (b) initialize `this.dirty` from stored meta when present, and (c) persist dirty state changes after a sync and on file watcher events. It also adds a new Vitest file `src/memory/manager-dirty-flag.test.ts` intended to verify persistence across manager instances. Main concern: the persistence logic currently only runs on the incremental sync path; the full reindex path (`runSafeReindex`) writes fresh meta without `dirty`, which can reintroduce the original false-positive after any sync that triggers a full reindex (first run, config/model/provider change, or force). Separately, the added watcher-related test doesn’t appear to actually wire the temp `workspaceDir` into the manager’s resolved workspace, so it may not be exercising the behavior it intends to validate. <h3>Confidence Score: 3/5</h3> - This PR is likely safe, but has a correctness gap around the full-reindex path and a test that may not exercise intended behavior. - The changes are localized and conceptually straightforward, but `dirty` persistence appears incomplete when `runSafeReindex()` is used, which can bring back the reported UX problem in common scenarios (first index/config changes/force). The added tests also seem misconfigured around workspace resolution, reducing confidence that the fix is fully covered. - src/memory/manager.ts (runSafeReindex meta write), src/memory/manager-dirty-flag.test.ts (workspace wiring / assertions) <!-- greptile_other_comments_section --> <sub>(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs