Skip to content

Commit 905e80b

Browse files
authored
Merge pull request #38 from sakamotopaya/story-13-session-persistence
these didn't make the 1st pr
2 parents c05e3c0 + 28c37ba commit 905e80b

File tree

4 files changed

+182
-62
lines changed

4 files changed

+182
-62
lines changed
Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
we are ready to work on issue #11 (docs/product-stories/cli-utility/story-11-browser-headless-mode.md) in repo https://github.com/sakamotopaya/code-agent.
2-
follow the normal git flow. create a new local branch for the story, code the tasks and unit tests that
3-
prove the task are complete.
1+
we are ready to work on issue #13 (docs/product-stories/cli-utility/story-13-session-persistence.md) in repo https://github.com/sakamotopaya/code-agent.
2+
follow the normal git flow. create a new local branch for the story, code the tasks and unit tests that
3+
prove the task are complete.
44
if you need information about prior stories, you can find them locally here docs/product-stories/cli-utility
5-
we oftern
6-
when you are finished with the code and tests, update the issue with a new comment describing your work and then
5+
we often get rejected trying to push our changes. make sure and run a build and lint prior to trying to push
6+
when you are finished with the code and tests, update the issue with a new comment describing your work and then
77
push your branch and create a pull request for this branch against main
88

99
We need to resume work on issue #7 (docs/product-stories/cli-utility/story-07-cli-configuration-management.md) in repo https://github.com/sakamotopaya/code-agent.
10-
review the documents and complete the story. when you are finished with the code and tests, update the issue with a new comment describing your work and then
11-
push your branch and create a pull request for this branch against main
10+
review the documents and complete the story. when you are finished with the code and tests, update the issue with a new comment describing your work and then
11+
push your branch and create a pull request for this branch against main

