Merge 6f402778d2 into da71eaebd2
This commit is contained in:
commit
32586c5063
148
src/memory/manager-dirty-flag.test.ts
Normal file
148
src/memory/manager-dirty-flag.test.ts
Normal file
@ -0,0 +1,148 @@
|
|||||||
|
/**
|
||||||
|
* Test to verify the dirty flag false positive fix.
|
||||||
|
*
|
||||||
|
* This test verifies that:
|
||||||
|
* 1. After a sync completes, dirty is set to false
|
||||||
|
* 2. The dirty state is persisted to metadata
|
||||||
|
* 3. A new manager instance restores the dirty state from metadata
|
||||||
|
* 4. The dirty flag doesn't incorrectly reset to true on each new instance
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||||
|
import fs from "fs/promises";
|
||||||
|
import path from "path";
|
||||||
|
import os from "os";
|
||||||
|
import { MemoryIndexManager } from "./manager.js";
|
||||||
|
import type { OpenClawConfig } from "../config/config.js";
|
||||||
|
|
||||||
|
describe("MemoryIndexManager dirty flag persistence", () => {
|
||||||
|
let tempDir: string;
|
||||||
|
let dbPath: string;
|
||||||
|
let workspaceDir: string;
|
||||||
|
let cfg: OpenClawConfig;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
// Create temporary directories for testing
|
||||||
|
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "memory-test-"));
|
||||||
|
dbPath = path.join(tempDir, "memory.db");
|
||||||
|
workspaceDir = path.join(tempDir, "workspace");
|
||||||
|
await fs.mkdir(workspaceDir, { recursive: true });
|
||||||
|
|
||||||
|
// Create a minimal config
|
||||||
|
cfg = {
|
||||||
|
agents: {
|
||||||
|
defaults: {
|
||||||
|
memory: {
|
||||||
|
search: {
|
||||||
|
enabled: true,
|
||||||
|
provider: "local",
|
||||||
|
sources: ["memory"],
|
||||||
|
store: {
|
||||||
|
path: dbPath,
|
||||||
|
vector: {
|
||||||
|
enabled: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
cache: {
|
||||||
|
enabled: false,
|
||||||
|
},
|
||||||
|
query: {
|
||||||
|
hybrid: {
|
||||||
|
enabled: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
sync: {
|
||||||
|
onSearch: false,
|
||||||
|
onSessionStart: false,
|
||||||
|
},
|
||||||
|
extraPaths: [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as unknown as OpenClawConfig;
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
// Clean up temp directory
|
||||||
|
await fs.rm(tempDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should persist dirty=false after sync and restore it on new instance", async () => {
|
||||||
|
// Create first manager instance
|
||||||
|
const manager1 = await MemoryIndexManager.get({
|
||||||
|
cfg,
|
||||||
|
agentId: "test-agent",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(manager1).not.toBeNull();
|
||||||
|
if (!manager1) return;
|
||||||
|
|
||||||
|
// Initial state should be dirty=true (memory source present)
|
||||||
|
let status = manager1.status();
|
||||||
|
expect(status.dirty).toBe(true);
|
||||||
|
|
||||||
|
// Run sync
|
||||||
|
await manager1.sync({ force: true });
|
||||||
|
|
||||||
|
// After sync, dirty should be false
|
||||||
|
status = manager1.status();
|
||||||
|
expect(status.dirty).toBe(false);
|
||||||
|
|
||||||
|
// Close the manager (removes from cache)
|
||||||
|
await manager1.close();
|
||||||
|
|
||||||
|
// Create a new manager instance (simulates next command)
|
||||||
|
const manager2 = await MemoryIndexManager.get({
|
||||||
|
cfg,
|
||||||
|
agentId: "test-agent",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(manager2).not.toBeNull();
|
||||||
|
if (!manager2) return;
|
||||||
|
|
||||||
|
// The new instance should restore dirty=false from metadata
|
||||||
|
status = manager2.status();
|
||||||
|
expect(status.dirty).toBe(false);
|
||||||
|
|
||||||
|
await manager2.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should set dirty=true when files change and persist it", async () => {
|
||||||
|
// Create manager and sync
|
||||||
|
const manager1 = await MemoryIndexManager.get({
|
||||||
|
cfg,
|
||||||
|
agentId: "test-agent",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(manager1).not.toBeNull();
|
||||||
|
if (!manager1) return;
|
||||||
|
|
||||||
|
await manager1.sync({ force: true });
|
||||||
|
expect(manager1.status().dirty).toBe(false);
|
||||||
|
|
||||||
|
// Simulate file change by manually setting dirty
|
||||||
|
// (In real usage, the watcher would do this)
|
||||||
|
await manager1.close();
|
||||||
|
|
||||||
|
// Create a test file to trigger watcher
|
||||||
|
const memoryDir = path.join(workspaceDir, "memory");
|
||||||
|
await fs.mkdir(memoryDir, { recursive: true });
|
||||||
|
await fs.writeFile(path.join(memoryDir, "test.md"), "# Test");
|
||||||
|
|
||||||
|
// Create new manager - it should detect the file
|
||||||
|
const manager2 = await MemoryIndexManager.get({
|
||||||
|
cfg,
|
||||||
|
agentId: "test-agent",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(manager2).not.toBeNull();
|
||||||
|
if (!manager2) return;
|
||||||
|
|
||||||
|
// After detecting new file, dirty should be true
|
||||||
|
// Note: This might need a small delay for watcher to trigger
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||||
|
|
||||||
|
await manager2.close();
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -65,6 +65,7 @@ type MemoryIndexMeta = {
|
|||||||
chunkTokens: number;
|
chunkTokens: number;
|
||||||
chunkOverlap: number;
|
chunkOverlap: number;
|
||||||
vectorDims?: number;
|
vectorDims?: number;
|
||||||
|
dirty?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
type SessionFileEntry = {
|
type SessionFileEntry = {
|
||||||
@ -245,7 +246,7 @@ export class MemoryIndexManager {
|
|||||||
this.ensureWatcher();
|
this.ensureWatcher();
|
||||||
this.ensureSessionListener();
|
this.ensureSessionListener();
|
||||||
this.ensureIntervalSync();
|
this.ensureIntervalSync();
|
||||||
this.dirty = this.sources.has("memory");
|
this.dirty = meta?.dirty ?? this.sources.has("memory");
|
||||||
this.batch = this.resolveBatchConfig();
|
this.batch = this.resolveBatchConfig();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -836,6 +837,7 @@ export class MemoryIndexManager {
|
|||||||
});
|
});
|
||||||
const markDirty = () => {
|
const markDirty = () => {
|
||||||
this.dirty = true;
|
this.dirty = true;
|
||||||
|
this.persistDirtyState();
|
||||||
this.scheduleWatchSync();
|
this.scheduleWatchSync();
|
||||||
};
|
};
|
||||||
this.watcher.on("add", markDirty);
|
this.watcher.on("add", markDirty);
|
||||||
@ -1261,6 +1263,7 @@ export class MemoryIndexManager {
|
|||||||
if (shouldSyncMemory) {
|
if (shouldSyncMemory) {
|
||||||
await this.syncMemoryFiles({ needsFullReindex, progress: progress ?? undefined });
|
await this.syncMemoryFiles({ needsFullReindex, progress: progress ?? undefined });
|
||||||
this.dirty = false;
|
this.dirty = false;
|
||||||
|
this.persistDirtyState();
|
||||||
}
|
}
|
||||||
|
|
||||||
if (shouldSyncSessions) {
|
if (shouldSyncSessions) {
|
||||||
@ -1486,6 +1489,14 @@ export class MemoryIndexManager {
|
|||||||
.run(META_KEY, value);
|
.run(META_KEY, value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private persistDirtyState() {
|
||||||
|
const meta = this.readMeta();
|
||||||
|
if (meta) {
|
||||||
|
meta.dirty = this.dirty;
|
||||||
|
this.writeMeta(meta);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private async listSessionFiles(): Promise<string[]> {
|
private async listSessionFiles(): Promise<string[]> {
|
||||||
const dir = resolveSessionTranscriptsDirForAgent(this.agentId);
|
const dir = resolveSessionTranscriptsDirForAgent(this.agentId);
|
||||||
try {
|
try {
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user