diff --git a/lib/projects.test.ts b/lib/projects.test.ts new file mode 100644 index 0000000..b02a678 --- /dev/null +++ b/lib/projects.test.ts @@ -0,0 +1,189 @@ +/** + * Tests for projects.ts session persistence + * Run with: npm test + */ +import { describe, it, before, after } from "node:test"; +import assert from "node:assert"; +import fs from "node:fs/promises"; +import path from "node:path"; +import os from "node:os"; +import { + type ProjectsData, + activateWorker, + deactivateWorker, + readProjects, + writeProjects, +} from "./projects.js"; + +describe("Session persistence", () => { + let tempDir: string; + let testWorkspaceDir: string; + + before(async () => { + // Create temp directory for test workspace + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "devclaw-test-")); + testWorkspaceDir = tempDir; + await fs.mkdir(path.join(testWorkspaceDir, "memory"), { recursive: true }); + + // Create initial projects.json + const initialData: ProjectsData = { + projects: { + "-test-group": { + name: "test-project", + repo: "~/git/test-project", + groupName: "Test Project", + deployUrl: "https://test.example.com", + baseBranch: "main", + deployBranch: "main", + autoChain: false, + channel: "telegram", + dev: { + active: false, + issueId: null, + startTime: null, + model: null, + sessions: { + junior: null, + medior: null, + senior: null, + }, + }, + qa: { + active: false, + issueId: null, + startTime: null, + model: null, + sessions: { + qa: null, + }, + }, + }, + }, + }; + await writeProjects(testWorkspaceDir, initialData); + }); + + after(async () => { + // Clean up temp directory + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it("should preserve sessions after task completion (single tier)", async () => { + // Simulate task pickup: activate worker with senior tier + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "42", + model: "senior", + sessionKey: "agent:test:subagent:senior-session-123", + startTime: new Date().toISOString(), + }); + + // Verify session was stored + let data = await readProjects(testWorkspaceDir); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.senior, + "agent:test:subagent:senior-session-123", + "Senior session should be stored after activation", + ); + assert.strictEqual( + data.projects["-test-group"].dev.active, + true, + "Worker should be active", + ); + + // Simulate task completion: deactivate worker + await deactivateWorker(testWorkspaceDir, "-test-group", "dev"); + + // Verify session persists after deactivation + data = await readProjects(testWorkspaceDir); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.senior, + "agent:test:subagent:senior-session-123", + "Senior session should persist after deactivation", + ); + assert.strictEqual( + data.projects["-test-group"].dev.active, + false, + "Worker should be inactive", + ); + assert.strictEqual( + data.projects["-test-group"].dev.issueId, + null, + "Issue ID should be cleared", + ); + }); + + it("should preserve all tier sessions after completion (multiple tiers)", async () => { + // Setup: create sessions for multiple tiers + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "10", + model: "junior", + sessionKey: "agent:test:subagent:junior-session-111", + startTime: new Date().toISOString(), + }); + await deactivateWorker(testWorkspaceDir, "-test-group", "dev"); + + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "20", + model: "medior", + sessionKey: "agent:test:subagent:medior-session-222", + startTime: new Date().toISOString(), + }); + await deactivateWorker(testWorkspaceDir, "-test-group", "dev"); + + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "30", + model: "senior", + sessionKey: "agent:test:subagent:senior-session-333", + startTime: new Date().toISOString(), + }); + await deactivateWorker(testWorkspaceDir, "-test-group", "dev"); + + // Verify all sessions persisted + const data = await readProjects(testWorkspaceDir); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.junior, + "agent:test:subagent:junior-session-111", + "Junior session should persist", + ); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.medior, + "agent:test:subagent:medior-session-222", + "Medior session should persist", + ); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.senior, + "agent:test:subagent:senior-session-333", + "Senior session should persist", + ); + }); + + it("should reuse existing session when picking up new task", async () => { + // Setup: create a session for senior tier + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "100", + model: "senior", + sessionKey: "agent:test:subagent:senior-reuse-999", + startTime: new Date().toISOString(), + }); + await deactivateWorker(testWorkspaceDir, "-test-group", "dev"); + + // Pick up new task with same tier (no sessionKey = reuse) + await activateWorker(testWorkspaceDir, "-test-group", "dev", { + issueId: "200", + model: "senior", + }); + + // Verify session was preserved (not overwritten) + const data = await readProjects(testWorkspaceDir); + assert.strictEqual( + data.projects["-test-group"].dev.sessions.senior, + "agent:test:subagent:senior-reuse-999", + "Senior session should be reused (not cleared)", + ); + assert.strictEqual( + data.projects["-test-group"].dev.issueId, + "200", + "Issue ID should be updated to new task", + ); + }); +}); diff --git a/lib/projects.ts b/lib/projects.ts index dea6a62..5808e04 100644 --- a/lib/projects.ts +++ b/lib/projects.ts @@ -171,6 +171,11 @@ export function getWorker( /** * Update worker state for a project. Only provided fields are updated. * This prevents accidentally nulling out fields that should be preserved. + * + * Session Preservation: + * - If updates.sessions is provided, it's merged with existing sessions (new keys added/updated, existing keys preserved) + * - If updates.sessions is NOT provided, existing sessions are preserved via spread operator + * - Sessions should NEVER be accidentally cleared during state updates */ export async function updateWorker( workspaceDir: string, @@ -185,10 +190,14 @@ export async function updateWorker( } const worker = project[role]; + // Merge sessions maps if both exist + // This ensures we preserve existing sessions while adding/updating new ones if (updates.sessions && worker.sessions) { updates.sessions = { ...worker.sessions, ...updates.sessions }; } + + // Spread worker first, then updates - this preserves any fields not in updates project[role] = { ...worker, ...updates }; await writeProjects(workspaceDir, data); @@ -198,6 +207,21 @@ export async function updateWorker( /** * Mark a worker as active with a new task. * Sets active=true, issueId, model (tier). Stores session key in sessions[tier]. + * + * Session Handling: + * - If sessionKey is provided: new session spawned, stored in sessions[model] + * - If sessionKey is omitted: existing session reused (sessions map preserved) + * - Other tier sessions in the sessions map are ALWAYS preserved + * + * Example flow: + * 1. First senior task: activateWorker({model: "senior", sessionKey: "abc"}) + * → sessions = {junior: null, medior: null, senior: "abc"} + * 2. Task completes: deactivateWorker() + * → sessions = {junior: null, medior: null, senior: "abc"} (preserved!) + * 3. Next senior task: activateWorker({model: "senior"}) [no sessionKey] + * → sessions = {junior: null, medior: null, senior: "abc"} (reused!) + * 4. Medior task: activateWorker({model: "medior", sessionKey: "xyz"}) + * → sessions = {junior: null, medior: "xyz", senior: "abc"} (both preserved!) */ export async function activateWorker( workspaceDir: string, @@ -215,7 +239,8 @@ export async function activateWorker( issueId: params.issueId, model: params.model, }; - // Store session key in the sessions map for this tier + // Store session key in the sessions map for this tier (if new spawn) + // If sessionKey is omitted, existing sessions are preserved via updateWorker if (params.sessionKey !== undefined) { updates.sessions = { [params.model]: params.sessionKey }; } @@ -228,16 +253,52 @@ export async function activateWorker( /** * Mark a worker as inactive after task completion. * Clears issueId and active, PRESERVES sessions map, model, startTime for reuse. + * + * IMPORTANT: This function MUST preserve the sessions map to enable session reuse + * across multiple tasks of the same tier. Do NOT pass `sessions` in the updates + * object, as this would overwrite the existing sessions. */ export async function deactivateWorker( workspaceDir: string, groupId: string, role: "dev" | "qa", ): Promise { - return updateWorker(workspaceDir, groupId, role, { + // Read current state to verify sessions will be preserved + const data = await readProjects(workspaceDir); + const project = data.projects[groupId]; + if (!project) { + throw new Error(`Project not found for groupId: ${groupId}`); + } + + const worker = project[role]; + const sessionsBefore = worker.sessions; + + // Update worker state (active=false, issueId=null) + // Sessions are preserved via spread operator in updateWorker + const result = await updateWorker(workspaceDir, groupId, role, { active: false, issueId: null, + // Explicitly DO NOT set sessions here to preserve them }); + + // Defensive verification: ensure sessions were not accidentally cleared + const updatedWorker = result.projects[groupId][role]; + const sessionsAfter = updatedWorker.sessions; + + // Verify sessions map was preserved + if (sessionsBefore && sessionsAfter) { + for (const [tier, sessionKey] of Object.entries(sessionsBefore)) { + if (sessionKey !== null && sessionsAfter[tier] !== sessionKey) { + throw new Error( + `BUG: Session for tier "${tier}" was lost during deactivateWorker! ` + + `Before: ${sessionKey}, After: ${sessionsAfter[tier]}. ` + + `This should never happen - sessions must persist for reuse.` + ); + } + } + } + + return result; } /** diff --git a/package.json b/package.json index c8ec17e..add54a5 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,8 @@ }, "scripts": { "check": "tsc --noEmit", - "watch": "tsc --noEmit --watch" + "watch": "tsc --noEmit --watch", + "test": "node --test lib/**/*.test.ts" }, "peerDependencies": { "openclaw": ">=2026.0.0"