From 70ba0832c23115f22adc9e259ed906297cbde0ef Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Fri, 21 Aug 2026 11:40:50 +0200 Subject: [PATCH] feat: let a project keep its own review notes Review notes lived only in ~/.diffity/, which is wrong for two cases: several worktrees of one repository each want their own notes, and notes that belong to a project should be able to travel with it rather than with the machine. DIFFITY_DATA_DIR, or dataDir in a committed .diffity.json, now chooses the directory. Relative paths resolve against the repository root and the environment wins over the file. A directory chosen for one project is used as given - the hashed subdirectory only exists to keep repositories apart inside the shared default. diffity warns at startup when the data directory sits inside the working tree without being ignored, since the notes would otherwise appear as untracked files in the diff being reviewed. The database quotes the code under review through anchor_content, so it and its WAL are created readable only by the owner, and so is the directory. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- README.md | 22 ++++++++ packages/cli/src/db.ts | 14 +++++ packages/cli/src/index.ts | 8 ++- packages/git/src/config.ts | 49 +++++++++++++++++ packages/git/src/index.ts | 4 +- packages/git/src/repo.ts | 35 ++++++++++-- packages/git/tests/data-dir.test.ts | 84 +++++++++++++++++++++++++++++ 7 files changed, 209 insertions(+), 7 deletions(-) create mode 100644 packages/git/src/config.ts create mode 100644 packages/git/tests/data-dir.test.ts diff --git a/README.md b/README.md index 4498905..34c0e05 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,7 @@ diffity list --json # machine-readable output | -------------- | ------------------------------------------------------------------------- | | `DIFFITY_HOST` | Hostname used in the printed URL (default: `localhost`). | | `DIFFITY_BIND` | Interface the server listens on (default: `127.0.0.1`). | +| `DIFFITY_DATA_DIR` | Where review notes are kept (default: `~/.diffity/`). | Useful when running diffity inside a VM or container and opening it from another machine: @@ -254,6 +255,27 @@ The server has no authentication: anything that can reach it can read the diff, repository's files and the review comments. Only widen `DIFFITY_BIND` on a network you trust. +## Where review notes live + +Review threads, walkthroughs and sessions are kept in a SQLite database. By default that is +`~/.diffity//reviews.db`, one per repository. + +A project can keep its own instead, which is what you want when several worktrees of the same +repository each need their own notes, or when the notes should travel with the project rather +than the machine. Commit a `.diffity.json` at the repository root: + +```json +{ "dataDir": "../.diffity" } +``` + +Relative paths resolve against the repository root, absolute paths are used as given, and +`DIFFITY_DATA_DIR` overrides both. A directory chosen this way is used as-is — no hashed +subdirectory, since there is nothing to disambiguate. + +Point it **outside** the working tree, or add it to `.gitignore`. Otherwise the notes show up as +untracked files in the very diff you are reviewing; diffity warns on startup when that happens. +The database quotes the code under review, so it is created readable only by you. + ## License [PolyForm Shield 1.0.0](./LICENSE) © [Kamran Ahmed](https://x.com/kamrify) diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index a7113a5..adcad7e 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -1,3 +1,4 @@ +import { chmodSync } from 'node:fs'; import { createRequire } from 'node:module'; import { join } from 'node:path'; import type { DatabaseSync, SQLInputValue } from 'node:sqlite'; @@ -19,6 +20,18 @@ function loadSqlite(): { DatabaseSync: new (path: string) => DatabaseSync } { } } +// The database holds `anchor_content` — the actual source lines a comment is attached to — so +// it must not be world-readable. WAL and shared-memory siblings hold the same content. +function restrictToOwner(dbPath: string): void { + for (const path of [dbPath, `${dbPath}-wal`, `${dbPath}-shm`]) { + try { + chmodSync(path, 0o600); + } catch { + // Not all siblings exist at every moment; the ones that do are what matter. + } + } +} + export function getDb(): DatabaseSync { if (db) { return db; @@ -27,6 +40,7 @@ export function getDb(): DatabaseSync { const { DatabaseSync: Database } = loadSqlite(); const dbPath = join(getDiffityDir(), 'reviews.db'); db = new Database(dbPath); + restrictToOwner(dbPath); db.exec('PRAGMA journal_mode = WAL'); db.exec('PRAGMA foreign_keys = ON'); migrateDb(db); diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index f3abab4..62f81f7 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -3,7 +3,7 @@ import { createHash } from 'node:crypto'; import { createRequire } from 'node:module'; import open from 'open'; import pc from 'picocolors'; -import { isGitRepo, isValidGitRef, getRepoRoot, getRepoName, normalizeRef, WORKING_TREE_REFS } from '@diffity/git'; +import { isGitRepo, isValidGitRef, getRepoRoot, getRepoName, normalizeRef, getDiffityDirPath, isDataDirUntracked, WORKING_TREE_REFS } from '@diffity/git'; import type { PrBase } from '@diffity/github'; import { isGitHubPrUrl, @@ -235,6 +235,12 @@ range syntax (main..feature, main...feature) also work.`) } const repoRoot = getRepoRoot(); + + if (!opts.quiet && isDataDirUntracked()) { + console.log(pc.yellow(` Note: review notes are kept in ${getDiffityDirPath()}, which git does not ignore.`)); + console.log(pc.dim(' Add it to .gitignore so they stay out of the diff you are reviewing.')); + } + const repoHash = createHash('sha256').update(repoRoot).digest('hex').slice(0, 12); const repoName = getRepoName(); diff --git a/packages/git/src/config.ts b/packages/git/src/config.ts new file mode 100644 index 0000000..9c1ecb5 --- /dev/null +++ b/packages/git/src/config.ts @@ -0,0 +1,49 @@ +import { readFileSync, existsSync } from 'node:fs'; +import { createHash } from 'node:crypto'; +import { isAbsolute, join, resolve } from 'node:path'; + +export const REPO_CONFIG_FILE = '.diffity.json'; + +export interface RepoConfig { + /** + * Where review threads, walkthroughs and sessions are kept. Relative paths resolve against + * the repository root, so a project can keep its review notes with itself. + */ + dataDir?: string; +} + +export function readRepoConfig(repoRoot: string): RepoConfig { + const path = join(repoRoot, REPO_CONFIG_FILE); + if (!existsSync(path)) { + return {}; + } + + try { + const parsed = JSON.parse(readFileSync(path, 'utf-8')) as RepoConfig; + return typeof parsed?.dataDir === 'string' ? { dataDir: parsed.dataDir } : {}; + } catch { + // A malformed config must not stop a review; the default is always usable. + return {}; + } +} + +/** + * The hashed subdirectory only exists to keep repositories apart inside the shared default + * location. A data directory chosen for one project needs no such disambiguation, so it is + * used as given — `.diffity/reviews.db` rather than `.diffity//reviews.db`. + */ +export function resolveDataDir(input: { + repoRoot: string; + homeDir: string; + envDir?: string; + configDir?: string; +}): string { + const chosen = input.envDir?.trim() || input.configDir?.trim(); + + if (chosen) { + return isAbsolute(chosen) ? chosen : resolve(input.repoRoot, chosen); + } + + const hash = createHash('sha256').update(input.repoRoot).digest('hex').slice(0, 12); + return join(input.homeDir, '.diffity', hash); +} diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 2031e21..a876586 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -1,9 +1,11 @@ export type { Commit, RepoInfo } from './types.js'; export type { RefCapabilities } from './repo.js'; -export { isGitRepo, getRepoRoot, getRepoName, getCurrentBranch, getRepoInfo, getHeadHash, getDiffityDir, getDiffityDirPath, getRefCapabilities, isValidGitRef } from './repo.js'; +export { isGitRepo, getRepoRoot, getRepoName, getCurrentBranch, getRepoInfo, getHeadHash, getDiffityDir, getDiffityDirPath, isDataDirUntracked, getRefCapabilities, isValidGitRef } from './repo.js'; export { getDiff, getDiffFiles, getDiffStat, getDiffStatForRef, getUntrackedFiles, getUntrackedDiff, getFileContent, getFileLineCount, getMergeBase, normalizeRef, resolveBaseRef, resolveThroughUpstream, resolveDiffArgs, resolveRef, revertFile, revertHunk, WORKING_TREE_REFS } from './diff.js'; export type { RefDiffArgs } from './diff.js'; export { getStagedFiles, getUnstagedFiles, isDirty } from './status.js'; export { getRecentCommits } from './commits.js'; +export { readRepoConfig, resolveDataDir, REPO_CONFIG_FILE } from './config.js'; +export type { RepoConfig } from './config.js'; export { getTree, getTreeEntries, getTreeFingerprint, getWorkingTreeFileContent, getWorkingTreeRawFile, resolveInRepo } from './tree.js'; export type { TreeEntry } from './tree.js'; diff --git a/packages/git/src/repo.ts b/packages/git/src/repo.ts index aded22e..eabeffa 100644 --- a/packages/git/src/repo.ts +++ b/packages/git/src/repo.ts @@ -1,9 +1,9 @@ import { execFileSync, execSync } from 'node:child_process'; -import { createHash } from 'node:crypto'; import { mkdirSync } from 'node:fs'; -import { join } from 'node:path'; +import { sep } from 'node:path'; import { homedir } from 'node:os'; import { exec } from './exec.js'; +import { readRepoConfig, resolveDataDir } from './config.js'; import { WORKING_TREE_REFS } from './diff.js'; import type { RepoInfo } from './types.js'; @@ -47,16 +47,41 @@ export function getHeadHash(): string { export function getDiffityDirPath(): string { const repoRoot = getRepoRoot(); - const hash = createHash('sha256').update(repoRoot).digest('hex').slice(0, 12); - return join(homedir(), '.diffity', hash); + return resolveDataDir({ + repoRoot, + homeDir: homedir(), + envDir: process.env.DIFFITY_DATA_DIR, + configDir: readRepoConfig(repoRoot).dataDir, + }); } export function getDiffityDir(): string { const dir = getDiffityDirPath(); - mkdirSync(dir, { recursive: true }); + // Review notes quote the code under review, so they are not readable by other accounts. + mkdirSync(dir, { recursive: true, mode: 0o700 }); return dir; } +/** + * True when the data directory sits inside the working tree without git ignoring it, which + * would otherwise show review notes as untracked changes in the very diff being reviewed. + */ +export function isDataDirUntracked(): boolean { + const dir = getDiffityDirPath(); + const repoRoot = getRepoRoot(); + + if (!dir.startsWith(repoRoot + sep)) { + return false; + } + + try { + execFileSync('git', ['check-ignore', '--quiet', dir], { stdio: 'pipe' }); + return false; + } catch { + return true; + } +} + export function isValidGitRef(ref: string): boolean { if (ref.includes('...')) { const parts = ref.split('...'); diff --git a/packages/git/tests/data-dir.test.ts b/packages/git/tests/data-dir.test.ts new file mode 100644 index 0000000..efe18d6 --- /dev/null +++ b/packages/git/tests/data-dir.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { readRepoConfig, resolveDataDir } from '../src/config.js'; + +let repoRoot: string; +const homeDir = '/home/someone'; + +beforeEach(() => { + repoRoot = mkdtempSync(join(tmpdir(), 'diffity-datadir-')); +}); + +afterEach(() => { + rmSync(repoRoot, { recursive: true, force: true }); +}); + +describe('resolveDataDir', () => { + it('keeps repositories apart under the shared default', () => { + const a = resolveDataDir({ repoRoot: '/repos/a', homeDir }); + const b = resolveDataDir({ repoRoot: '/repos/b', homeDir }); + + expect(a).toMatch(/^\/home\/someone\/\.diffity\/[0-9a-f]{12}$/); + expect(a).not.toBe(b); + }); + + it('is stable for the same repository', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir })).toBe( + resolveDataDir({ repoRoot: '/repos/a', homeDir }), + ); + }); + + it('uses a configured relative directory inside the project, with no hash', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, configDir: '.diffity' })).toBe( + '/repos/a/.diffity', + ); + }); + + it('uses a configured absolute directory as given', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, configDir: '/srv/notes' })).toBe( + '/srv/notes', + ); + }); + + it('lets the environment win over the repository config', () => { + expect( + resolveDataDir({ repoRoot: '/repos/a', homeDir, envDir: '/srv/env', configDir: '.diffity' }), + ).toBe('/srv/env'); + }); + + it('ignores blank values rather than resolving to the repository root', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, envDir: ' ' })).toMatch(/\.diffity\//); + }); +}); + +describe('readRepoConfig', () => { + it('reads dataDir', () => { + writeFileSync(join(repoRoot, '.diffity.json'), JSON.stringify({ dataDir: '.notes' })); + + expect(readRepoConfig(repoRoot)).toEqual({ dataDir: '.notes' }); + }); + + it('is empty when there is no config', () => { + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('survives malformed json rather than failing the review', () => { + writeFileSync(join(repoRoot, '.diffity.json'), '{ not json'); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('ignores a dataDir of the wrong type', () => { + writeFileSync(join(repoRoot, '.diffity.json'), JSON.stringify({ dataDir: 42 })); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('ignores a directory instead of a config file', () => { + mkdirSync(join(repoRoot, '.diffity.json')); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); +});