peter/devclaw-gitea
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: add defensive safeguards and documentation for session persistence (#27)

Browse Source 
- Add defensive verification in deactivateWorker to catch any accidental
  session clearing bugs
- Enhance documentation in activateWorker, deactivateWorker, and
  updateWorker to clearly explain session preservation behavior
- Add comprehensive example flow in activateWorker docs showing session
  reuse across multiple tasks and tiers
- Add test suite for session persistence (projects.test.ts) to prevent
  regression
- Add npm test script to run test suite

This ensures sessions persist per tier after task completion, enabling
session reuse across multiple tasks of the same tier for massive token
savings (~50K per reuse) and context preservation.

Fixes: Bug where sessions could theoretically be accidentally cleared
during worker state updates, though current code was already correct.
This adds defense-in-depth to make the invariant bulletproof.
This commit is contained in:
Lauren ten Hoor
2026-02-10 10:52:12 +08:00
parent 38ad3fe27f
commit d1a02d988d
3 changed files with 254 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

189
lib/projects.test.ts Normal file
View File

@@ -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",
);
});
});

65
lib/projects.ts
View File

@@ -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<ProjectsData> {
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;
}
/**

3
package.json
View File

@@ -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"