diff --git a/CHANGELOG.md b/CHANGELOG.md index aea140d..08dc1b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ format and uses semantic versioning when versioned releases are published. ## [Unreleased] +### Fixed + +- `plan` now resolves the repository root via `git rev-parse --show-toplevel` + and runs every read-only git command from it, so invoking the CLI from any + subdirectory produces the same root-relative plan as the repository root + (previously untracked files outside the current directory were dropped and + listed with cwd-relative paths). +- Outside a git repository, `atomcommit plan` prints a single + `atomcommit: not a git repository` stderr line and exits 1 instead of + surfacing raw `git diff` usage output and an uncaught Node.js stack trace. + ### Added - Release-candidate metadata, package allowlist, and npm pack smoke coverage. diff --git a/README.md b/README.md index 780b59c..6b80108 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ npm install -g atomcommit ## Quickstart -From a checkout, generate an atomic commit plan for the current repository: +From a checkout, generate an atomic commit plan for the current repository (from the repository root or any subdirectory): ```sh node src/index.js plan @@ -52,14 +52,17 @@ atomcommit --help `atomcommit` shells out only to these read-only Git commands: -- `git diff --name-status` -- `git diff --cached --name-status` -- `git diff --numstat` +- `git rev-parse --show-toplevel` +- `git diff --name-status -z` +- `git diff --cached --name-status -z` +- `git diff --numstat -z` +- `git diff --cached --numstat -z` - `git diff --stat` +- `git diff --cached --stat` - `git ls-files --others --exclude-standard -z` - `git diff --no-index --numstat -- /dev/null ` -The `git ls-files` query includes ordinary untracked files while respecting Git ignore rules. NUL-delimited paths preserve spaces and other special characters. The CLI remains read-only: it does not stage files, alter the index, or modify the working tree. +The first command resolves the repository root, and every later command runs from that root, so invoking `atomcommit` in any subdirectory produces the same root-relative plan as invoking it at the repository root. Outside a git repository the CLI prints `atomcommit: not a git repository` to stderr and exits `1`. The `git ls-files` query includes ordinary untracked files while respecting Git ignore rules. NUL-delimited paths preserve spaces and other special characters. The CLI remains read-only: it does not stage files, alter the index, or modify the working tree. JSON output preserves path values exactly. In Markdown output, paths are displayed as JSON string literals inside inline code spans. Control characters therefore appear as escapes such as `\\n` and `\\t`, and the code-span fence automatically expands when a filename contains backticks, keeping every path on one unambiguous list item. diff --git a/scripts/smoke.sh b/scripts/smoke.sh index 8b02457..8be4776 100755 --- a/scripts/smoke.sh +++ b/scripts/smoke.sh @@ -27,4 +27,29 @@ if (!plan.commits.some((commit) => commit.riskFlags.includes("deletion"))) throw grep -q '^# Atomic Commit Plan' "$tmp_dir/plan.md" grep -q 'Suggested commit message:' "$tmp_dir/plan.md" -printf 'Smoke passed: fixture plan generated in Markdown and JSON.\n' +# Subdirectory invocations must produce the identical root-relative plan. +cd "$fixture_repo/docs" +node "$repo_root/src/index.js" plan --json > "$tmp_dir/plan-sub.json" +node -e ' +const fs = require("node:fs"); +const [root, sub] = process.argv.slice(1).map((f) => fs.readFileSync(f, "utf8")); +if (root !== sub) throw new Error("plan differs when generated from a subdirectory"); +' "$tmp_dir/plan.json" "$tmp_dir/plan-sub.json" + +# Outside a git repository the CLI must fail with one concise stderr line. +nongit_dir="$tmp_dir/not-a-repo" +mkdir -p "$nongit_dir" +set +e +nongit_stderr="$(cd "$nongit_dir" && node "$repo_root/src/index.js" plan 2>&1 >/dev/null)" +nongit_status=$? +set -e +if [ "$nongit_status" -ne 1 ]; then + echo "expected exit 1 outside a git repository, got $nongit_status" >&2 + exit 1 +fi +if [ "$nongit_stderr" != "atomcommit: not a git repository" ]; then + echo "expected one concise stderr line outside a git repository, got: $nongit_stderr" >&2 + exit 1 +fi + +printf 'Smoke passed: fixture plan generated in Markdown and JSON, subdirectory-stable, and non-repository error concise.\n' diff --git a/src/index.js b/src/index.js index 88c234e..2927d36 100755 --- a/src/index.js +++ b/src/index.js @@ -216,10 +216,27 @@ export function renderMarkdown(plan) { return `${lines.join('\n').trimEnd()}\n`; } +export function resolveRepositoryRoot(cwd = process.cwd()) { + const result = spawnSync('git', ['rev-parse', '--show-toplevel'], { cwd, encoding: 'utf8' }); + if (result.status !== 0) { + return null; + } + return result.stdout.trim(); +} + +// Returns null when cwd is not inside a git repository; callers decide how to +// report it so the CLI can stay free of raw git usage dumps and stack traces. export function collectGitDiff(cwd = process.cwd()) { - const unstaged = parseNameStatus(runGit(['diff', '--name-status', '-z'], cwd), 'unstaged'); - const staged = parseNameStatus(runGit(['diff', '--cached', '--name-status', '-z'], cwd), 'staged'); - const untrackedPaths = parseUntrackedPaths(runGit(['ls-files', '--others', '--exclude-standard', '-z'], cwd)); + // Pin every git invocation to the repository root so diff enumeration, + // ls-files untracked discovery, and per-file stats share one root-relative + // path space no matter which subdirectory the CLI was invoked from. + const root = resolveRepositoryRoot(cwd); + if (root === null || root === '') { + return null; + } + const unstaged = parseNameStatus(runGit(['diff', '--name-status', '-z'], root), 'unstaged'); + const staged = parseNameStatus(runGit(['diff', '--cached', '--name-status', '-z'], root), 'staged'); + const untrackedPaths = parseUntrackedPaths(runGit(['ls-files', '--others', '--exclude-standard', '-z'], root)); const untracked = untrackedPaths.map((path) => ({ path, previousPath: null, @@ -229,16 +246,16 @@ export function collectGitDiff(cwd = process.cwd()) { score: null, source: 'untracked', })); - const untrackedStats = new Map(untrackedPaths.map((path) => [path, untrackedStat(path, cwd)])); + const untrackedStats = new Map(untrackedPaths.map((path) => [path, untrackedStat(path, root)])); const changes = mergeChanges([...staged, ...unstaged, ...untracked]); const stats = mergeStats([ - parseNumstat(runGit(['diff', '--cached', '--numstat', '-z'], cwd)), - parseNumstat(runGit(['diff', '--numstat', '-z'], cwd)), + parseNumstat(runGit(['diff', '--cached', '--numstat', '-z'], root)), + parseNumstat(runGit(['diff', '--numstat', '-z'], root)), untrackedStats, ]); const diffStat = mergeDiffStats([ - parseDiffStat(runGit(['diff', '--cached', '--stat'], cwd)), - parseDiffStat(runGit(['diff', '--stat'], cwd)), + parseDiffStat(runGit(['diff', '--cached', '--stat'], root)), + parseDiffStat(runGit(['diff', '--stat'], root)), untrackedDiffStat(untrackedStats), ], changes.length); @@ -370,7 +387,7 @@ function runGit(args, cwd, allowedStatuses = [0]) { } function printHelp() { - console.log(`Usage: atomcommit [plan] [--json]\n\nCommands:\n plan Analyze local tracked and untracked changes and print an atomic commit plan.\n\nDefault:\n atomcommit is equivalent to atomcommit plan.\n\nOptions:\n --json Print machine-readable JSON instead of Markdown.\n -h, --help Show this help.\n -v, --version Print the CLI version.\n\nSafety:\n atomcommit only runs read-only git diff and git ls-files commands and never stages, commits, or modifies files.`); + console.log(`Usage: atomcommit [plan] [--json]\n\nCommands:\n plan Analyze local tracked and untracked changes and print an atomic commit plan.\n\nDefault:\n atomcommit is equivalent to atomcommit plan.\n\nOptions:\n --json Print machine-readable JSON instead of Markdown.\n -h, --help Show this help.\n -v, --version Print the CLI version.\n\nSafety:\n atomcommit only runs read-only Git commands (rev-parse, diff, ls-files) from the repository root and never stages, commits, or modifies files. Outside a git repository it prints 'atomcommit: not a git repository' and exits 1.`); } export function main(argv = process.argv.slice(2), cwd = process.cwd()) { @@ -400,6 +417,11 @@ export function main(argv = process.argv.slice(2), cwd = process.cwd()) { } const plan = collectGitDiff(cwd); + if (plan === null) { + console.error('atomcommit: not a git repository'); + return 1; + } + if (options.includes('--json')) { console.log(JSON.stringify(plan, null, 2)); } else { diff --git a/test/cli.test.js b/test/cli.test.js index 0c0f9b6..d1258c7 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -1,10 +1,76 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { execFileSync, execSync } from 'node:child_process'; +import { execFileSync, execSync, spawnSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { pathToFileURL } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const cliPath = fileURLToPath(new URL('../src/index.js', import.meta.url)); + +function runCliJson(args, cwd) { + const result = spawnSync(process.execPath, [cliPath, ...args], { cwd, encoding: 'utf8' }); + assert.equal(result.status, 0, `atomcommit ${args.join(' ')} failed: ${result.stderr}`); + return JSON.parse(result.stdout); +} + +function planFilePaths(plan) { + return plan.commits.flatMap((commit) => commit.files.map((file) => file.path)).sort(); +} + +function initScratchRepo(parent, name = 'repo') { + const repo = join(parent, name); + mkdirSync(repo, { recursive: true }); + execFileSync('git', ['init', '--initial-branch', 'main'], { cwd: repo, stdio: 'pipe' }); + execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: repo }); + execFileSync('git', ['config', 'user.name', 'Test User'], { cwd: repo }); + writeFileSync(join(repo, 'tracked.txt'), 'one\n'); + execFileSync('git', ['add', 'tracked.txt'], { cwd: repo }); + execFileSync('git', ['commit', '-m', 'initial'], { cwd: repo, stdio: 'pipe' }); + return repo; +} + +test('plan invoked from a subdirectory matches the root-relative plan from the repository root', (t) => { + const parent = mkdtempSync(join(tmpdir(), 'atomcommit-subdir-')); + t.after(() => rmSync(parent, { recursive: true, force: true })); + + const repo = initScratchRepo(parent); + mkdirSync(join(repo, 'sub')); + writeFileSync(join(repo, 'notes-at-root.txt'), 'untracked at repository root\n'); + writeFileSync(join(repo, 'sub', 'file.txt'), 'untracked in subdirectory\n'); + + const fromRoot = runCliJson(['plan', '--json'], repo); + const fromSub = runCliJson(['plan', '--json'], join(repo, 'sub')); + + assert.deepEqual(planFilePaths(fromSub), planFilePaths(fromRoot)); + assert.deepEqual( + planFilePaths(fromRoot), + ['notes-at-root.txt', 'sub/file.txt'].sort(), + 'root scan must list both untracked files with root-relative paths', + ); + assert.equal( + fromSub.summary.filesChanged, + fromRoot.summary.filesChanged, + 'subdirectory invocation must not drop or duplicate files', + ); + + const subFile = fromSub.commits.flatMap((commit) => commit.files).find((file) => file.path === 'sub/file.txt'); + assert.ok(subFile, 'plan from subdirectory still reports sub/file.txt root-relative'); + assert.equal(subFile.stats.added, 1); +}); + +test('plan outside a git repository exits 1 with one concise stderr line and no stack trace', (t) => { + const dir = mkdtempSync(join(tmpdir(), 'atomcommit-nongit-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + + const result = spawnSync(process.execPath, [cliPath, 'plan'], { cwd: dir, encoding: 'utf8' }); + + assert.equal(result.status, 1); + assert.equal(result.stdout, ''); + assert.equal(result.stderr.trim(), 'atomcommit: not a git repository'); + assert.equal(result.stderr.trim().split('\n').length, 1, 'stderr must be a single concise line'); + assert.doesNotMatch(result.stderr, /Error|usage|fatal|index\.js/, 'stderr must not embed git usage output or a stack trace'); +}); test('atomcommit plan test - CLI should handle --help', () => { try {