From 465813499cde311ae6da4f620f5e202934082881 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Fri, 21 Aug 2026 11:42:11 +0200 Subject: [PATCH] fix(session): keep open findings when HEAD moves A session is identified by ref and HEAD hash together, so the first commit after a review created a new one and left every thread and walkthrough behind in the old session - present in the database, invisible in the browser. That breaks reviewing your own change exactly where it matters: acting on a finding is what moves HEAD. Open threads and walkthroughs are now moved into the new session. Resolved and dismissed threads stay behind, since 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; anchor_content is already stored for that. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- package-lock.json | 3 +- package.json | 2 +- packages/cli/package.json | 7 +- packages/cli/src/session.ts | 33 +++++++ packages/cli/tests/session-continuity.test.ts | 85 +++++++++++++++++++ 5 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 packages/cli/tests/session-continuity.test.ts 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); + }); +});