From 86ffaec566ac78ef9cc062d7e72527e46f804a55 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 00:50:56 +0800 Subject: [PATCH 1/9] Harden watch fingerprinting --- scripts/build-diff-data.mjs | 72 +++++++++++++---- tests/remote-targets.test.mjs | 145 ++++++++++++++++++++++++++++++++-- 2 files changed, 197 insertions(+), 20 deletions(-) diff --git a/scripts/build-diff-data.mjs b/scripts/build-diff-data.mjs index a70c4e1..db6bcdf 100644 --- a/scripts/build-diff-data.mjs +++ b/scripts/build-diff-data.mjs @@ -4,8 +4,10 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import { existsSync, + lstatSync, mkdirSync, readFileSync, + readlinkSync, renameSync, statSync, writeFileSync, @@ -1146,6 +1148,29 @@ function build() { return true; } +function untrackedFileKind(stat) { + if (stat.isFile()) return 'file'; + if (stat.isSymbolicLink()) return 'symlink'; + if (stat.isDirectory()) return 'directory'; + return 'other'; +} + +function fingerprintUntrackedPath(content, path) { + const stat = lstatSync(resolve(repo, path), { bigint: true }); + content.update(path); + content.update('\0'); + content.update(untrackedFileKind(stat)); + content.update('\0'); + content.update(String(stat.size)); + content.update('\0'); + content.update(String(stat.mtimeNs)); + content.update('\0'); + if (stat.isSymbolicLink()) { + content.update(readlinkSync(resolve(repo, path))); + content.update('\0'); + } +} + function fingerprint() { let summariesTime = ''; if (!ignoreSummaryWatch && !noSummaries) { @@ -1163,7 +1188,7 @@ function fingerprint() { } const content = createHash('sha256'); content.update( - tryRepo(['diff', '--no-ext-diff', '--binary', 'HEAD', '--']), + tryRepo(['diff', '--no-ext-diff', '--no-textconv', '--binary', 'HEAD', '--']), ); const untracked = tryRepo([ 'ls-files', @@ -1175,10 +1200,7 @@ function fingerprint() { .filter((path) => path && !excludedPaths.has(path)) .sort(); for (const path of untracked) { - content.update('\0'); - content.update(path); - content.update('\0'); - content.update(readFileSync(resolve(repo, path))); + fingerprintUntrackedPath(content, path); } return [ tryRepo(['rev-parse', 'HEAD']), @@ -1202,18 +1224,38 @@ const refresh = () => { const started = refresh(); if (watching && started) { - let last = fingerprint(); + let last; let remoteWait = 0; - const watcher = setInterval(() => { - const next = fingerprint(); - remoteWait += watchInterval; - const remoteDue = remoteMode && remoteWait >= remoteRefreshInterval; - if (next !== last || remoteDue || watchContent) { - last = next; - remoteWait = 0; - if (!refresh()) clearInterval(watcher); + let watcher; + const stopWatching = (error) => { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; + if (watcher) clearInterval(watcher); + }; + const poll = () => { + try { + const next = fingerprint(); + if (last === undefined) { + last = next; + return true; + } + remoteWait += watchInterval; + const remoteDue = remoteMode && remoteWait >= remoteRefreshInterval; + if (next !== last || remoteDue || watchContent) { + last = next; + remoteWait = 0; + if (!refresh()) { + clearInterval(watcher); + return false; + } + } + return true; + } catch (error) { + stopWatching(error); + return false; } - }, watchInterval); + }; + if (poll()) watcher = setInterval(poll, watchInterval); } else if (watching) { process.exitCode = 1; } diff --git a/tests/remote-targets.test.mjs b/tests/remote-targets.test.mjs index 5eedd31..a93faff 100644 --- a/tests/remote-targets.test.mjs +++ b/tests/remote-targets.test.mjs @@ -1,12 +1,21 @@ import assert from "node:assert/strict"; import { execFileSync, spawn, spawnSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; -import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; const script = new URL("../scripts/build-diff-data.mjs", import.meta.url).pathname; +const unavailableSymlinkCodes = new Set(["EACCES", "EPERM"]); function git(repo, ...args) { return execFileSync("git", ["-C", repo, ...args], { @@ -163,13 +172,14 @@ async function publishFeatureUpdate(fixture, path, content) { return head; } -function startWatcher(repo, args) { +function startWatcher(repo, args, options = {}) { const child = spawn( process.execPath, [script, "--repo", repo, ...args], { env: { ...process.env, + ...options.env, DIFFSPLAIN_WATCH_INTERVAL_MS: "25", DIFFSPLAIN_REMOTE_REFRESH_INTERVAL_MS: "100", }, @@ -197,6 +207,16 @@ async function stopIfRunning(watched) { if (watched?.child.exitCode === null) await stop(watched.child); } +async function createDirectorySymlink(target, link) { + try { + await symlink(target, link, "dir"); + return undefined; + } catch (error) { + if (unavailableSymlinkCodes.has(error.code)) return error.code; + throw error; + } +} + test("builds a remote branch range without changing the checkout", async () => { const fixture = await makeRemoteRepo(); const output = join(fixture.root, "branch.json"); @@ -633,9 +653,12 @@ test("stops remote and pull-request lookups without replacing complete data", as test("watches local changes without changing the selected target", async () => { const fixture = await makeRemoteRepo(); const localOutput = join(fixture.root, "local-watch.json"); + const firstContent = "local update\n"; + const secondContent = "fresh update\n"; let watched; try { + assert.equal(Buffer.byteLength(firstContent), Buffer.byteLength(secondContent)); watched = startWatcher( fixture.repo, [ @@ -651,7 +674,7 @@ test("watches local changes without changing the selected target", async () => { (payload) => payload.repo.target.kind === "checkout", ); await new Promise((resolve) => setTimeout(resolve, 150)); - await writeFile(join(fixture.repo, "watched.txt"), "local update\n"); + await writeFile(join(fixture.repo, "watched.txt"), firstContent); const local = await waitForSnapshot( localOutput, watched, @@ -659,14 +682,14 @@ test("watches local changes without changing the selected target", async () => { ); assert.equal(local.repo.target.kind, "checkout"); - await writeFile(join(fixture.repo, "watched.txt"), "local update again\n"); + await writeFile(join(fixture.repo, "watched.txt"), secondContent); const refreshed = await waitForSnapshot( localOutput, watched, (payload) => payload.files .find((file) => file.path === "watched.txt") - ?.patch.includes("local update again"), + ?.patch.includes("fresh update"), ); assert.equal(refreshed.repo.target.kind, "checkout"); } finally { @@ -675,6 +698,118 @@ test("watches local changes without changing the selected target", async () => { } }); +test("does not run text converters while watching local changes", async () => { + const fixture = await makeRemoteRepo(); + const localOutput = join(fixture.root, "textconv-watch.json"); + const marker = join(fixture.root, "textconv-ran"); + let watched; + + try { + await writeFile(join(fixture.repo, ".gitattributes"), "converted.txt diff=marker\n"); + await writeFile(join(fixture.repo, "converted.txt"), "before\n"); + git(fixture.repo, "add", ".gitattributes", "converted.txt"); + git(fixture.repo, "commit", "-qm", "add converted file"); + git( + fixture.repo, + "config", + "diff.marker.textconv", + `sh -c 'touch ${marker}; cat'`, + ); + await writeFile(join(fixture.repo, "converted.txt"), "after\n"); + + watched = startWatcher(fixture.repo, ["--checkout", "--watch", "--output", localOutput]); + await waitForSnapshot(localOutput, watched, () => true); + await new Promise((resolve) => setTimeout(resolve, 150)); + assert.equal(existsSync(marker), false); + assert.equal(watched.child.exitCode, null); + } finally { + await stopIfRunning(watched); + await rm(fixture.root, { recursive: true, force: true }); + } +}); + +test("watches untracked symlinks without reading their targets", async (t) => { + const fixture = await makeRemoteRepo(); + const localOutput = join(fixture.root, "symlink-watch.json"); + const outside = join(fixture.root, "outside"); + const link = join(fixture.repo, "outside-link"); + let watched; + + try { + await mkdir(outside); + const unavailable = await createDirectorySymlink(outside, link); + if (unavailable) { + t.skip(`symlink creation is unavailable: ${unavailable}`); + return; + } + watched = startWatcher(fixture.repo, ["--checkout", "--watch", "--output", localOutput]); + const snapshot = await waitForSnapshot( + localOutput, + watched, + (payload) => payload.files.some((file) => file.path === "outside-link"), + ); + assert.equal(snapshot.files.find((file) => file.path === "outside-link")?.status, "added"); + await new Promise((resolve) => setTimeout(resolve, 150)); + assert.equal(watched.child.exitCode, null); + } finally { + await stopIfRunning(watched); + await rm(fixture.root, { recursive: true, force: true }); + } +}); + +test("exits cleanly when a fingerprint path disappears during polling", async () => { + const fixture = await makeRemoteRepo(); + const localOutput = join(fixture.root, "fingerprint-race-watch.json"); + const bin = join(fixture.root, "git-proxy"); + const proxy = join(bin, "git"); + const marker = join(fixture.root, "delete-on-ls-files"); + const raced = join(fixture.repo, "raced.txt"); + let watched; + + try { + await writeFile(raced, "race\n"); + await mkdir(bin); + await writeFile( + proxy, + `#!/usr/bin/env node +const { existsSync, unlinkSync } = require("node:fs"); +const { spawnSync } = require("node:child_process"); +const args = process.argv.slice(2); +const result = spawnSync(process.env.DIFFSPLAIN_REAL_GIT, args, { encoding: "utf8" }); +process.stdout.write(result.stdout || ""); +process.stderr.write(result.stderr || ""); +if (args.includes("ls-files") && existsSync(process.env.DIFFSPLAIN_RACE_MARKER)) { + unlinkSync(process.env.DIFFSPLAIN_RACE_PATH); +} +process.exit(result.status ?? 1); +`, + ); + await chmod(proxy, 0o755); + watched = startWatcher( + fixture.repo, + ["--checkout", "--watch", "--output", localOutput], + { + env: { + DIFFSPLAIN_RACE_MARKER: marker, + DIFFSPLAIN_RACE_PATH: raced, + DIFFSPLAIN_REAL_GIT: process.env.PATH.split(":").map((path) => join(path, "git")).find(existsSync), + PATH: `${bin}:${process.env.PATH}`, + }, + }, + ); + await waitForSnapshot(localOutput, watched, () => true); + await writeFile(marker, "delete the next fingerprint path\n"); + await waitFor(() => watched.child.exitCode !== null); + assert.equal(watched.child.exitCode, 1); + assert.match(watched.logs(), /ENOENT: no such file or directory, lstat/); + assert.equal((watched.logs().match(/ENOENT: no such file or directory, lstat/g) || []).length, 1); + assert.doesNotMatch(watched.logs(), /\n\s*at /); + } finally { + await stopIfRunning(watched); + await rm(fixture.root, { recursive: true, force: true }); + } +}); + test("periodically refreshes remote targets without changing the checkout", async () => { const fixture = await makeRemoteRepo(); const remoteOutput = join(fixture.root, "remote-watch.json"); From 9158fb9ef42ee88550ccdba93748f7bb8ceff6ca Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 00:56:31 +0800 Subject: [PATCH 2/9] Use platform path basename --- scripts/build-diff-data.mjs | 4 +-- tests/platform-shell.test.mjs | 50 ++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/scripts/build-diff-data.mjs b/scripts/build-diff-data.mjs index db6bcdf..481329b 100644 --- a/scripts/build-diff-data.mjs +++ b/scripts/build-diff-data.mjs @@ -12,7 +12,7 @@ import { statSync, writeFileSync, } from 'node:fs'; -import { dirname, relative, resolve } from 'node:path'; +import { basename, dirname, relative, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { summaryPath } from './summary-path.mjs'; @@ -1090,7 +1090,7 @@ function build() { const change = changeSummary(sourceSummaries.change, target.changeDefaults); const content = { repo: { - name: remoteRepository?.name || repo.split('/').pop(), + name: remoteRepository?.name || basename(repo), root: localWorkspace ? repo : target.remote?.url || repo, base: target.base, head: target.head, diff --git a/tests/platform-shell.test.mjs b/tests/platform-shell.test.mjs index 7a5a335..b83bed8 100644 --- a/tests/platform-shell.test.mjs +++ b/tests/platform-shell.test.mjs @@ -1,12 +1,18 @@ import assert from 'node:assert/strict'; -import { chmod, link, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { execFileSync } from 'node:child_process'; +import { chmod, link, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { basename, join } from 'node:path'; import test from 'node:test'; +import { fileURLToPath } from 'node:url'; import { findCommand } from '../scripts/coding-agents.mjs'; import { doctorReport } from '../scripts/doctor.mjs'; import { npmCommand } from '../scripts/release.mjs'; +const builder = fileURLToPath( + new URL('../scripts/build-diff-data.mjs', import.meta.url), +); + async function fakeCommand(directory, name) { if (process.platform === 'win32') { const path = join(directory, `${name}.EXE`); @@ -65,3 +71,45 @@ test('uses the host npm launcher without a shell wrapper', () => { assert.equal(npmCommand('darwin'), 'npm'); assert.equal(npmCommand('win32'), 'npm.cmd'); }); + +test('uses the checkout basename in builder output', async () => { + const root = await mkdtemp(join(tmpdir(), 'diffsplain-platform-builder-')); + const repo = join(root, 'platform-repo'); + const output = join(root, 'diff-data.json'); + + try { + execFileSync('git', ['init', '-q', '-b', 'main', repo]); + for (const [key, value] of [ + ['user.email', 'diffsplain@example.test'], + ['user.name', 'Diffsplain'], + ['commit.gpgsign', 'false'], + ]) { + execFileSync('git', ['-C', repo, 'config', key, value]); + } + await writeFile(join(repo, 'file.txt'), 'before\n'); + execFileSync('git', ['-C', repo, 'add', 'file.txt']); + execFileSync('git', ['-C', repo, 'commit', '-qm', 'base']); + await writeFile(join(repo, 'file.txt'), 'after\n'); + + execFileSync( + process.execPath, + [ + builder, + '--worktree', + '--repo', + repo, + '--no-summaries', + '--output', + output, + ], + { stdio: 'pipe' }, + ); + + const payload = JSON.parse(await readFile(output, 'utf8')); + assert.equal(payload.repo.name, basename(repo)); + assert.notEqual(payload.repo.name, repo); + assert.equal(payload.repo.root, repo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); From af5f5ac9ae378a4a53d131272a397272f2f73891 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 00:45:32 +0800 Subject: [PATCH 3/9] Cover the local server --- .c8rc.json | 3 ++- package.json | 2 +- tests/test-lanes.test.mjs | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.c8rc.json b/.c8rc.json index 1f371e0..f0583ad 100644 --- a/.c8rc.json +++ b/.c8rc.json @@ -4,7 +4,8 @@ "scripts/cli-args.mjs", "scripts/build-diff-data.mjs", "scripts/generate-summaries.mjs", - "scripts/present.mjs" + "scripts/present.mjs", + "scripts/serve-built.mjs" ], "reporter": [ "text", diff --git a/package.json b/package.json index 5d0134c..a8bfcad 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "test": "pnpm run test:unit && pnpm run test:integration && pnpm run test:coverage && pnpm run test:browser && pnpm run test:platform", "test:unit": "node --test tests/access-token.test.mjs tests/automation-trust.test.mjs tests/cache.test.mjs tests/cli-args.test.mjs tests/cli-docs.test.mjs tests/coding-agents.test.mjs tests/data-contract.test.mjs tests/doctor.test.mjs tests/landing-demo.test.mjs tests/package-manifest.test.mjs tests/plan-ledger.test.mjs tests/presenter-runtime.test.mjs tests/product-gate.test.mjs tests/release.test.mjs tests/summary-path.test.mjs tests/support-record.test.mjs tests/test-lanes.test.mjs tests/tool-profiles.test.mjs", "test:integration": "pnpm run build && node --test --test-concurrency=1 tests/dev.test.mjs tests/generate-summaries.test.mjs tests/live-update-speed.test.mjs tests/performance-gate.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/presenter-recovery.test.mjs tests/remote-targets.test.mjs tests/rendered-html.test.mjs tests/serve-built.test.mjs tests/setup-environments.test.mjs", - "test:coverage": "pnpm run build && c8 node --test --test-concurrency=1 tests/cli-args.test.mjs tests/generate-summaries.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/remote-targets.test.mjs", + "test:coverage": "pnpm run build && c8 node --test --test-concurrency=1 tests/cli-args.test.mjs tests/generate-summaries.test.mjs tests/present-agent.test.mjs tests/present-help.test.mjs tests/present-instances.test.mjs tests/remote-targets.test.mjs tests/serve-built.test.mjs", "test:browser": "pnpm run build && node --test tests/browser/*.test.mjs", "test:browser:install": "playwright install chromium", "test:cloud": "node --test tests/generate-summaries.test.mjs tests/present-agent.test.mjs", diff --git a/tests/test-lanes.test.mjs b/tests/test-lanes.test.mjs index 8f800a3..fc83578 100644 --- a/tests/test-lanes.test.mjs +++ b/tests/test-lanes.test.mjs @@ -34,6 +34,7 @@ test('keeps the test lanes separate and composes the complete test gate', async assert.match(scripts['test:integration'], /--test-concurrency=1/); assert.match(scripts['test:coverage'], /pnpm run build/); assert.match(scripts['test:coverage'], /--test-concurrency=1/); + assert.match(scripts['test:coverage'], /tests\/serve-built\.test\.mjs/); assert.equal( scripts['test:browser'], 'pnpm run build && node --test tests/browser/*.test.mjs', @@ -73,6 +74,7 @@ test('holds each core path to the documented coverage floor', async () => { 'scripts/build-diff-data.mjs', 'scripts/generate-summaries.mjs', 'scripts/present.mjs', + 'scripts/serve-built.mjs', ]); assert.deepEqual( { From 924b11c033a9f851c98f4552b0693df37c50a6ea Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 01:00:58 +0800 Subject: [PATCH 4/9] Align provider documentation --- PRODUCT.md | 5 ++++- docs/content/agent-notes.mdx | 7 +++++-- docs/content/development.mdx | 9 ++++++--- docs/content/index.mdx | 7 +++++-- tests/cli-docs.test.mjs | 35 ++++++++++++++++++++++++++++++++++- 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/PRODUCT.md b/PRODUCT.md index c5bf66b..b4bd336 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -28,7 +28,10 @@ Developers run `npx diffsplain` in a Git checkout. With no arguments, it compare - Compare the current checkout with its default branch when no target is passed. - Accept local paths, Git URLs, and GitHub `owner/name` repo names. - Report local dependency paths, versions, and readiness with `diffsplain doctor`. -- Try Codex, Claude, Copilot, Cursor, then OpenCode when no agent is chosen. +- Try Codex, Claude, Copilot, then OpenCode when no agent is chosen. Cursor + detection does not mean support: Diffsplain detects it only so it can report + that Cursor is unsupported. It has no supported read-only, no-network, + no-tool mode, so it cannot generate notes. - Show tracked and untracked worktree changes, exact local ranges, and remote branches as secondary targets. - Present full or shortened unified diffs, including binary-file metadata. - Pair the whole change and each file with agent-written summaries, reasons, details, and risks. diff --git a/docs/content/agent-notes.mdx b/docs/content/agent-notes.mdx index 17c7e28..59a08ee 100644 --- a/docs/content/agent-notes.mdx +++ b/docs/content/agent-notes.mdx @@ -6,8 +6,11 @@ sidebar: --- A coding agent writes short notes for each changed file by default. Diffsplain -tries Codex, Claude, Copilot, Cursor, then OpenCode, and uses the first installed -CLI. Each CLI uses its current login. +tries Codex, Claude, Copilot, then OpenCode, and uses the first installed CLI. +Each CLI uses its current login. Cursor detection does not mean support: +Diffsplain detects it only to give a clear unsupported-provider result. Cursor +stays disabled for note generation because it has no supported read-only, +no-network, no-tool mode. Use a plain diff when you do not want notes: diff --git a/docs/content/development.mdx b/docs/content/development.mdx index a4c3f8b..1b82cd7 100644 --- a/docs/content/development.mdx +++ b/docs/content/development.mdx @@ -165,9 +165,12 @@ corepack pnpm run cloud:check `cloud:check` runs the clean-checkout gate and the provider/browser tests. Those tests use fake coding providers and a fake browser command. Real Codex, -Claude, Copilot, Cursor, OpenCode, GitHub, or browser login is optional and is -needed only for a live integration task. Keep credentials in Codex environment -settings, not checked-in rules or scripts. +Claude, Copilot, OpenCode, GitHub, or browser login is optional and is needed +only for a live integration task. Diffsplain tries Codex, Claude, Copilot, then +OpenCode for notes. Cursor detection does not mean support: it is detected only +to report that Cursor is unsupported. Cursor stays disabled because it has no +supported read-only, no-network, no-tool mode. Keep credentials in Codex +environment settings, not checked-in rules or scripts. To test the linked-worktree path itself, run: diff --git a/docs/content/index.mdx b/docs/content/index.mdx index 70bf878..404e433 100644 --- a/docs/content/index.mdx +++ b/docs/content/index.mdx @@ -11,8 +11,11 @@ patch on the left and a short coding agent note on the right. ## Start a review -You need Node.js 22.13 or newer and a signed-in Codex, Claude, Copilot, Cursor, -or OpenCode CLI. +You need Node.js 22.13 or newer and a signed-in Codex, Claude, Copilot, or +OpenCode CLI. Diffsplain tries Codex, Claude, Copilot, then OpenCode when you +do not choose one. Cursor detection does not mean support: it is detected only +to report that Cursor is unsupported. Cursor has no supported read-only, +no-network, no-tool mode required for note generation, so it stays disabled. ```sh npx diffsplain diff --git a/tests/cli-docs.test.mjs b/tests/cli-docs.test.mjs index 9a0950a..e682b2f 100644 --- a/tests/cli-docs.test.mjs +++ b/tests/cli-docs.test.mjs @@ -6,11 +6,18 @@ import { helpText, parseCliArgs, } from '../scripts/cli-args.mjs'; +import { + agentDisabledReason, + enabledCodingAgents, +} from '../scripts/coding-agents.mjs'; -const [docs, agentNotes, packageText] = await Promise.all([ +const [docs, agentNotes, development, index, packageText, product] = await Promise.all([ readFile(new URL('../docs/content/cli.mdx', import.meta.url), 'utf8'), readFile(new URL('../docs/content/agent-notes.mdx', import.meta.url), 'utf8'), + readFile(new URL('../docs/content/development.mdx', import.meta.url), 'utf8'), + readFile(new URL('../docs/content/index.mdx', import.meta.url), 'utf8'), readFile(new URL('../package.json', import.meta.url), 'utf8'), + readFile(new URL('../PRODUCT.md', import.meta.url), 'utf8'), ]); test('lists each accepted option in public help and the CLI reference', () => { @@ -44,6 +51,32 @@ test('documents provider inputs and limits', () => { assert.doesNotMatch(agentNotes, /--agent claude[\s\S]{0,100}--reasoning/); }); +test('documents the enabled provider order and Cursor boundary', () => { + const providerNames = enabledCodingAgents.map( + (agent) => (agent === 'opencode' + ? 'OpenCode' + : agent[0].toUpperCase() + agent.slice(1)), + ); + const providerOrder = + `${providerNames.slice(0, -1).join(', ')}, then ${providerNames.at(-1)}`; + const cursorBoundary = agentDisabledReason('cursor') + ?.match(/read-only, no-network, no-tool mode/)?.[0]; + assert.ok(cursorBoundary); + + for (const document of [product, index, agentNotes, development]) { + const text = document.replace(/\s+/g, ' '); + assert.match(text, new RegExp(providerOrder)); + assert.match(text, /Cursor detection does not mean support/i); + assert.match(text, /Cursor.{0,100}unsupported/i); + assert.match(text, new RegExp(cursorBoundary)); + } + + assert.doesNotMatch( + development.replace(/\s+/g, ' '), + /Cursor.{0,100}(?:sign in|login)/i, + ); +}); + test('derives documented numeric defaults and bounds from the parser', () => { const batchSize = cliOptions['--batch-size']; const jobs = cliOptions['--jobs']; From 832598646f2cece2a7048243dd6fb67fed09f889 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 01:03:13 +0800 Subject: [PATCH 5/9] Cache pnpm dependencies in pull requests --- .github/workflows/product-gate.yml | 6 ++++-- .github/workflows/test-lanes.yml | 20 ++++++++++++++++++ tests/test-lanes.test.mjs | 33 ++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/.github/workflows/product-gate.yml b/.github/workflows/product-gate.yml index 981e774..b6fbd74 100644 --- a/.github/workflows/product-gate.yml +++ b/.github/workflows/product-gate.yml @@ -17,12 +17,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: ${{ matrix.node-version }} - - name: Enable the pinned pnpm version - run: corepack enable + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install lockfile dependencies run: pnpm install --frozen-lockfile - name: Run product gate diff --git a/.github/workflows/test-lanes.yml b/.github/workflows/test-lanes.yml index 7f98bd2..b2d85dc 100644 --- a/.github/workflows/test-lanes.yml +++ b/.github/workflows/test-lanes.yml @@ -20,10 +20,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: 22.13.0 + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: corepack pnpm install --frozen-lockfile - name: Run fast unit tests @@ -35,10 +39,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: 22.13.0 + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: corepack pnpm install --frozen-lockfile - name: Run Git and server tests @@ -50,10 +58,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: 22.13.0 + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: corepack pnpm install --frozen-lockfile - name: Check core coverage @@ -72,10 +84,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: 22.13.0 + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: corepack pnpm install --frozen-lockfile - name: Install browser system packages @@ -95,10 +111,14 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v6 + - name: Enable the pinned pnpm version + run: corepack enable - name: Set up Node uses: actions/setup-node@v6 with: node-version: 22.13.0 + cache: pnpm + cache-dependency-path: pnpm-lock.yaml - name: Install dependencies run: corepack pnpm install --frozen-lockfile - name: Check host shell assumptions diff --git a/tests/test-lanes.test.mjs b/tests/test-lanes.test.mjs index fc83578..d9eb318 100644 --- a/tests/test-lanes.test.mjs +++ b/tests/test-lanes.test.mjs @@ -12,6 +12,12 @@ function listedTests(command) { return command.match(/tests\/[^ ]+\.test\.mjs/g) || []; } +function workflowSteps(workflow) { + return [...workflow.matchAll( + /^(? *)- name: (?.+)\n(?(?:^\k {2}.+(?:\n|$))*)/gm, + )].map(({ groups }) => groups); +} + test('keeps the test lanes separate and composes the complete test gate', async () => { const packageJson = await json('package.json'); const scripts = packageJson.scripts; @@ -112,3 +118,30 @@ test('runs pull request lanes on Linux and scheduled shell checks elsewhere', as assert.match(workflow, /permissions:\n contents: read/); assert.doesNotMatch(workflow, /pull_request_target:|secrets\.|write-all/); }); + +test('caches pnpm dependencies before each Node setup', async () => { + const workflows = await Promise.all([ + 'product-gate.yml', + 'test-lanes.yml', + ].map(async (name) => ({ + name, + steps: workflowSteps(await readFile( + new URL(`../.github/workflows/${name}`, import.meta.url), + 'utf8', + )), + }))); + + for (const { name, steps } of workflows) { + const nodeSetups = steps + .map((step, index) => ({ ...step, index })) + .filter(({ body }) => body.includes('uses: actions/setup-node@v6')); + const expectedSetups = name === 'product-gate.yml' ? 1 : 5; + + assert.equal(nodeSetups.length, expectedSetups); + for (const { body, index } of nodeSetups) { + assert.match(body, /^\s+cache: pnpm$/m); + assert.match(body, /^\s+cache-dependency-path: pnpm-lock.yaml$/m); + assert.match(steps[index - 1]?.body ?? '', /^\s+run: corepack enable$/m); + } + } +}); From aa09d7f12e5cce9355d167990068dd4290317914 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 00:52:21 +0800 Subject: [PATCH 6/9] Bound cache leases by heartbeat age --- scripts/cache.mjs | 19 ------------------- tests/cache.test.mjs | 24 +++++++++++++++++++++++- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/scripts/cache.mjs b/scripts/cache.mjs index f9c69c3..d2d631a 100644 --- a/scripts/cache.mjs +++ b/scripts/cache.mjs @@ -40,32 +40,14 @@ function leaseRecord(path) { } } -function leaseOwnerIsActive(record) { - return ( - record?.hostname === hostname() && - Number.isSafeInteger(record.pid) && - processIsAlive(record.pid) - ); -} - function leaseIsActive(path, now = Date.now(), duration = leaseDurationMs) { try { - if (leaseOwnerIsActive(leaseRecord(path))) return true; return now - statSync(path).mtimeMs < duration; } catch { return false; } } -function processIsAlive(pid) { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return error?.code === 'EPERM'; - } -} - function createLease(path, record, duration) { const descriptor = openSync(path, 'wx', 0o600); writeFileSync(descriptor, `${JSON.stringify(record)}\n`); @@ -116,7 +98,6 @@ function rejectActiveLease(active, path) { function handleLeaseConflict(error, path, now, duration) { rejectNonConflict(error); const observed = leaseRecord(path); - rejectActiveLease(leaseOwnerIsActive(observed), path); rejectActiveLease(leaseIsActive(path, now, duration), path); removeStaleLease(path, observed?.token); } diff --git a/tests/cache.test.mjs b/tests/cache.test.mjs index 493d167..c0c4811 100644 --- a/tests/cache.test.mjs +++ b/tests/cache.test.mjs @@ -32,7 +32,7 @@ test('fences a delayed writer after stale-lease recovery', async () => { const older = acquireLease(lock, { token: 'older', now, - pid: 999_999, + pid: process.pid, }); publishLeaseFile(older, note, 'older result'); await utimes(lock, new Date(now - leaseDurationMs - 1), new Date(now - leaseDurationMs - 1)); @@ -49,6 +49,28 @@ test('fences a delayed writer after stale-lease recovery', async () => { } }); +test('keeps a fresh lease active when its recorded PID is dead', async () => { + const root = await mkdtemp(join(tmpdir(), 'diffsplain-cache-')); + const lock = join(root, 'summaries', 'target.json.lock'); + const now = Date.now(); + let lease; + try { + lease = acquireLease(lock, { + token: 'fresh-dead-pid', + now, + pid: 999_999, + }); + + assert.throws( + () => acquireLease(lock, { token: 'newer', now }), + /already being generated/i, + ); + } finally { + if (lease) releaseLease(lease); + await rm(root, { recursive: true, force: true }); + } +}); + test('does not remove a new lease found during stale recovery', async () => { const root = await mkdtemp(join(tmpdir(), 'diffsplain-cache-')); const lock = join(root, 'summaries', 'target.json.lock'); From 383391a2e4baf113325416e3ec86e8910ea82c49 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Sun, 2 Aug 2026 01:09:37 +0800 Subject: [PATCH 7/9] Index picker files once --- app/page.tsx | 10 ++-- tests/browser/review-journey.test.mjs | 69 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/app/page.tsx b/app/page.tsx index 4d917f1..e824e6e 100644 --- a/app/page.tsx +++ b/app/page.tsx @@ -480,8 +480,11 @@ export default function Home() { const visibleFiles = useMemo(() => { const cleanQuery = query.trim().toLowerCase(); - if (!cleanQuery) return files; - return files.filter((file) => file.path.toLowerCase().includes(cleanQuery)); + return files.flatMap((file, index) => + !cleanQuery || file.path.toLowerCase().includes(cleanQuery) + ? [{ file, index }] + : [], + ); }, [files, query]); if (!snapshot) { @@ -918,8 +921,7 @@ export default function Home() { className="picker-list" id="file-picker-list" > - {visibleFiles.map((file) => { - const index = files.findIndex((item) => item.path === file.path); + {visibleFiles.map(({ file, index }) => { const active = file.path === currentFile.path; return (