src/cli/services/SessionManager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,9 @@ export class SessionManager extends EventEmitter implements ISessionManager {
295295
this.activeSession.metadata.updatedAt = new Date()
296296

297297
// Trim history if it exceeds max length
298-
if (this.activeSession.history.messages.length > this.config.maxHistoryLength) {
298+
if (this.activeSession.history.messages.length > this.activeSession.config.maxHistoryLength) {
299299
this.activeSession.history.messages = this.activeSession.history.messages.slice(
300-
-this.config.maxHistoryLength,
300+
-this.activeSession.config.maxHistoryLength,
301301
)
302302
}
303303

src/cli/services/__tests__/SessionManager.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ describe("SessionManager", () => {
2929
mockStorage.getSessionSize = jest.fn()
3030
})
3131

32+
afterEach(() => {
33+
// Clean up any timers or async operations
34+
sessionManager.destroy()
35+
})
36+
3237
describe("initialization", () => {
3338
it("should initialize storage", async () => {
3439
await sessionManager.initialize()

src/cli/services/__tests__/SessionStorage.test.ts

Lines changed: 168 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,31 @@
11
import { SessionStorage } from "../SessionStorage"
2-
import { SessionStatus, SESSION_FORMAT_VERSION } from "../../types/session-types"
3-
import type { Session, SessionFile } from "../../types/session-types"
2+
import { SessionStatus, SESSION_FORMAT_VERSION, Session, SessionFile } from "../../types/session-types"
43
import * as fs from "fs/promises"
54
import * as path from "path"
65
import * as os from "os"
76

87
jest.mock("fs/promises")
9-
jest.mock("zlib")
8+
jest.mock("os")
9+
jest.mock("util", () => ({
10+
promisify: (fn: any) => fn, // Return the original function since our mocks already return promises
11+
}))
12+
jest.mock("zlib", () => ({
13+
gzip: jest.fn().mockImplementation((data: any) => Promise.resolve(Buffer.from(data))),
14+
gunzip: jest.fn().mockImplementation((data: any) => Promise.resolve(Buffer.from(data.toString()))),
15+
}))
1016

1117
const mockFs = fs as jest.Mocked<typeof fs>
1218

13-
// Mock crypto
14-
const mockCreateHash = jest.fn().mockReturnValue({
15-
update: jest.fn().mockReturnThis(),
16-
digest: jest.fn().mockReturnValue("mock-checksum"),
17-
})
19+
// Mock os module
20+
const mockOs = os as jest.Mocked<typeof os>
21+
jest.mocked(mockOs.homedir).mockReturnValue("/home/test")
1822

23+
// Mock crypto
1924
jest.mock("crypto", () => ({
20-
createHash: mockCreateHash,
25+
createHash: jest.fn().mockReturnValue({
26+
update: jest.fn().mockReturnThis(),
27+
digest: jest.fn().mockReturnValue("mock-checksum"),
28+
}),
2129
}))
2230

2331
describe("SessionStorage", () => {
@@ -27,6 +35,25 @@ describe("SessionStorage", () => {
2735

2836
beforeEach(() => {
2937
tempDir = "/tmp/test-sessions"
38+
39+
// Reset all mocks before each test
40+
jest.clearAllMocks()
41+
42+
// Setup default mock behaviors
43+
mockFs.mkdir.mockResolvedValue(undefined)
44+
mockFs.access.mockResolvedValue()
45+
mockFs.writeFile.mockResolvedValue()
46+
mockFs.readFile.mockResolvedValue(
47+
JSON.stringify({
48+
version: SESSION_FORMAT_VERSION,
49+
created: new Date().toISOString(),
50+
sessions: {},
51+
}),
52+
)
53+
mockFs.readdir.mockResolvedValue([])
54+
mockFs.stat.mockResolvedValue({ size: 1024 } as any)
55+
mockFs.unlink.mockResolvedValue()
56+
3057
storage = new SessionStorage({
3158
sessionDirectory: tempDir,
3259
compressionLevel: 0, // Disable compression for easier testing
@@ -87,8 +114,6 @@ describe("SessionStorage", () => {
87114
maxSessionSize: 100,
88115
},
89116
}
90-
91-
jest.clearAllMocks()
92117
})
93118

94119
describe("initialization", () => {
@@ -117,21 +142,13 @@ describe("SessionStorage", () => {
117142
describe("session operations", () => {
118143
describe("saveSession", () => {
119144
it("should save session to file", async () => {
120-
mockFs.mkdir.mockResolvedValue(undefined)
121-
mockFs.writeFile.mockResolvedValue()
122-
mockFs.readFile.mockResolvedValue("{}") // For metadata update
123-
124145
await storage.saveSession(mockSession)
125146

126147
const expectedPath = path.join(tempDir, `session-${mockSession.id}.json`)
127148
expect(mockFs.writeFile).toHaveBeenCalledWith(expectedPath, expect.any(String), { mode: 0o600 })
128149
})
129150

130151
it("should sanitize sensitive data before saving", async () => {
131-
mockFs.mkdir.mockResolvedValue(undefined)
132-
mockFs.writeFile.mockResolvedValue()
133-
mockFs.readFile.mockResolvedValue("{}")
134-
135152
const sessionWithSensitiveData = {
136153
...mockSession,
137154
config: {
@@ -159,8 +176,7 @@ describe("SessionStorage", () => {
159176
compressed: false,
160177
}
161178

162-
mockFs.readFile.mockResolvedValue(JSON.stringify(sessionFile))
163-
mockFs.writeFile.mockResolvedValue() // For updating last accessed time
179+
mockFs.readFile.mockResolvedValueOnce(JSON.stringify(sessionFile))
164180

165181
// Mock checksum validation
166182
const crypto = require("crypto")
@@ -169,7 +185,7 @@ describe("SessionStorage", () => {
169185
digest: jest.fn().mockReturnValue("test-checksum"),
170186
})
171187

172-
const result = await storage.loadSession(mockSession.id)
188+
const result = await storage.loadSession(mockSession.id, false) // Disable last accessed update
173189

174190
expect(result.id).toBe(mockSession.id)
175191
expect(result.name).toBe(mockSession.name)
@@ -184,7 +200,7 @@ describe("SessionStorage", () => {
184200
compressed: false,
185201
}
186202

187-
mockFs.readFile.mockResolvedValue(JSON.stringify(sessionFile))
203+
mockFs.readFile.mockResolvedValueOnce(JSON.stringify(sessionFile))
188204

189205
// Mock checksum validation to fail
190206
const crypto = require("crypto")
@@ -193,16 +209,12 @@ describe("SessionStorage", () => {
193209
digest: jest.fn().mockReturnValue("different-checksum"),
194210
})
195211

196-
await expect(storage.loadSession(mockSession.id)).rejects.toThrow("checksum validation failed")
212+
await expect(storage.loadSession(mockSession.id, false)).rejects.toThrow("checksum validation failed")
197213
})
198214
})
199215

200216
describe("deleteSession", () => {
201217
it("should delete session file", async () => {
202-
mockFs.unlink.mockResolvedValue()
203-
mockFs.readFile.mockResolvedValue("{}")
204-
mockFs.writeFile.mockResolvedValue()
205-
206218
await storage.deleteSession(mockSession.id)
207219

208220
const expectedPath = path.join(tempDir, `session-${mockSession.id}.json`)
@@ -213,40 +225,36 @@ describe("SessionStorage", () => {
213225
const error = new Error("File not found") as NodeJS.ErrnoException
214226
error.code = "ENOENT"
215227
mockFs.unlink.mockRejectedValue(error)
216-
mockFs.readFile.mockResolvedValue("{}")
217-
mockFs.writeFile.mockResolvedValue()
218228

219229
await expect(storage.deleteSession(mockSession.id)).resolves.not.toThrow()
220230
})
221231
})
222232

223233
describe("listSessions", () => {
224234
it("should list all session files", async () => {
225-
mockFs.mkdir.mockResolvedValue(undefined)
226235
mockFs.readdir.mockResolvedValue([
227236
"session-id1.json",
228237
"session-id2.json",
229238
"metadata.json",
230239
"other-file.txt",
231240
] as any)
232241

233-
// Mock loading session info
234-
mockFs.stat.mockResolvedValue({ size: 1024 } as any)
235-
const sessionFile: SessionFile = {
242+
const sessionFile1: SessionFile = {
236243
version: SESSION_FORMAT_VERSION,
237-
session: mockSession,
244+
session: { ...mockSession, id: "id1" },
245+
checksum: "test-checksum",
246+
compressed: false,
247+
}
248+
const sessionFile2: SessionFile = {
249+
version: SESSION_FORMAT_VERSION,
250+
session: { ...mockSession, id: "id2" },
238251
checksum: "test-checksum",
239252
compressed: false,
240253
}
241-
mockFs.readFile.mockResolvedValue(JSON.stringify(sessionFile))
242-
mockFs.writeFile.mockResolvedValue()
243254

244-
// Mock checksum validation
245-
const crypto = require("crypto")
246-
crypto.createHash = jest.fn().mockReturnValue({
247-
update: jest.fn(),
248-
digest: jest.fn().mockReturnValue("test-checksum"),
249-
})
255+
mockFs.readFile
256+
.mockResolvedValueOnce(JSON.stringify(sessionFile1)) // First session file
257+
.mockResolvedValueOnce(JSON.stringify(sessionFile2)) // Second session file
250258

251259
const sessions = await storage.listSessions()
252260

@@ -256,12 +264,11 @@ describe("SessionStorage", () => {
256264
})
257265

258266
it("should filter sessions by status", async () => {
259-
mockFs.mkdir.mockResolvedValue(undefined)
260267
mockFs.readdir.mockResolvedValue(["session-id1.json"] as any)
261-
mockFs.stat.mockResolvedValue({ size: 1024 } as any)
262268

263269
const activeSession = {
264270
...mockSession,
271+
id: "id1",
265272
metadata: { ...mockSession.metadata, status: SessionStatus.ACTIVE },
266273
}
267274
const sessionFile: SessionFile = {
@@ -270,14 +277,7 @@ describe("SessionStorage", () => {
270277
checksum: "test-checksum",
271278
compressed: false,
272279
}
273-
mockFs.readFile.mockResolvedValue(JSON.stringify(sessionFile))
274-
mockFs.writeFile.mockResolvedValue()
275-
276-
const crypto = require("crypto")
277-
crypto.createHash = jest.fn().mockReturnValue({
278-
update: jest.fn(),
279-
digest: jest.fn().mockReturnValue("test-checksum"),
280-
})
280+
mockFs.readFile.mockResolvedValueOnce(JSON.stringify(sessionFile))
281281

282282
const sessions = await storage.listSessions({
283283
status: SessionStatus.ACTIVE,
@@ -375,5 +375,120 @@ describe("SessionStorage", () => {
375375

376376
expect(isValid).toBe(false)
377377
})
378+
379+
describe("sanitizeSession", () => {
380+
it("should preserve Date objects when sanitizing sessions", async () => {
381+
// Create a session with Date objects
382+
const testDate = new Date("2024-01-01T12:00:00Z")
383+
const sessionWithDates: Session = {
384+
...mockSession,
385+
metadata: {
386+
...mockSession.metadata,
387+
createdAt: testDate,
388+
updatedAt: testDate,
389+
lastAccessedAt: testDate,
390+
},
391+
history: {
392+
...mockSession.history,
393+
messages: [
394+
{
395+
id: "msg-1",
396+
timestamp: testDate,
397+
role: "user",
398+
content: "Test message",
399+
},
400+
],
401+
},
402+
tools: [
403+
{
404+
toolName: "test-tool",
405+
configuration: {},
406+
cache: { someData: "test" },
407+
lastUsed: testDate,
408+
usageCount: 1,
409+
results: [
410+
{
411+
timestamp: testDate,
412+
input: "test",
413+
output: "result",
414+
success: true,
415+
},
416+
],
417+
},
418+
],
419+
files: {
420+
...mockSession.files,
421+
lastScanTime: testDate,
422+
},
423+
}
424+
425+
// Save the session
426+
await storage.saveSession(sessionWithDates)
427+
428+
// Find the session file write call (should contain the SessionFile structure)
429+
const sessionFileCall = mockFs.writeFile.mock.calls.find((call) => {
430+
try {
431+
const data = JSON.parse(call[1] as string)
432+
return data.version && data.session && data.checksum
433+
} catch {
434+
return false
435+
}
436+
})
437+
438+
expect(sessionFileCall).toBeDefined()
439+
const savedData = JSON.parse(sessionFileCall![1] as string)
440+
const sanitizedSession = savedData.session
441+
442+
// Check that Date objects are preserved as Date objects, not strings
443+
expect(sanitizedSession.metadata.createdAt).toEqual(testDate.toISOString())
444+
expect(sanitizedSession.metadata.updatedAt).toEqual(testDate.toISOString())
445+
expect(sanitizedSession.metadata.lastAccessedAt).toEqual(testDate.toISOString())
446+
expect(sanitizedSession.history.messages[0].timestamp).toEqual(testDate.toISOString())
447+
expect(sanitizedSession.tools[0].lastUsed).toEqual(testDate.toISOString())
448+
expect(sanitizedSession.tools[0].results[0].timestamp).toEqual(testDate.toISOString())
449+
expect(sanitizedSession.files.lastScanTime).toEqual(testDate.toISOString())
450+
451+
// Verify cache was cleared
452+
expect(sanitizedSession.tools[0].cache).toEqual({})
453+
})
454+
455+
it("should remove sensitive configuration data during sanitization", async () => {
456+
// Create a session with sensitive config data
457+
const sessionWithSensitiveData: Session = {
458+
...mockSession,
459+
config: {
460+
...mockSession.config,
461+
apiKey: "secret-api-key",
462+
encryptionKey: "secret-encryption-key",
463+
password: "secret-password",
464+
token: "secret-token",
465+
secret: "secret-value",
466+
} as any,
467+
}
468+
469+
// Save the session
470+
await storage.saveSession(sessionWithSensitiveData)
471+
472+
// Find the session file write call (should contain the SessionFile structure)
473+
const sessionFileCall = mockFs.writeFile.mock.calls.find((call) => {
474+
try {
475+
const data = JSON.parse(call[1] as string)
476+
return data.version && data.session && data.checksum
477+
} catch {
478+
return false
479+
}
480+
})
481+
482+
expect(sessionFileCall).toBeDefined()
483+
const savedData = JSON.parse(sessionFileCall![1] as string)
484+
const sanitizedSession = savedData.session
485+
486+
expect(sanitizedSession.config.apiKey).toBeUndefined()
487+
expect(sanitizedSession.config.encryptionKey).toBeUndefined()
488+
expect(sanitizedSession.config.password).toBeUndefined()
489+
expect(sanitizedSession.config.token).toBeUndefined()
490+
expect(sanitizedSession.config.secret).toBeUndefined()
491+
})
492+
})
378493
})
379494
})

0 commit comments

Comments
 (0)