From 6d1949e139315fb873ac17661540e2dba587d47c Mon Sep 17 00:00:00 2001 From: Art Pai Date: Wed, 16 Sep 2026 23:56:27 +1000 Subject: [PATCH 1/2] Exclude files and directories from inspect captures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add repeatable `inspect --exclude ` beside the `-- ...` list so an agent can drop unrelated files without touching the working tree, staging, or ignore rules. Exclusions are literal Git pathspecs applied before Diffwalk reads any file, so an excluded file cannot block capture. Co-Authored-By: ことね --- docs/architecture.md | 7 +- docs/usage.md | 31 ++++-- skills/diffwalk/SKILL.md | 30 +++--- src/authoring/git.ts | 20 +++- src/cli.ts | 7 +- src/cli/commands/inspect.ts | 37 +++++-- test/cli.test.ts | 198 +++++++++++++++++++++++++++++++++++- test/git.test.ts | 188 ++++++++++++++++++++++++++++++++++ 8 files changed, 484 insertions(+), 34 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 976f223..4b21299 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -114,9 +114,10 @@ exact corresponding diffs in a deliberate order. - `src/format/schema.ts`: boundary-only Zod schemas for the machine-owned capture and the author-edited explanations, plus the version 1 ExplainDocument and its optional attribution metadata. - `src/authoring/git.ts`: captures staged, unstaged, deleted, renamed, and untracked text - and binary files from an immutable Git base commit, optionally reading the index or - limiting the capture to named paths. A text side keeps its UTF-8 content; a binary side - keeps only its byte size and SHA-256 content hash. + and binary files from an immutable Git base commit, optionally reading the index and + limiting the capture to literal selected paths or away from literal excluded paths, + with Git applying every exclusion before any file is read. A text side keeps its UTF-8 + content; a binary side keeps only its byte size and SHA-256 content hash. - `src/authoring/capture.ts`: derives text change blocks, file-level binary change blocks, and the content `captureId`, and materializes exact section patches from capture plus explanations. Text-only files hash into `captureId` exactly as before binary support, so diff --git a/docs/usage.md b/docs/usage.md index a87eda6..83f26bf 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -73,12 +73,31 @@ publication tokens: Capture selected working-tree changes: ```bash -diffwalk inspect --staged # staged changes only -diffwalk inspect -- src/a.ts src/b.ts # selected paths -diffwalk inspect --staged -- src/a.ts # both -diffwalk inspect --base main # working tree relative to main +diffwalk inspect --staged # staged changes only +diffwalk inspect -- src/a.ts src/b.ts # selected paths +diffwalk inspect --staged -- src/a.ts # both +diffwalk inspect --base main # working tree relative to main +diffwalk inspect --exclude experiments # omit a file or directory +diffwalk inspect --exclude notes.md --exclude experiments +diffwalk inspect --exclude src/legacy -- src # exclusion wins inside a selected path ``` +`--exclude` is repeatable and takes literal file or directory paths relative to the +repository root. A directory excludes everything under it on path boundaries, so +`--exclude experiments` also omits `experiments/old.ts` but not `experiments.ts`. +Positive paths after `--` and `--exclude` values combine: a change is captured when it +matches the `--` list (or that list is empty) and matches no `--exclude` value, so an +exclusion always wins. Both selections are literal, so `--exclude 'notes[1].md'` names +that exact file and never a Git pattern. Exclusions apply to working-tree captures +(including untracked files) and to `--staged` captures, and are applied before Diffwalk +reads or validates files, so an unsupported file outside the requested scope cannot block +the capture. +When a rename crosses the selection, only its in-scope side is captured: moving a file +out of an excluded directory appears as an addition, and moving one in as a deletion. +When the selection matches no changes, `inspect` stops with `Nothing to capture` instead +of writing an empty walk. Say which paths you excluded when you share the review, since +the review cannot show changes that were never captured. + Or capture committed changes without checking out either revision: ```bash @@ -86,8 +105,8 @@ diffwalk inspect # commit relative to its first parent diffwalk inspect --from main --to feature # compare two committed revisions ``` -Revision captures ignore local changes and cannot be limited by path. Single-commit -inspection requires a parent, so it does not support root commits. +Revision captures ignore local changes and cannot be limited by path or `--exclude`. +Single-commit inspection requires a parent, so it does not support root commits. `.diffwalk/current` selects the walk used by later commands. Capturing the same source and contents reuses it; a different capture creates a new walk and preserves previous walks. diff --git a/skills/diffwalk/SKILL.md b/skills/diffwalk/SKILL.md index bf2bd5e..5f818b8 100644 --- a/skills/diffwalk/SKILL.md +++ b/skills/diffwalk/SKILL.md @@ -15,12 +15,16 @@ explained. - Run `diffwalk inspect` for staged, unstaged, renamed, deleted, and untracked working-tree changes relative to `HEAD`. Use `--base ` when the user names a different working-tree base. - - Add `--staged` to capture the index instead of the working tree, or list paths - after `--` to limit the capture to them, for example - `diffwalk inspect --staged -- src/a.ts`. Path limiting applies only to - working-tree captures. Paths are literal file or directory names relative to - the repository root; there is no `--exclude` option or Git pathspec magic. - To omit unrelated files, list the files or directories that belong in the review. + - Add `--staged` to capture the index instead of the working tree. Limit a + working-tree capture with literal paths after `--`, with repeatable + `--exclude `, or both, for example `diffwalk inspect --staged -- src/a.ts` + or `diffwalk inspect --exclude experiments --exclude notes.md`. Paths are + literal file or directory names relative to the repository root: there is no + Git pathspec magic, `--exclude experiments` omits the whole directory, and an + exclusion always wins over a `--` path. Selection applies only to local captures + (the working tree or the index); commit and range captures reject it. When the + selection matches no changes, `inspect` stops with `Nothing to capture`. Prefer + `--exclude` over listing every included path. - Run `diffwalk inspect ` for one commit relative to its first parent. A root commit has no first parent, so use an explicit range instead. - Run `diffwalk inspect --from --to ` for a committed range. @@ -171,11 +175,13 @@ hide ownership or order; otherwise let Diffwalk's exact diff carry the code. Diffwalk reports it. Symbolic links and non-file Git paths are still rejected at capture time; do not bypass that boundary. If the reported files are outside the requested review scope, rerun - `inspect -- ...` with the relevant files or directories and continue. - Use `--staged` only when the requested review scope is the index. State what was - omitted; do not change files, staging, or Git ignore rules to make capture succeed. - If unsupported files belong to the requested scope, report the limitation rather - than silently omitting them. + `inspect -- ...` to include only the relevant files, or rerun + `inspect --exclude ...` to drop the irrelevant ones, and continue. Use + `--staged` only when the requested review scope is the index. Always state which + paths you excluded or omitted: an omission is intentional, and reviewers cannot see + changes that were never captured. Do not change files, staging, or Git ignore rules + to make capture succeed. If unsupported files belong to the requested scope, report + the limitation rather than silently omitting them. - Binary files are represented at file level, not by their bytes. The capture records the path, status, modes, and each side's byte size and SHA-256 hash; the review shows a metadata card instead of a patch. A side counts as binary when its bytes contain a @@ -191,7 +197,7 @@ hide ownership or order; otherwise let Diffwalk's exact diff carry the code. ## Commands ```bash -diffwalk inspect [revision] [--staged] [--base ] [--from --to ] [--output ] [--explanations ] [-- ...] +diffwalk inspect [revision] [--staged] [--base ] [--from --to ] [--exclude ...] [--output ] [--explanations ] [-- ...] diffwalk walks diffwalk use diffwalk delete diff --git a/src/authoring/git.ts b/src/authoring/git.ts index 9f98427..bff4a42 100644 --- a/src/authoring/git.ts +++ b/src/authoring/git.ts @@ -21,6 +21,7 @@ export interface GitCapture { export interface CaptureGitChangesOptions { staged?: boolean paths?: string[] + exclude?: string[] } export interface GitRevisionCapture extends GitCapture { @@ -45,15 +46,16 @@ export async function captureGitChanges( cwd = process.cwd(), options: CaptureGitChangesOptions = {}, ): Promise { - const { staged = false, paths = [] } = options + const { staged = false, paths = [], exclude = [] } = options const root = (await gitText(['rev-parse', '--show-toplevel'], cwd)).trim() const baseCommit = ( await gitText(['rev-parse', '--verify', '--end-of-options', `${base}^{commit}`], root) ).trim() - const pathEnvironment = paths.length > 0 ? { GIT_LITERAL_PATHSPECS: '1' } : {} + const pathspecs = selectionPathspecs(paths, exclude) + const pathEnvironment = pathspecs.length > 0 ? { GIT_LITERAL_PATHSPECS: '0' } : {} const changes = parseGitChanges( await gitBytes( - ['diff', '--raw', '-z', '--find-renames', ...(staged ? ['--cached'] : []), baseCommit, '--', ...paths], + ['diff', '--raw', '-z', '--find-renames', ...(staged ? ['--cached'] : []), baseCommit, '--', ...pathspecs], root, pathEnvironment, ), @@ -119,7 +121,7 @@ export async function captureGitChanges( if (!staged) { const untracked = splitNulls( await gitBytes( - ['ls-files', '--others', '--exclude-standard', '--exclude=.diffwalk/', '-z', '--', ...paths], + ['ls-files', '--others', '--exclude-standard', '--exclude=.diffwalk/', '-z', '--', ...pathspecs], root, pathEnvironment, ), @@ -409,6 +411,16 @@ function splitNulls(bytes: Uint8Array): string[] { return value === '' ? [] : value.slice(0, value.endsWith('\0') ? -1 : undefined).split('\0') } +// Positive paths and exclusions are both literal, so special characters name a file rather +// than a Git pattern, and a directory matches everything it contains. Git applies every +// exclusion after the positive paths, so an exclusion always wins over a `--` path. +function selectionPathspecs(paths: string[], exclude: string[]): string[] { + return [ + ...paths.map((path) => `:(literal)${path}`), + ...exclude.map((path) => `:(exclude,literal)${path}`), + ] +} + function parseGitChanges(bytes: Uint8Array): GitChange[] { const fields = splitNulls(bytes) const changes: GitChange[] = [] diff --git a/src/cli.ts b/src/cli.ts index 08b7d6e..a7f8b4b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -41,10 +41,11 @@ function createCli(): Command { .option('--base ', 'Git base to diff against') .option('--from ', 'Committed revision range start') .option('--to ', 'Committed revision range end') + .option('--exclude ', 'Exclude a literal file or directory; repeatable', collectExclude, []) .option('--output ', 'Write capture to an explicit path') .option('--explanations ', 'Write or preserve authoring YAML at an explicit path') .allowExcessArguments(true) - .addHelpText('after', '\nLimit a working-tree capture with --staged or a `-- ...` list.') + .addHelpText('after', '\nLimit a working-tree capture with --staged, a `-- ...` list, or repeatable `--exclude `. An exclusion always wins over an included path.') .action((_revision: string | undefined, options: Record, command: Command) => { const positionals = inspectPositionals(command) return inspectChanges( @@ -151,6 +152,10 @@ function validateRevisionCount(count: number, hasPathSeparator: boolean): void { } } +function collectExclude(path: string, paths: string[]): string[] { + return [...paths, path] +} + function reportError(error: unknown): void { console.error(error instanceof Error ? error.message : String(error)) process.exitCode = 1 diff --git a/src/cli/commands/inspect.ts b/src/cli/commands/inspect.ts index 723468f..f9b01de 100644 --- a/src/cli/commands/inspect.ts +++ b/src/cli/commands/inspect.ts @@ -19,6 +19,7 @@ const inspectOptionsSchema = z.object({ base: z.string().optional(), from: z.string().optional(), to: z.string().optional(), + exclude: z.array(z.string()).default([]), output: z.string().optional(), explanations: z.string().optional(), }) @@ -29,16 +30,18 @@ export async function inspectChanges( options: z.input, paths: string[] = [], ): Promise { - const { base, from, to, staged, output, explanations } = inspectOptionsSchema.parse(options) - validateCaptureOptions(revision, { base, from, to, staged }, paths) + const { base, from, to, staged, exclude, output, explanations } = inspectOptionsSchema.parse(options) + validateCaptureOptions(revision, { base, from, to, staged, exclude }, paths) + validateSelectionPaths(paths, exclude) const capturedAt = new Date().toISOString() - const capture = await captureChanges(revision, { base, from, to, staged }, paths, capturedAt) + const capture = await captureChanges(revision, { base, from, to, staged, exclude }, paths, capturedAt) + validateSelection(capture, paths, exclude) await saveCapture(capture, { output, explanations }, capturedAt) } function validateCaptureOptions( revision: string | undefined, - { base, from, to, staged }: Pick, + { base, from, to, staged, exclude }: Pick, paths: string[], ): void { if (from !== undefined || to !== undefined) { @@ -51,6 +54,9 @@ function validateCaptureOptions( if (paths.length > 0) { throw new UsageError('Path limiting applies only to working-tree captures') } + if (exclude.length > 0) { + throw new UsageError('--exclude applies only to working-tree captures') + } if (staged) { throw new UsageError('Do not combine --staged with --from/--to') } @@ -61,15 +67,34 @@ function validateCaptureOptions( if (paths.length > 0) { throw new UsageError('Path limiting applies only to working-tree captures') } + if (exclude.length > 0) { + throw new UsageError('--exclude applies only to working-tree captures') + } if (staged) { throw new UsageError('Do not combine --staged with a positional commit revision') } } } +function validateSelection(capture: ExplainCapture, paths: string[], exclude: string[]): void { + if ((paths.length > 0 || exclude.length > 0) && capture.files.length === 0) { + throw new UsageError( + 'Nothing to capture: the selected paths and --exclude values match no working-tree changes', + ) + } +} + +// Git reads an empty literal pathspec as "match everything", so a missing shell variable +// would silently invert the selection instead of failing. Reject empty values at the boundary. +function validateSelectionPaths(paths: string[], exclude: string[]): void { + if ([...paths, ...exclude].some((path) => path === '')) { + throw new UsageError('Selection paths and --exclude values must not be empty') + } +} + async function captureChanges( revision: string | undefined, - { base, from, to, staged }: Pick, + { base, from, to, staged, exclude }: Pick, paths: string[], capturedAt: string, ): Promise { @@ -93,7 +118,7 @@ async function captureChanges( }) } else { const resolvedBase = base ?? 'HEAD' - const git = await captureGitChanges(resolvedBase, process.cwd(), { staged, paths }) + const git = await captureGitChanges(resolvedBase, process.cwd(), { staged, paths, exclude }) return createExplainCapture(git.files, { kind: 'working-tree', from: { revision: resolvedBase, commit: git.baseCommit }, diff --git a/test/cli.test.ts b/test/cli.test.ts index c04b868..803b6ed 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, test } from 'bun:test' import { createHash } from 'node:crypto' import { existsSync } from 'node:fs' -import { chmod, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' +import { chmod, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { captureIdFor } from '../src/authoring/capture' @@ -651,12 +651,14 @@ describe('inspect', () => { expect(check.stdout).toContain(`capture ${capture.captureId.slice(0, 12)}`) }) - test('rejects path limiting and --staged outside working-tree captures', async () => { + test('rejects path limiting, --exclude, and --staged outside working-tree captures', async () => { const repo = await committedFixtureRepo() for (const args of [ ['inspect', 'HEAD', '--', 'committed.ts'], ['inspect', '--from', 'HEAD^1', '--to', 'HEAD', '--', 'committed.ts'], + ['inspect', 'HEAD', '--exclude', 'committed.ts'], + ['inspect', '--from', 'HEAD^1', '--to', 'HEAD', '--exclude', 'committed.ts'], ['inspect', '--staged', 'HEAD'], ['inspect', '--staged', '--from', 'HEAD^1', '--to', 'HEAD'], ['inspect', 'HEAD', 'extra'], @@ -666,6 +668,198 @@ describe('inspect', () => { expect(result.stderr.length).toBeGreaterThan(0) } }) + + test('excludes named files and directories from a working-tree capture', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await writeFile(join(repo, 'experiments', 'exp.ts'), 'exp old\n') + await writeFile(join(repo, 'notes.md'), 'notes old\n') + await writeFile(join(repo, 'kept.ts'), 'kept old\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await writeFile(join(repo, 'experiments', 'exp.ts'), 'exp new\n') + await writeFile(join(repo, 'experiments', 'untracked.ts'), 'untracked\n') + await writeFile(join(repo, 'notes.md'), 'notes new\n') + await writeFile(join(repo, 'kept.ts'), 'kept new\n') + + const result = await runCli(['inspect', '--exclude', 'experiments', '--exclude', 'notes.md'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual(['kept.ts']) + }) + + test('combines --exclude with paths after -- and lets the exclusion win', async () => { + const repo = await fixtureRepo() + await mkdir(join(repo, 'src')) + await writeFile(join(repo, 'src', 'skip.ts'), 'skip\n') + await writeFile(join(repo, 'src', 'kept.ts'), 'kept\n') + + const combined = await runCli(['inspect', '--exclude', 'src/skip.ts', '--', 'src', 'greeting.ts'], repo) + expect(combined.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual(['greeting.ts', 'src/kept.ts']) + }) + + test('treats an exclusion with special characters as a literal path', async () => { + const repo = await fixtureRepo() + await writeFile(join(repo, 'notes[1].md'), 'notes\n') + await writeFile(join(repo, 'notesA.md'), 'other\n') + + const result = await runCli(['inspect', '--exclude', 'notes[1].md'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual([ + 'greeting.ts', + 'notesA.md', + 'untracked.ts', + ]) + }) + + test('rejects an empty selection instead of writing a walk', async () => { + const repo = await fixtureRepo() + + const excludedEverything = await runCli(['inspect', '--exclude', '.'], repo) + expect(excludedEverything.exitCode).not.toBe(0) + expect(excludedEverything.stderr).toContain('Nothing to capture') + + const noMatchingPath = await runCli(['inspect', '--', 'nonexistent.ts'], repo) + expect(noMatchingPath.exitCode).not.toBe(0) + expect(noMatchingPath.stderr).toContain('Nothing to capture') + + expect(existsSync(diffwalkDir(repo))).toBe(false) + }) + + test('rejects empty selection values instead of silently changing scope', async () => { + const repo = await fixtureRepo() + + for (const args of [ + ['inspect', '--exclude', ''], + ['inspect', '--exclude='], + ['inspect', '--', ''], + ]) { + const result = await runCli(args, repo) + expect(result.exitCode).not.toBe(0) + expect(result.stderr).toContain('must not be empty') + } + expect(existsSync(diffwalkDir(repo))).toBe(false) + }) + + test('excludes an untracked file by exact name', async () => { + const repo = await fixtureRepo() + + const result = await runCli(['inspect', '--exclude', 'untracked.ts'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual(['greeting.ts']) + }) + + test('lets an exact exclusion protect an unsupported file inside a selected path', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await symlink('one', join(repo, 'experiments', 'link')) + await writeFile(join(repo, 'experiments', 'other.ts'), 'other old\n') + await writeFile(join(repo, 'kept.ts'), 'kept old\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await rm(join(repo, 'experiments', 'link')) + await symlink('two', join(repo, 'experiments', 'link')) + await writeFile(join(repo, 'experiments', 'other.ts'), 'other new\n') + await writeFile(join(repo, 'kept.ts'), 'kept new\n') + + const result = await runCli( + ['inspect', '--exclude', 'experiments/link', '--', 'experiments', 'kept.ts'], + repo, + ) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual([ + 'experiments/other.ts', + 'kept.ts', + ]) + }) + + test('captures only the in-scope side of a rename crossing an excluded directory', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await writeFile(join(repo, 'moved.ts'), 'same\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await git(['mv', 'moved.ts', 'experiments/moved.ts'], repo) + + const result = await runCli(['inspect', '--exclude', 'experiments'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files).toEqual([ + { + path: 'moved.ts', + status: 'deleted', + oldMode: '100644', + newMode: '000000', + oldContent: 'same\n', + newContent: '', + }, + ]) + }) + + test('excludes an unsupported file so an unrelated capture can succeed', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await symlink('one', join(repo, 'experiments', 'link')) + await writeFile(join(repo, 'kept.ts'), 'kept old\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await rm(join(repo, 'experiments', 'link')) + await symlink('two', join(repo, 'experiments', 'link')) + await writeFile(join(repo, 'kept.ts'), 'kept new\n') + + const blocked = await runCli(['inspect'], repo) + expect(blocked.exitCode).not.toBe(0) + expect(blocked.stderr).toContain('Unsupported Git file type') + + const excluded = await runCli(['inspect', '--exclude', 'experiments'], repo) + expect(excluded.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual(['kept.ts']) + }) + + test('exposes --exclude in inspect help', async () => { + const repo = await fixtureRepo() + + const result = await runCli(['inspect', '--help'], repo) + + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('--exclude ') + expect(result.stdout).toContain('An exclusion always wins') + }) + + test('--exclude composes with --staged', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await writeFile(join(repo, 'first.ts'), 'first old\n') + await writeFile(join(repo, 'second.ts'), 'second old\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await writeFile(join(repo, 'first.ts'), 'first new\n') + await writeFile(join(repo, 'second.ts'), 'second new\n') + await git(['add', '.'], repo) + + const result = await runCli(['inspect', '--staged', '--exclude', 'first.ts'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files.map((file) => file.path)).toEqual(['second.ts']) + }) }) describe('walks', () => { diff --git a/test/git.test.ts b/test/git.test.ts index 6b0562c..785aac2 100644 --- a/test/git.test.ts +++ b/test/git.test.ts @@ -691,6 +691,194 @@ describe('captureGitChanges selection', () => { expect(capture.files).toEqual([]) }) + test('excludes named files and directories from a working-tree capture', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await writeFile(join(directory, 'experiments', 'inside.ts'), 'inside old\n') + await writeFile(join(directory, 'experiments.ts'), 'sibling old\n') + await writeFile(join(directory, 'notes.md'), 'notes old\n') + await writeFile(join(directory, 'kept.ts'), 'kept old\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await writeFile(join(directory, 'experiments', 'inside.ts'), 'inside new\n') + await writeFile(join(directory, 'experiments.ts'), 'sibling new\n') + await writeFile(join(directory, 'notes.md'), 'notes new\n') + await writeFile(join(directory, 'kept.ts'), 'kept new\n') + await writeFile(join(directory, 'experiments', 'untracked.ts'), 'untracked\n') + + const capture = await captureGitChanges('HEAD', directory, { + exclude: ['experiments', 'notes.md'], + }) + + // `experiments` excludes the tracked directory contents and its untracked file, but + // not the sibling `experiments.ts`: the match is literal, on path boundaries. + expect(capture.files.map((file) => file.path)).toEqual(['experiments.ts', 'kept.ts']) + }) + + test('applies exclusions after positive paths so an exclusion always wins', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'src', 'nested'), { recursive: true }) + await writeFile(join(directory, 'src', 'a.ts'), 'a old\n') + await writeFile(join(directory, 'src', 'skip.ts'), 'skip old\n') + await writeFile(join(directory, 'src', 'nested', 'deep.ts'), 'deep old\n') + await writeFile(join(directory, 'other.ts'), 'other old\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await writeFile(join(directory, 'src', 'a.ts'), 'a new\n') + await writeFile(join(directory, 'src', 'skip.ts'), 'skip new\n') + await writeFile(join(directory, 'src', 'nested', 'deep.ts'), 'deep new\n') + await writeFile(join(directory, 'other.ts'), 'other new\n') + + const capture = await captureGitChanges('HEAD', directory, { + paths: ['src', 'other.ts'], + exclude: ['src/skip.ts', 'src/nested'], + }) + + expect(capture.files.map((file) => file.path)).toEqual(['other.ts', 'src/a.ts']) + }) + + test('reports a rename into an excluded directory as a deletion of its in-scope side', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await writeFile(join(directory, 'experiments', 'kept.ts'), 'kept\n') + await writeFile(join(directory, 'moved.ts'), 'same\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'moved.ts', 'experiments/moved.ts'], directory) + + const capture = await captureGitChanges('HEAD', directory, { exclude: ['experiments'] }) + + expect(capture.files).toEqual([ + { + path: 'moved.ts', + status: 'deleted', + oldMode: '100644', + newMode: '000000', + oldContent: 'same\n', + newContent: '', + }, + ]) + }) + + test('reports a rename out of an excluded directory as an addition of its in-scope side', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await writeFile(join(directory, 'experiments', 'moved.ts'), 'same\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'experiments/moved.ts', 'back.ts'], directory) + + const capture = await captureGitChanges('HEAD', directory, { exclude: ['experiments'] }) + + expect(capture.files).toEqual([ + { + path: 'back.ts', + status: 'added', + oldMode: '000000', + newMode: '100644', + oldContent: '', + newContent: 'same\n', + }, + ]) + }) + + test('does not read or reject unsupported files that an exclusion omits', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await symlink('one', join(directory, 'experiments', 'link')) + await writeFile(join(directory, 'script.sh'), '#!/bin/sh\n') + await writeFile(join(directory, 'kept.ts'), 'kept old\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await unlink(join(directory, 'experiments', 'link')) + await symlink('two', join(directory, 'experiments', 'link')) + await chmod(join(directory, 'script.sh'), 0o755) + await writeFile(join(directory, 'kept.ts'), 'kept new\n') + + // Both the symlink (unsupported type) and the mode-only change are outside the + // requested scope, so they must not be read or validated. + const capture = await captureGitChanges('HEAD', directory, { + exclude: ['experiments', 'script.sh'], + }) + + expect(capture.files.map((file) => file.path)).toEqual(['kept.ts']) + await expect(captureGitChanges('HEAD', directory)).rejects.toThrow('Unsupported Git file type') + }) + + test('treats exclusion paths literally, including special characters', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + for (const name of ['star*file.ts', 'starXfile.ts', 'q?mark.ts', 'brack[et].ts', 'sp ace.ts']) { + await writeFile(join(directory, name), `${name} old\n`) + } + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + for (const name of ['star*file.ts', 'starXfile.ts', 'q?mark.ts', 'brack[et].ts', 'sp ace.ts']) { + await writeFile(join(directory, name), `${name} new\n`) + } + + const capture = await captureGitChanges('HEAD', directory, { + exclude: ['star*file.ts', 'q?mark.ts', 'brack[et].ts', 'sp ace.ts'], + }) + + // `star*file.ts` excludes only that literal name, so the glob-similar `starXfile.ts` stays. + expect(capture.files.map((file) => file.path)).toEqual(['starXfile.ts']) + }) + + test('applies an exclusion to a staged capture, including a spaced filename', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await writeFile(join(directory, 'sp ace.ts'), 'old\n') + await writeFile(join(directory, 'kept.ts'), 'old\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await writeFile(join(directory, 'sp ace.ts'), 'new\n') + await writeFile(join(directory, 'kept.ts'), 'new\n') + await git(['add', '.'], directory) + + const capture = await captureGitChanges('HEAD', directory, { + staged: true, + exclude: ['sp ace.ts'], + }) + + expect(capture.files.map((file) => file.path)).toEqual(['kept.ts']) + }) + + test('returns an empty capture when every change is excluded', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await writeFile(join(directory, 'tracked.ts'), 'old\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await writeFile(join(directory, 'tracked.ts'), 'new\n') + await writeFile(join(directory, 'untracked.ts'), 'untracked\n') + + const capture = await captureGitChanges('HEAD', directory, { exclude: ['.'] }) + + expect(capture.files).toEqual([]) + }) + test('captures a staged rename with both sides', async () => { const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) directories.push(directory) From 005e13be4afc27f45e1a102f1bc71172edae9719 Mon Sep 17 00:00:00 2001 From: Art Pai Date: Thu, 17 Sep 2026 07:31:53 +1000 Subject: [PATCH 2/2] Keep detected renames across inspect exclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply literal --exclude after Git detects renames and before Diffwalk reads or validates omitted sides, so a move crossing an exclusion keeps both paths and is reported as Moved to/from excluded path with the omitted side marked excluded rather than shown as empty. Exclusions are matched in Diffwalk instead of as Git pathspecs, so Git can still pair the two sides while Diffwalk never reads the excluded one; an excluded unsupported file no longer blocks the capture. Co-Authored-By: ことね --- docs/architecture.md | 16 ++- docs/usage.md | 12 ++- skills/diffwalk/SKILL.md | 13 +++ src/authoring/capture.ts | 22 +++- src/authoring/git.ts | 199 +++++++++++++++++++++++-------------- src/cli.ts | 2 +- src/cli/commands/change.ts | 10 +- src/cli/commands/file.ts | 10 ++ src/cli/output.ts | 30 +++++- src/format/schema.ts | 15 ++- src/format/status.ts | 26 +++++ src/format/types.ts | 17 +++- src/report/render.ts | 30 ++++-- test/authoring.test.ts | 94 ++++++++++++++++++ test/cli.test.ts | 89 ++++++++++++++++- test/git.test.ts | 119 ++++++++++++++++++++-- test/report.test.ts | 69 +++++++++++++ 17 files changed, 658 insertions(+), 115 deletions(-) create mode 100644 src/format/status.ts diff --git a/docs/architecture.md b/docs/architecture.md index 4b21299..bdddd83 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -115,13 +115,19 @@ exact corresponding diffs in a deliberate order. explanations, plus the version 1 ExplainDocument and its optional attribution metadata. - `src/authoring/git.ts`: captures staged, unstaged, deleted, renamed, and untracked text and binary files from an immutable Git base commit, optionally reading the index and - limiting the capture to literal selected paths or away from literal excluded paths, - with Git applying every exclusion before any file is read. A text side keeps its UTF-8 - content; a binary side keeps only its byte size and SHA-256 content hash. + limiting the capture to literal selected paths or away from literal excluded paths. + Positive paths narrow the diff before Git pairs renames; exclusions drop paths from the + result, before any file is read, so a detected rename crossing an exclusion keeps both + paths while only the included side is read. A text side keeps its UTF-8 content; a binary + side keeps only its byte size and SHA-256 content hash. Git itself may read excluded + content while detecting renames. +- `src/format/status.ts`: names the direction of a rename that crosses an exclusion and + labels the omitted side. - `src/authoring/capture.ts`: derives text change blocks, file-level binary change blocks, and the content `captureId`, and materializes exact section patches from capture plus - explanations. Text-only files hash into `captureId` exactly as before binary support, so - existing captures and their explanations stay matched. + explanations. A rename crossing an exclusion becomes a file-level move block that keeps + the included side and names the excluded one. Text-only files hash into `captureId` + exactly as before binary support, so existing captures and their explanations stay matched. - `src/cli/explanations.ts`: strict safe YAML 1.2 parsing into the explanations schema. - `src/cli/commands/`: each command owns its option schema and validates inputs before calling internal logic. `cli.ts` registers commands and forwards their arguments. diff --git a/docs/usage.md b/docs/usage.md index 83f26bf..8cfbae4 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -92,8 +92,16 @@ that exact file and never a Git pattern. Exclusions apply to working-tree captur (including untracked files) and to `--staged` captures, and are applied before Diffwalk reads or validates files, so an unsupported file outside the requested scope cannot block the capture. -When a rename crosses the selection, only its in-scope side is captured: moving a file -out of an excluded directory appears as an addition, and moving one in as a deletion. +Selection happens in two stages. Diffwalk hands Git the literal `--` paths first, and +Git detects renames among the changes that survive that scope. Diffwalk then drops +`--exclude` paths from the result. Git may read excluded file contents while it looks +for similarities, so exclusions cannot hide content from Git's rename detection, but +Diffwalk never reads or validates an omitted side. Rename detection is a heuristic, and +a rename that crosses an initial `--` path boundary may lose its relationship and appear +as an ordinary addition or deletion; only `--exclude` keeps detected moves. When a +detected rename crosses an `--exclude` boundary, the review keeps both paths and shows +`Moved to excluded path` or `Moved from excluded path`, omits the excluded side's content +explicitly rather than as an empty file, and drops a rename whose both sides are excluded. When the selection matches no changes, `inspect` stops with `Nothing to capture` instead of writing an empty walk. Say which paths you excluded when you share the review, since the review cannot show changes that were never captured. diff --git a/skills/diffwalk/SKILL.md b/skills/diffwalk/SKILL.md index 5f818b8..f9dccaf 100644 --- a/skills/diffwalk/SKILL.md +++ b/skills/diffwalk/SKILL.md @@ -25,6 +25,14 @@ explained. (the working tree or the index); commit and range captures reject it. When the selection matches no changes, `inspect` stops with `Nothing to capture`. Prefer `--exclude` over listing every included path. + - Selection runs in two stages: Git sees the literal `--` paths and detects renames + first, then Diffwalk drops `--exclude` paths. Git may read excluded content to detect + similarities, but Diffwalk never reads or validates the omitted side. A detected + rename crossing an exclusion keeps both paths and is reported as `Moved to excluded + path` or `Moved from excluded path`, with the excluded side marked `excluded` and no + content; explain that move instead of describing it as an addition or deletion. A + rename with both sides excluded is omitted. A rename crossing an initial `--` path + boundary can lose its relationship because Git filters before it detects renames. - Run `diffwalk inspect ` for one commit relative to its first parent. A root commit has no first parent, so use an explicit range instead. - Run `diffwalk inspect --from --to ` for a committed range. @@ -190,6 +198,11 @@ hide ownership or order; otherwise let Diffwalk's exact diff carry the code. captured metadata. Diffwalk does not generate binary patches or image previews, so explain a binary change from its identity rather than expecting its contents in the diff. Never reconstruct the missing bytes or paste them into `text`. +- A rename crossing an `--exclude` boundary is also a file-level card: it keeps both + paths and one side, reports `Moved to excluded path` or `Moved from excluded path`, and + marks the omitted side `excluded`. The omitted content is not in the capture, so treat + the card as the move it is and disclose the omission in your explanation instead of + treating it as a deleted or added file. - Treat a pure rename as a real assignable change. Diffwalk renders it as a move rather than an empty textual diff. - The capture contains full file contents. Treat it as potentially sensitive and do not publish or send it without the user's authorization. diff --git a/src/authoring/capture.ts b/src/authoring/capture.ts index e1dae10..9080d56 100644 --- a/src/authoring/capture.ts +++ b/src/authoring/capture.ts @@ -13,13 +13,14 @@ import type { Explanations, TextChangeBlock, } from '../format/types' +import { excludedSide, isMovedStatus } from '../format/status' export function createExplainCapture(files: DraftFile[], source: CaptureSource): ExplainCapture { let nextId = 1 const changes: ChangeBlock[] = [] for (const file of [...files].sort((left, right) => left.path.localeCompare(right.path))) { - if (file.oldBinary !== undefined || file.newBinary !== undefined) { + if (file.oldBinary !== undefined || file.newBinary !== undefined || isMovedStatus(file.status)) { changes.push(binaryChangeBlock(file, changeId(nextId++))) continue } @@ -65,6 +66,7 @@ function binaryChangeBlock(file: DraftFile, id: string): BinaryChangeBlock { path: file.path, status: file.status, ...(file.oldPath === undefined ? {} : { oldPath: file.oldPath }), + ...(file.excludedPath === undefined ? {} : { excludedPath: file.excludedPath }), oldMode: file.oldMode, newMode: file.newMode, ...(before === undefined ? {} : { before }), @@ -73,9 +75,14 @@ function binaryChangeBlock(file: DraftFile, id: string): BinaryChangeBlock { } // Every existing side has an identity, whether its bytes decode as text or stay binary, so a -// text-to-binary or binary-to-text transition keeps both sides instead of dropping one. +// text-to-binary or binary-to-text transition keeps both sides instead of dropping one. A +// side omitted by an exclusion is absent here too, but its status says why, so a renderer +// never mistakes it for an empty file. function changeSide(file: DraftFile, side: 'old' | 'new'): ChangeSide | undefined { - const absent = side === 'old' ? file.status === 'added' : file.status === 'deleted' + const absent = + side === 'old' + ? file.status === 'added' || excludedSide(file.status) === 'old' + : file.status === 'deleted' || excludedSide(file.status) === 'new' if (absent) return undefined const binary = side === 'old' ? file.oldBinary : file.newBinary @@ -98,6 +105,12 @@ export function captureIdFor(files: DraftFile[], includeModes = true): string { hash.update('\0') hash.update(file.oldPath ?? '') hash.update('\0') + // Only a rename crossing an exclusion has an excluded path, so files without one keep + // the capture ID they had before this field existed. + if (file.excludedPath !== undefined) { + hash.update(`excluded:${file.excludedPath}`) + hash.update('\0') + } if (includeModes) { hash.update(file.oldMode) hash.update('\0') @@ -216,13 +229,14 @@ function materializeStep( // confirms the captured sizes, hashes, status, and modes still describe it. function validateBinaryChange(change: BinaryChangeBlock, filesByPath: Map): void { const file = findFile(filesByPath, change.path) - if (file.oldBinary === undefined && file.newBinary === undefined) { + if (file.oldBinary === undefined && file.newBinary === undefined && !isMovedStatus(file.status)) { throw new Error(`Change block no longer matches captured file content: ${change.id}`) } const expected = binaryChangeBlock(file, change.id) if ( expected.status !== change.status || expected.oldPath !== change.oldPath || + expected.excludedPath !== change.excludedPath || expected.oldMode !== change.oldMode || expected.newMode !== change.newMode || JSON.stringify(expected.before) !== JSON.stringify(change.before) || diff --git a/src/authoring/git.ts b/src/authoring/git.ts index bff4a42..c1d81c8 100644 --- a/src/authoring/git.ts +++ b/src/authoring/git.ts @@ -35,8 +35,8 @@ interface GitChange { kind: string path: string oldPath?: string - oldMode: DraftFile['oldMode'] - newMode: DraftFile['newMode'] + oldMode: string + newMode: string } const supportedGitModes = new Set(['000000', '100644', '100755']) @@ -51,11 +51,13 @@ export async function captureGitChanges( const baseCommit = ( await gitText(['rev-parse', '--verify', '--end-of-options', `${base}^{commit}`], root) ).trim() - const pathspecs = selectionPathspecs(paths, exclude) - const pathEnvironment = pathspecs.length > 0 ? { GIT_LITERAL_PATHSPECS: '0' } : {} + // Positive paths narrow the diff before Git pairs renames. Exclusions stay out of the + // pathspec: Git must still see both sides of a rename so the move can be preserved, and + // the excluded side is dropped after detection, before Diffwalk reads any content. + const pathEnvironment = paths.length > 0 ? { GIT_LITERAL_PATHSPECS: '1' } : {} const changes = parseGitChanges( await gitBytes( - ['diff', '--raw', '-z', '--find-renames', ...(staged ? ['--cached'] : []), baseCommit, '--', ...pathspecs], + ['diff', '--raw', '-z', '--find-renames', ...(staged ? ['--cached'] : []), baseCommit, '--', ...paths], root, pathEnvironment, ), @@ -66,67 +68,21 @@ export async function captureGitChanges( const files: DraftFile[] = [] for (const change of changes) { - if (change.kind === 'R') { - files.push( - draftFile({ - path: change.path, - oldPath: change.oldPath, - status: 'renamed', - oldMode: change.oldMode, - newMode: change.newMode, - oldSide: await gitFile(baseCommit, change.oldPath!, root), - newSide: await newContentFor(change.path), - }), - ) - } else if (change.kind === 'M') { - const oldSide = await gitFile(baseCommit, change.path, root) - const newSide = await newContentFor(change.path) - validateFileModeChange(change.path, change.oldMode, change.newMode, oldSide, newSide) - files.push( - draftFile({ - path: change.path, - status: 'modified', - oldMode: change.oldMode, - newMode: change.newMode, - oldSide, - newSide, - }), - ) - } else if (change.kind === 'A') { - files.push( - draftFile({ - path: change.path, - status: 'added', - oldMode: change.oldMode, - newMode: change.newMode, - newSide: await newContentFor(change.path), - }), - ) - } else if (change.kind === 'D') { - files.push( - draftFile({ - path: change.path, - status: 'deleted', - oldMode: change.oldMode, - newMode: change.newMode, - oldSide: await gitFile(baseCommit, change.path, root), - }), - ) - } else { - throw new Error(`Unsupported Git change status: ${change.kind}`) - } + const file = await draftChange(change, { baseCommit, root, newContentFor, exclude }) + if (file !== undefined) files.push(file) } const filesByPath = new Map(files.map((file) => [file.path, file])) if (!staged) { const untracked = splitNulls( await gitBytes( - ['ls-files', '--others', '--exclude-standard', '--exclude=.diffwalk/', '-z', '--', ...pathspecs], + ['ls-files', '--others', '--exclude-standard', '--exclude=.diffwalk/', '-z', '--', ...paths], root, pathEnvironment, ), ) for (const path of untracked) { + if (isExcluded(path, exclude)) continue const existing = filesByPath.get(path) if (existing !== undefined) { if (existing.status !== 'deleted') continue @@ -172,6 +128,113 @@ export async function captureGitChanges( } } +async function draftChange( + change: GitChange, + context: { + baseCommit: string + root: string + newContentFor: (path: string) => Promise + exclude: string[] + }, +): Promise { + const { baseCommit, root, newContentFor, exclude } = context + if (change.kind === 'R') { + const oldExcluded = isExcluded(change.oldPath!, exclude) + const newExcluded = isExcluded(change.path, exclude) + if (oldExcluded && newExcluded) return undefined + const { oldMode, newMode } = supportedModes(change) + if (newExcluded) { + return draftFile({ + path: change.oldPath!, + excludedPath: change.path, + status: 'moved-to-excluded', + oldMode, + newMode, + oldSide: await gitFile(baseCommit, change.oldPath!, root), + }) + } + if (oldExcluded) { + return draftFile({ + path: change.path, + excludedPath: change.oldPath!, + status: 'moved-from-excluded', + oldMode, + newMode, + newSide: await newContentFor(change.path), + }) + } + return draftFile({ + path: change.path, + oldPath: change.oldPath, + status: 'renamed', + oldMode, + newMode, + oldSide: await gitFile(baseCommit, change.oldPath!, root), + newSide: await newContentFor(change.path), + }) + } + + // Excluded paths are dropped before any type check or read, so an unsupported file that + // the selection omits never blocks the capture. + if (isExcluded(change.path, exclude)) return undefined + const { oldMode, newMode } = supportedModes(change) + if (change.kind === 'M') { + const oldSide = await gitFile(baseCommit, change.path, root) + const newSide = await newContentFor(change.path) + validateFileModeChange(change.path, oldMode, newMode, oldSide, newSide) + return draftFile({ + path: change.path, + status: 'modified', + oldMode, + newMode, + oldSide, + newSide, + }) + } + if (change.kind === 'A') { + return draftFile({ + path: change.path, + status: 'added', + oldMode, + newMode, + newSide: await newContentFor(change.path), + }) + } + if (change.kind === 'D') { + return draftFile({ + path: change.path, + status: 'deleted', + oldMode, + newMode, + oldSide: await gitFile(baseCommit, change.path, root), + }) + } + throw new Error(`Unsupported Git change status: ${change.kind}`) +} + +// Git describes the file type in the mode field. Only regular files are supported; symbolic +// links and submodules are rejected here, after exclusions have had their say. +function supportedModes(change: GitChange): { oldMode: DraftFile['oldMode']; newMode: DraftFile['newMode'] } { + if (!supportedGitModes.has(change.oldMode) || !supportedGitModes.has(change.newMode)) { + throw new Error(`Unsupported Git file type: ${change.path}`) + } + return { + oldMode: change.oldMode as DraftFile['oldMode'], + newMode: change.newMode as DraftFile['newMode'], + } +} + +// Exclusions name literal files or directories relative to the repository root. A directory +// covers everything under it on path boundaries, and "." is the whole repository. Comparing +// strings directly keeps names with glob characters literal. +function isExcluded(path: string, exclude: string[]): boolean { + return exclude.some((candidate) => { + const name = candidate.startsWith('./') ? candidate.slice(2) : candidate + if (name === '' || name === '.') return true + return path === name || path.startsWith(name.endsWith('/') ? name : `${name}/`) + }) +} + export async function captureGitRevisionChanges( from: string, to: string, @@ -186,10 +249,11 @@ export async function captureGitRevisionChanges( const files: DraftFile[] = [] for (const change of changes) { + const { oldMode, newMode } = supportedModes(change) const oldSide = change.kind === 'A' ? undefined : await gitFile(fromCommit, change.oldPath ?? change.path, root) const newSide = change.kind === 'D' ? undefined : await gitFile(toCommit, change.path, root) if (change.kind === 'M') { - validateFileModeChange(change.path, change.oldMode, change.newMode, oldSide!, newSide!) + validateFileModeChange(change.path, oldMode, newMode, oldSide!, newSide!) } files.push( draftFile({ @@ -203,8 +267,8 @@ export async function captureGitRevisionChanges( : change.kind === 'A' ? 'added' : 'deleted', - oldMode: change.oldMode, - newMode: change.newMode, + oldMode, + newMode, oldSide, newSide, }), @@ -268,6 +332,7 @@ function validateFileModeChange( function draftFile(input: { path: string oldPath?: string + excludedPath?: string status: DraftFile['status'] oldMode: DraftFile['oldMode'] newMode: DraftFile['newMode'] @@ -277,6 +342,7 @@ function draftFile(input: { return { path: input.path, ...(input.oldPath === undefined ? {} : { oldPath: input.oldPath }), + ...(input.excludedPath === undefined ? {} : { excludedPath: input.excludedPath }), status: input.status, oldMode: input.oldMode, newMode: input.newMode, @@ -411,16 +477,6 @@ function splitNulls(bytes: Uint8Array): string[] { return value === '' ? [] : value.slice(0, value.endsWith('\0') ? -1 : undefined).split('\0') } -// Positive paths and exclusions are both literal, so special characters name a file rather -// than a Git pattern, and a directory matches everything it contains. Git applies every -// exclusion after the positive paths, so an exclusion always wins over a `--` path. -function selectionPathspecs(paths: string[], exclude: string[]): string[] { - return [ - ...paths.map((path) => `:(literal)${path}`), - ...exclude.map((path) => `:(exclude,literal)${path}`), - ] -} - function parseGitChanges(bytes: Uint8Array): GitChange[] { const fields = splitNulls(bytes) const changes: GitChange[] = [] @@ -434,15 +490,12 @@ function parseGitChanges(bytes: Uint8Array): GitChange[] { const firstPath = requireField(fields[index++], header) const oldPath = kind === 'R' ? firstPath : undefined const path = kind === 'R' ? requireField(fields[index++], header) : firstPath - if (!supportedGitModes.has(oldMode!) || !supportedGitModes.has(newMode!)) { - throw new Error(`Unsupported Git file type: ${path}`) - } changes.push({ kind: kind!, path, oldPath, - oldMode: oldMode as DraftFile['oldMode'], - newMode: newMode as DraftFile['newMode'], + oldMode: oldMode!, + newMode: newMode!, }) } diff --git a/src/cli.ts b/src/cli.ts index a7f8b4b..7985a7f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -45,7 +45,7 @@ function createCli(): Command { .option('--output ', 'Write capture to an explicit path') .option('--explanations ', 'Write or preserve authoring YAML at an explicit path') .allowExcessArguments(true) - .addHelpText('after', '\nLimit a working-tree capture with --staged, a `-- ...` list, or repeatable `--exclude `. An exclusion always wins over an included path.') + .addHelpText('after', '\nLimit a working-tree capture with --staged, a `-- ...` list, or repeatable `--exclude `. An exclusion always wins over an included path; a rename crossing an exclusion keeps both paths and omits the excluded side.') .action((_revision: string | undefined, options: Record, command: Command) => { const positionals = inspectPositionals(command) return inspectChanges( diff --git a/src/cli/commands/change.ts b/src/cli/commands/change.ts index a2d11f4..b4d724d 100644 --- a/src/cli/commands/change.ts +++ b/src/cli/commands/change.ts @@ -1,7 +1,8 @@ import { z } from 'zod' import type { BinaryChangeBlock, ChangeBlock, ExplainCapture, TextChangeBlock } from '../../format/types' +import { excludedSide } from '../../format/status' import { captureInput, readCapture } from '../input' -import { binarySideLine, coordinates } from '../output' +import { binarySideLine, changePathLabel, changeStatusLabel, coordinates } from '../output' const changeOptionsSchema = z.object({ input: z.string().optional(), @@ -33,10 +34,11 @@ function printChangeDetails(change: ChangeBlock): void { } function printBinaryChange(change: BinaryChangeBlock): void { - process.stdout.write(`${change.id} ${change.path} binary ${change.status} + const excluded = excludedSide(change.status) + process.stdout.write(`${change.id} ${changePathLabel(change)} ${changeStatusLabel(change.status)} modes: ${change.oldMode} → ${change.newMode} -${binarySideLine('before', change.before)} -${binarySideLine('after', change.after)} +${binarySideLine('before', change.before, excluded === 'old')} +${binarySideLine('after', change.after, excluded === 'new')} `) } diff --git a/src/cli/commands/file.ts b/src/cli/commands/file.ts index 21c3879..aa82fdb 100644 --- a/src/cli/commands/file.ts +++ b/src/cli/commands/file.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import type { BinarySide, DraftFile, ExplainCapture } from '../../format/types' +import { excludedSide } from '../../format/status' import { captureInput, readCapture } from '../input' import { UsageError } from '../usage' @@ -14,6 +15,7 @@ export async function printFile(filePath: string, options: z.input -
${escapeHtml(label)} Binary · ${escapeHtml(change.status)}
+
${escapeHtml(label)} ${escapeHtml(stats)}
- ${renderBinarySide('Before', change.before)} - ${renderBinarySide('After', change.after)} + ${renderBinarySide('Before', change.before, excluded === 'old')} + ${renderBinarySide('After', change.after, excluded === 'new')}
` } -function renderBinarySide(label: string, side: BinaryChangeBlock['before']): string { - const detail = - side === undefined ? 'absent' : `${side.kind} · ${side.size} B · sha256 ${side.hash}` +function renderBinarySide(label: string, side: BinaryChangeBlock['before'], excluded: boolean): string { + const detail = excluded + ? 'excluded' + : side === undefined + ? 'absent' + : `${side.kind} · ${side.size} B · sha256 ${side.hash}` return `
${label}
${escapeHtml(detail)}
` } diff --git a/test/authoring.test.ts b/test/authoring.test.ts index 1bd7da7..33bfcb2 100644 --- a/test/authoring.test.ts +++ b/test/authoring.test.ts @@ -640,3 +640,97 @@ describe('binary changes', () => { ) }) }) + +describe('cross-exclusion moves', () => { + const movedToExcluded: DraftFile = { + path: 'moved.ts', + excludedPath: 'experiments/moved.ts', + status: 'moved-to-excluded', + ...regularModes, + oldContent: 'same\n', + newContent: '', + } + + const movedFromExcluded: DraftFile = { + path: 'back.ts', + excludedPath: 'experiments/moved.ts', + status: 'moved-from-excluded', + ...regularModes, + oldContent: '', + newContent: 'same\n', + } + + test('keeps the included side and names the excluded one instead of showing it empty', () => { + const capture = createExplainCapture([movedToExcluded], source) + const change = capture.changes[0]! + + expect(change).toEqual({ + kind: 'binary', + id: 'change-001', + path: 'moved.ts', + status: 'moved-to-excluded', + excludedPath: 'experiments/moved.ts', + oldMode: '100644', + newMode: '100644', + before: { + kind: 'text', + size: 5, + hash: createHash('sha256').update('same\n').digest('hex'), + }, + }) + expect(change).not.toHaveProperty('after') + }) + + test('keeps the destination side when a rename arrives from an excluded path', () => { + const capture = createExplainCapture([movedFromExcluded], source) + const change = capture.changes[0]! + + expect(change).toEqual({ + kind: 'binary', + id: 'change-001', + path: 'back.ts', + status: 'moved-from-excluded', + excludedPath: 'experiments/moved.ts', + oldMode: '100644', + newMode: '100644', + after: { + kind: 'text', + size: 5, + hash: createHash('sha256').update('same\n').digest('hex'), + }, + }) + expect(change).not.toHaveProperty('before') + }) + + test('materializes the move block so the rendered review keeps both paths', () => { + const capture = createExplainCapture([movedToExcluded], source) + const document = materializeExplainDocument(capture, allChangesAssigned(capture)) + + expect(document.sections[0]!.steps[0]!.binary).toEqual([ + expect.objectContaining({ + status: 'moved-to-excluded', + path: 'moved.ts', + excludedPath: 'experiments/moved.ts', + }), + ]) + }) + + test('captureId distinguishes the omitted path from an ordinary deletion', () => { + const moved = createExplainCapture([movedToExcluded], source).captureId + const deleted = createExplainCapture( + [ + { + path: 'moved.ts', + status: 'deleted', + oldMode: '100644', + newMode: '000000', + oldContent: 'same\n', + newContent: '', + }, + ], + source, + ).captureId + + expect(moved).not.toBe(deleted) + }) +}) diff --git a/test/cli.test.ts b/test/cli.test.ts index 803b6ed..b0db103 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -783,7 +783,7 @@ describe('inspect', () => { ]) }) - test('captures only the in-scope side of a rename crossing an excluded directory', async () => { + test('preserves a rename into an excluded directory as a move with both paths', async () => { const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) directories.push(repo) await initializeRepository(repo) @@ -800,13 +800,96 @@ describe('inspect', () => { expect((await readCapture(repo)).files).toEqual([ { path: 'moved.ts', - status: 'deleted', + excludedPath: 'experiments/moved.ts', + status: 'moved-to-excluded', oldMode: '100644', - newMode: '000000', + newMode: '100644', oldContent: 'same\n', newContent: '', }, ]) + const changes = await runCli(['changes'], repo) + expect(changes.stdout).toContain('Moved to excluded path') + expect(changes.stdout).toContain('moved.ts → experiments/moved.ts') + expect(changes.stdout).toContain('after excluded') + }) + + test('preserves a staged rename out of an excluded directory as a move with both paths', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await writeFile(join(repo, 'experiments', 'moved.ts'), 'same\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await git(['mv', 'experiments/moved.ts', 'back.ts'], repo) + await git(['add', '-A'], repo) + + const result = await runCli(['inspect', '--staged', '--exclude', 'experiments'], repo) + + expect(result.exitCode).toBe(0) + expect((await readCapture(repo)).files).toEqual([ + { + path: 'back.ts', + excludedPath: 'experiments/moved.ts', + status: 'moved-from-excluded', + oldMode: '100644', + newMode: '100644', + oldContent: '', + newContent: 'same\n', + }, + ]) + const changes = await runCli(['changes'], repo) + expect(changes.stdout).toContain('Moved from excluded path') + expect(changes.stdout).toContain('experiments/moved.ts → back.ts') + expect(changes.stdout).toContain('before excluded') + }) + + test('renders a cross-exclusion move with both paths and an explicit omitted side', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await writeFile(join(repo, 'moved.ts'), 'same\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await git(['mv', 'moved.ts', 'experiments/moved.ts'], repo) + await runCli(['inspect', '--exclude', 'experiments'], repo) + await authorEveryChange(repo) + const output = join(repo, 'out', 'report.html') + + const result = await runCli(['export', 'html', '--output', output], repo) + + expect(result.exitCode).toBe(0) + const html = await readFile(output, 'utf8') + expect(html).toContain('Moved to excluded path') + expect(html).toContain('moved.ts → experiments/moved.ts') + expect(html).toContain('excluded content omitted') + expect(html).toContain('
excluded
') + expect(html).not.toContain('
absent
') + }) + + test('refuses to print the omitted side of a cross-exclusion move', async () => { + const repo = await mkdtemp(join(tmpdir(), 'diffwalk-cli-')) + directories.push(repo) + await initializeRepository(repo) + await mkdir(join(repo, 'experiments')) + await writeFile(join(repo, 'moved.ts'), 'same\n') + await git(['add', '.'], repo) + await git(['commit', '-q', '-m', 'fixture'], repo) + + await git(['mv', 'moved.ts', 'experiments/moved.ts'], repo) + await runCli(['inspect', '--exclude', 'experiments'], repo) + + const before = await runCli(['file', 'moved.ts', '--before'], repo) + expect(before.exitCode).toBe(0) + expect(before.stdout).toBe('same\n') + + const after = await runCli(['file', 'moved.ts', '--after'], repo) + expect(after.exitCode).not.toBe(0) + expect(after.stderr).toContain('excluded from this capture') }) test('excludes an unsupported file so an unrelated capture can succeed', async () => { diff --git a/test/git.test.ts b/test/git.test.ts index 785aac2..b99466d 100644 --- a/test/git.test.ts +++ b/test/git.test.ts @@ -743,7 +743,7 @@ describe('captureGitChanges selection', () => { expect(capture.files.map((file) => file.path)).toEqual(['other.ts', 'src/a.ts']) }) - test('reports a rename into an excluded directory as a deletion of its in-scope side', async () => { + test('preserves a rename into an excluded directory as a move with both paths', async () => { const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) directories.push(directory) await initializeRepository(directory) @@ -757,19 +757,22 @@ describe('captureGitChanges selection', () => { const capture = await captureGitChanges('HEAD', directory, { exclude: ['experiments'] }) + // Git detects the rename before the exclusion drops the destination, so the in-scope old + // side keeps the move instead of degrading to a deletion. expect(capture.files).toEqual([ { path: 'moved.ts', - status: 'deleted', + excludedPath: 'experiments/moved.ts', + status: 'moved-to-excluded', oldMode: '100644', - newMode: '000000', + newMode: '100644', oldContent: 'same\n', newContent: '', }, ]) }) - test('reports a rename out of an excluded directory as an addition of its in-scope side', async () => { + test('preserves a rename out of an excluded directory as a move with both paths', async () => { const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) directories.push(directory) await initializeRepository(directory) @@ -785,8 +788,9 @@ describe('captureGitChanges selection', () => { expect(capture.files).toEqual([ { path: 'back.ts', - status: 'added', - oldMode: '000000', + excludedPath: 'experiments/moved.ts', + status: 'moved-from-excluded', + oldMode: '100644', newMode: '100644', oldContent: '', newContent: 'same\n', @@ -794,6 +798,109 @@ describe('captureGitChanges selection', () => { ]) }) + test('omits a rename whose both sides are excluded', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await writeFile(join(directory, 'experiments', 'inside.ts'), 'inside\n') + await writeFile(join(directory, 'kept.ts'), 'kept\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'experiments/inside.ts', 'experiments/renamed.ts'], directory) + await writeFile(join(directory, 'kept.ts'), 'kept changed\n') + + const capture = await captureGitChanges('HEAD', directory, { exclude: ['experiments'] }) + + // The rename lives entirely inside the excluded directory, so the whole move is omitted + // and only the unrelated modification remains. + expect(capture.files.map((file) => file.path)).toEqual(['kept.ts']) + }) + + test('keeps a rename with neither side excluded as a normal rename', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await writeFile(join(directory, 'kept-old.ts'), 'kept\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'kept-old.ts', 'kept-new.ts'], directory) + + const capture = await captureGitChanges('HEAD', directory, { exclude: ['experiments'] }) + + expect(capture.files).toEqual([ + { + path: 'kept-new.ts', + oldPath: 'kept-old.ts', + status: 'renamed', + oldMode: '100644', + newMode: '100644', + oldContent: 'kept\n', + newContent: 'kept\n', + }, + ]) + }) + + test('preserves a staged rename crossing an exclusion with only the index side read', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await mkdir(join(directory, 'experiments')) + await writeFile(join(directory, 'moved.ts'), 'same\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'moved.ts', 'experiments/moved.ts'], directory) + await git(['add', '-A'], directory) + + const capture = await captureGitChanges('HEAD', directory, { + staged: true, + exclude: ['experiments'], + }) + + expect(capture.files).toEqual([ + { + path: 'moved.ts', + excludedPath: 'experiments/moved.ts', + status: 'moved-to-excluded', + oldMode: '100644', + newMode: '100644', + oldContent: 'same\n', + newContent: '', + }, + ]) + }) + + test('loses a rename that crosses the initial positive path scope', async () => { + const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) + directories.push(directory) + await initializeRepository(directory) + await writeFile(join(directory, 'old.ts'), 'same\n') + await writeFile(join(directory, 'other.ts'), 'other\n') + await git(['add', '.'], directory) + await git(['commit', '-q', '-m', 'fixture'], directory) + + await git(['mv', 'old.ts', 'new.ts'], directory) + + const capture = await captureGitChanges('HEAD', directory, { paths: ['old.ts'] }) + + // Git resolves the positive pathspec before it pairs renames, so the destination outside + // the scope leaves the in-scope source looking like a deletion. Only `--exclude` keeps + // detected moves; this limitation is documented. + expect(capture.files).toEqual([ + { + path: 'old.ts', + status: 'deleted', + oldMode: '100644', + newMode: '000000', + oldContent: 'same\n', + newContent: '', + }, + ]) + }) + test('does not read or reject unsupported files that an exclusion omits', async () => { const directory = await mkdtemp(join(tmpdir(), 'diffwalk-git-')) directories.push(directory) diff --git a/test/report.test.ts b/test/report.test.ts index d63bf07..2515c7f 100644 --- a/test/report.test.ts +++ b/test/report.test.ts @@ -900,4 +900,73 @@ describe('binary change cards', () => { expect(html).not.toContain('') expect(html).toContain('assets/<img src=x>.png') }) + + test('shows a move to an excluded path with both paths and an explicit omitted side', () => { + const html = renderReport( + document([ + { + title: 'Move', + steps: [ + { + text: 'Moved out of scope.', + binary: [ + { + kind: 'binary', + id: 'change-001', + path: 'moved.ts', + status: 'moved-to-excluded', + excludedPath: 'experiments/moved.ts', + oldMode: '100644', + newMode: '100644', + before: { kind: 'text', size: 5, hash: 'a'.repeat(64) }, + }, + ], + }, + ], + }, + ]), + stubClient, + ) + + expect(html).toContain('moved.ts → experiments/moved.ts') + expect(html).toContain('Moved to excluded path') + expect(html).toContain('excluded content omitted') + expect(html).toContain('
excluded
') + expect(html).not.toContain('
absent
') + expect(html).not.toContain('data-diff-mount') + }) + + test('shows a move from an excluded path in reading order with both paths', () => { + const html = renderReport( + document([ + { + title: 'Move', + steps: [ + { + text: 'Moved into scope.', + binary: [ + { + kind: 'binary', + id: 'change-001', + path: 'back.ts', + status: 'moved-from-excluded', + excludedPath: 'experiments/moved.ts', + oldMode: '100644', + newMode: '100644', + after: { kind: 'text', size: 5, hash: 'b'.repeat(64) }, + }, + ], + }, + ], + }, + ]), + stubClient, + ) + + expect(html).toContain('experiments/moved.ts → back.ts') + expect(html).toContain('Moved from excluded path') + expect(html).toContain('excluded content omitted') + expect(html).toContain('
excluded
') + expect(html).not.toContain('
absent
') + }) })