diff --git a/package-lock.json b/package-lock.json index e7d3738..833eb96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7769,7 +7769,8 @@ "@types/node": "^25.5.0", "esbuild": "^0.27.0", "tsx": "^4.21.0", - "typescript": "^5.9.3" + "typescript": "^5.9.3", + "vitest": "^4.1.0" }, "engines": { "node": ">=22.13" diff --git a/package.json b/package.json index c222123..9a06edc 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "scripts": { "build": "tsx scripts/build.ts", "build:skills": "tsx scripts/build-skills.ts", - "test": "npm run test -w @diffity/git && npm run test -w @diffity/github && npm run test -w @diffity/parser && npm run test -w @diffity/ui && npm run test:scripts", + "test": "npm run test -w @diffity/git && npm run test -w @diffity/github && npm run test -w @diffity/parser && npm run test -w @diffity/ui && npm run test -w diffity && npm run test:scripts", "link-dev": "tsx scripts/link-dev.ts", "dev": "tsx scripts/dev.ts", "release:patch": "npm run build && tsx scripts/release.ts patch && npm publish -w packages/cli", diff --git a/packages/cli/package.json b/packages/cli/package.json index 3590a6b..db29f2d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -9,7 +9,9 @@ "scripts": { "build": "tsx build.ts", "dev": "tsx src/index.ts", - "dev:watch": "tsx build.ts --watch" + "dev:watch": "tsx build.ts --watch", + "test": "vitest run", + "test:watch": "vitest" }, "dependencies": { "commander": "^14.0.3", @@ -45,6 +47,7 @@ "@types/node": "^25.5.0", "esbuild": "^0.27.0", "tsx": "^4.21.0", - "typescript": "^5.9.3" + "typescript": "^5.9.3", + "vitest": "^4.1.0" } } diff --git a/packages/cli/src/session.ts b/packages/cli/src/session.ts index a6bfd03..fa05100 100644 --- a/packages/cli/src/session.ts +++ b/packages/cli/src/session.ts @@ -30,16 +30,49 @@ export function findOrCreateSession(ref: string): Session { return session; } + // A session is identified by the commit as well as the ref, so committing creates a new + // one. Anything still open has to come with it: the whole point of reviewing your own + // change is to act on the findings, and acting on them moves HEAD. + const previous = queryOne<{ id: string }>( + 'SELECT id FROM review_sessions WHERE ref = ? ORDER BY created_at DESC, rowid DESC LIMIT 1', + ref, + ); + const id = randomUUID(); db.prepare( 'INSERT INTO review_sessions (id, ref, head_hash) VALUES (?, ?, ?)' ).run(id, ref, headHash); + if (previous) { + carryForward(previous.id, id); + } + const session: Session = { id, ref, headHash }; writeFileSync(sessionFilePath(), JSON.stringify(session)); return session; } +/** + * Moves rather than copies, so thread ids stay stable and nothing is duplicated. Threads that + * were resolved or dismissed stay behind: they belong to the commit where they were dealt with, + * and reopening them on every later commit would be noise. + * + * Line numbers are not re-anchored yet, so a thread whose code moved still points at the line + * it was written against. + */ +export function carryForward(fromSessionId: string, toSessionId: string): void { + const db = getDb(); + + db.prepare( + "UPDATE comment_threads SET session_id = ? WHERE session_id = ? AND status = 'open'", + ).run(toSessionId, fromSessionId); + + db.prepare('UPDATE tours SET session_id = ? WHERE session_id = ?').run( + toSessionId, + fromSessionId, + ); +} + export function getCurrentSession(): Session | null { try { const raw = readFileSync(sessionFilePath(), 'utf-8'); diff --git a/packages/cli/tests/session-continuity.test.ts b/packages/cli/tests/session-continuity.test.ts new file mode 100644 index 0000000..30f09c3 --- /dev/null +++ b/packages/cli/tests/session-continuity.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +let root: string; +let repoDir: string; +let origCwd: string; + +function git(args: string[]): string { + return execFileSync('git', args, { cwd: repoDir, encoding: 'utf-8', stdio: 'pipe' }).trim(); +} + +function commit(name: string): void { + writeFileSync(join(repoDir, name), `${name}\n`); + git(['add', '.']); + git(['commit', '-m', name]); +} + +beforeAll(() => { + origCwd = process.cwd(); + root = mkdtempSync(join(tmpdir(), 'diffity-session-')); + repoDir = join(root, 'repo'); + + execFileSync('git', ['init', '-b', 'main', repoDir], { stdio: 'pipe' }); + execFileSync('git', ['config', 'user.email', 'test@test.com'], { cwd: repoDir, stdio: 'pipe' }); + execFileSync('git', ['config', 'user.name', 'Test'], { cwd: repoDir, stdio: 'pipe' }); + commit('a.txt'); + + // Set before anything opens the database, which resolves its path once. + process.env.DIFFITY_DATA_DIR = join(root, 'notes'); + process.chdir(repoDir); +}); + +afterAll(() => { + process.chdir(origCwd); + delete process.env.DIFFITY_DATA_DIR; + rmSync(root, { recursive: true, force: true }); +}); + +describe('a session when HEAD moves', () => { + it('brings open threads and walkthroughs with it, and leaves closed ones behind', async () => { + const { findOrCreateSession } = await import('../src/session.js'); + const { createThread, getThreadsForSession, updateThreadStatus } = await import('../src/threads.js'); + const { createTour, getToursForSession } = await import('../src/tours.js'); + + const first = findOrCreateSession('work'); + const open = createThread(first.id, 'a.txt', 'new', 1, 1, 'P1: still a problem', { + name: 'Agent', + type: 'agent', + }); + const dealtWith = createThread(first.id, 'a.txt', 'new', 2, 2, 'P2: fixed already', { + name: 'Agent', + type: 'agent', + }); + updateThreadStatus(dealtWith.id, 'resolved'); + const tour = createTour(first.id, 'Reading order', ''); + + // Acting on a finding is what moves HEAD. + commit('b.txt'); + const second = findOrCreateSession('work'); + + expect(second.id).not.toBe(first.id); + + const carried = getThreadsForSession(second.id); + expect(carried.map(thread => thread.id)).toEqual([open.id]); + expect(getToursForSession(second.id).map(t => t.id)).toEqual([tour.id]); + + const left = getThreadsForSession(first.id); + expect(left.map(thread => thread.id)).toEqual([dealtWith.id]); + }); + + it('returns the same session while HEAD stays put', async () => { + const { findOrCreateSession } = await import('../src/session.js'); + + expect(findOrCreateSession('work').id).toBe(findOrCreateSession('work').id); + }); + + it('keeps refs apart', async () => { + const { findOrCreateSession } = await import('../src/session.js'); + + expect(findOrCreateSession('work').id).not.toBe(findOrCreateSession('main').id); + }); +});