From 22260ddb4dfdea054226843757d8b427f8ea6447 Mon Sep 17 00:00:00 2001 From: jecanore Date: Sat, 25 Jul 2026 00:23:26 -0500 Subject: [PATCH] feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn (#2640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn Adds a new `--self-commit` flag to `gitnexus analyze`. When passed, any AGENTS.md/CLAUDE.md changes the run makes (including first-time creation) are auto-committed, scoped to only those two files (never `git add -A`). No-ops silently if neither exists, neither changed, or the repo has no git identity configured — never fails the surrounding analyze run. Complements #1478 (--no-stats): that flag removes the volatile counts entirely, this one keeps them but eliminates the dangling working-tree diff they otherwise leave behind on every run. Closes #2639. * fix(analyze): log a warning when --self-commit fails to commit Addresses review feedback on #2640: the commit step's catch block was silently swallowing failures (e.g. missing git identity) with no signal to the user. Logs via the existing pino logger (matching the rest of the codebase's convention) with the error and the file list, while still never throwing — analyze must not fail over this. New test forces a real commit failure (missing identity, with useConfigOnly + isolated HOME/XDG_CONFIG_HOME/GIT_CONFIG_NOSYSTEM so no ambient global git config on the CI runner can mask it) and asserts the warning is captured via logger's _captureLogger test hook. * fix(analyze): refuse to sweep pre-existing edits into --self-commit Addresses both state-safety blockers from review round 2 on #2640: 1. selfCommitContextFiles could not distinguish a pre-existing unstaged user edit in AGENTS.md/CLAUDE.md from this run's generated stats refresh — both just showed up as "the file is dirty" — so a user edit sitting in either file got silently swept into the generated commit. Fixed by snapshotting each candidate's cleanliness via the new snapshotSelfCommitSafety() BEFORE analyze writes to it; only files confirmed safe (nonexistent pre-run, i.e. first-time creation, or clean pre-run) are ever added/committed. A file already dirty pre-run is skipped and logged, never touched. 2. On a failed `git commit` (e.g. missing identity), the preceding `git add` had already staged the safe files, and analyze reported nothing happened while silently leaving them staged. Fixed with a `git reset -- ` in the commit-failure catch, restoring the index to its pre-add state for exactly the files this helper staged. Wired analyze.ts to call snapshotSelfCommitSafety() once before runFullAnalysis (which is where the actual AGENTS.md/CLAUDE.md write happens, on both the fast path and the primary run), threading the result through both existing selfCommitContextFiles() call sites. New tests: a pre-dirty AGENTS.md is skipped while a clean CLAUDE.md still commits normally, and a post-add commit failure leaves nothing staged. Updated all existing selfCommitContextFiles() call sites for the new required safety-map parameter. * i18n(cli): add zh-CN translation for --self-commit help text Addresses magyargergo's follow-up on #2640: --self-commit was missing from the analyze command's OPTION_DESCRIPTION_KEYS map, so its help text never went through localizeCliHelp and always rendered in English regardless of locale. Adds the help.option.analyze.selfCommit key to both en.ts and zh-CN.ts and wires it into help-i18n.ts, matching the existing --no-stats/--skills entries. --------- Co-authored-by: Gergő Magyar --- gitnexus/src/cli/analyze.ts | 37 +- gitnexus/src/cli/help-i18n.ts | 1 + gitnexus/src/cli/i18n/en.ts | 2 + gitnexus/src/cli/i18n/zh-CN.ts | 2 + gitnexus/src/cli/index.ts | 6 + gitnexus/src/storage/git.ts | 120 ++++++- .../unit/analyze-self-commit-bridge.test.ts | 203 +++++++++++ gitnexus/test/unit/git-utils.test.ts | 330 ++++++++++++++++++ 8 files changed, 699 insertions(+), 2 deletions(-) create mode 100644 gitnexus/test/unit/analyze-self-commit-bridge.test.ts diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index d4b42cf746..b9a50a720b 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -33,7 +33,13 @@ import { assertAnalysisFinalized, type AnalyzerRunnerIdentity, } from '../storage/repo-manager.js'; -import { getGitRoot, hasGitDir, getDefaultBranch } from '../storage/git.js'; +import { + getGitRoot, + hasGitDir, + getDefaultBranch, + selfCommitContextFiles, + snapshotSelfCommitSafety, +} from '../storage/git.js'; import { IndexLockTimeoutError } from '../storage/index-lock.js'; import { loadAnalyzeConfig, @@ -649,6 +655,13 @@ export interface AnalyzeOptions { * default-on case. */ stats?: boolean; + /** + * Opt-in auto-commit of any AGENTS.md/CLAUDE.md changes this `analyze` run + * makes. Scoped to only those two files (never `git add -A`); no-ops + * silently if neither exists, neither changed, or the commit step itself + * fails (e.g. no git identity configured). See #2639. + */ + selfCommit?: boolean; /** Skip installing standard GitNexus skill files directly under .claude/skills/. */ skipSkills?: boolean; /** @@ -1395,6 +1408,15 @@ const analyzeCommandImpl = async ( const bootstrapArgs: [] | [AnalyzerRunnerIdentity] = runnerIdentityAtBootstrap ? [runnerIdentityAtBootstrap] : []; + // #2639 review round 2: snapshot which of AGENTS.md/CLAUDE.md are safe to + // auto-commit BEFORE runFullAnalysis (and the --skills regeneration + // further down) writes to them, so selfCommitContextFiles can tell a + // pre-existing unstaged user edit apart from this run's stats refresh + // and refuse to sweep the former into the latter's commit. + const selfCommitSafety = + options.selfCommit === true + ? snapshotSelfCommitSafety(repoPath, ['AGENTS.md', 'CLAUDE.md']) + : undefined; const result = await runFullAnalysis(repoPath, runOptions, runCallbacks, ...bootstrapArgs); if (result.alreadyUpToDate) { @@ -1439,6 +1461,11 @@ const analyzeCommandImpl = async ( ` Updated base_ref to "${resolvedDefaultBranch}" in ${baseRefRefreshed.join(', ')}\n`, ); } + // #2639: opt-in self-commit of any AGENTS.md/CLAUDE.md churn from this + // fast path (e.g. a base_ref refresh above). Best-effort — never throws. + if (options.selfCommit === true && selfCommitSafety) { + selfCommitContextFiles(repoPath, ['AGENTS.md', 'CLAUDE.md'], selfCommitSafety); + } // Safe to return without process.exit(0) — the early-return path in // runFullAnalysis never opens LadybugDB, so no native handles prevent exit. return; @@ -1528,6 +1555,14 @@ const analyzeCommandImpl = async ( } } + // #2639: opt-in self-commit of any AGENTS.md/CLAUDE.md churn written by + // this run (the primary generateAIContextFiles call inside + // runFullAnalysis, and/or the --skills regeneration above). Best-effort + // — never throws, so a missing git identity etc. can't fail `analyze`. + if (options.selfCommit === true && selfCommitSafety) { + selfCommitContextFiles(repoPath, ['AGENTS.md', 'CLAUDE.md'], selfCommitSafety); + } + const totalTime = ((Date.now() - t0) / 1000).toFixed(1); clearInterval(elapsedTimer); diff --git a/gitnexus/src/cli/help-i18n.ts b/gitnexus/src/cli/help-i18n.ts index ff110615b0..58f28d11ac 100644 --- a/gitnexus/src/cli/help-i18n.ts +++ b/gitnexus/src/cli/help-i18n.ts @@ -57,6 +57,7 @@ const OPTION_DESCRIPTION_KEYS = { 'analyze|--skills': 'help.option.analyze.skills', 'analyze|--skip-agents-md': 'help.option.analyze.skipAgentsMd', 'analyze|--no-stats': 'help.option.analyze.noStats', + 'analyze|--self-commit': 'help.option.analyze.selfCommit', 'analyze|--skip-skills': 'help.option.analyze.skipSkills', 'analyze|--index-only': 'help.option.analyze.indexOnly', 'analyze|--skip-git': 'help.option.skipGit', diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index bbec28e2c7..37811cc9e6 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -184,6 +184,8 @@ export const en = { 'help.option.analyze.skipAgentsMd': 'Skip updating the gitnexus section in AGENTS.md and CLAUDE.md', 'help.option.analyze.noStats': 'Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md', + 'help.option.analyze.selfCommit': + 'Auto-commit AGENTS.md/CLAUDE.md changes after analyze (opt-in, off by default). Scoped to only those two files (never `git add -A`); no-ops if neither exists, neither changed, or the repo has no git identity configured.', 'help.option.analyze.skipSkills': 'Skip installing standard GitNexus skill files directly under .claude/skills/ and .agents/skills/. Does not suppress community skills from --skills (those use .claude/skills/gitnexus-area-*). Use --index-only to skip all AI-context file injection.', 'help.option.analyze.indexOnly': diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 7506b8d4b4..d411764074 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -175,6 +175,8 @@ export const zhCN = { '根据检测到的社区生成仓库专属 skill 文件（同时设置 --index-only 时无效）。', 'help.option.analyze.skipAgentsMd': '跳过更新 AGENTS.md 和 CLAUDE.md 中的 gitnexus 区块', 'help.option.analyze.noStats': '从 AGENTS.md 和 CLAUDE.md 中省略易变的文件/符号计数', + 'help.option.analyze.selfCommit': + '在 analyze 后自动提交 AGENTS.md/CLAUDE.md 的变更（默认关闭，需显式开启）。仅限这两个文件（绝不使用 `git add -A`）；若两者均不存在、均未变更，或仓库未配置 git 身份，则不执行任何操作。', 'help.option.analyze.skipSkills': '跳过直接安装在 .claude/skills/ 和 .agents/skills/ 下的标准 GitNexus skill 文件。不抑制 --skills 生成的社区 skill（位于 .claude/skills/gitnexus-area-*）。使用 --index-only 可跳过所有 AI 上下文文件注入。', 'help.option.analyze.indexOnly': '纯索引模式：跳过所有文件注入（AGENTS.md、CLAUDE.md、skills）', diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 9526040025..0ba1c55480 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -92,6 +92,12 @@ program 'checked-out working tree. Distinct from --default-branch (cosmetic base_ref).', ) .option('--no-stats', 'Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md') + .option( + '--self-commit', + 'Auto-commit AGENTS.md/CLAUDE.md changes after analyze (opt-in, off by default). ' + + 'Scoped to only those two files (never `git add -A`); no-ops if neither exists, ' + + 'neither changed, or the repo has no git identity configured.', + ) .option( '--skip-skills', 'Skip installing standard GitNexus skill files directly under .claude/skills/ and .agents/skills/. ' + diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index df4bbaa1c5..585322b116 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -1,7 +1,8 @@ import { execFileSync, execSync } from 'child_process'; -import { statSync } from 'fs'; +import { statSync, existsSync } from 'fs'; import path from 'path'; import os from 'os'; +import { logger } from '../core/logger.js'; // Git utilities for repository detection, commit tracking, and diff analysis @@ -51,6 +52,123 @@ export const isWorkingTreeDirty = (repoPath: string): boolean => { } }; +/** + * Snapshot, per candidate file, whether it is safe for `selfCommitContextFiles` + * to auto-commit — call this BEFORE `analyze` writes AGENTS.md/CLAUDE.md. + * A file is safe when it does not exist yet (first-time creation, the normal + * case) or is currently clean (`git status --porcelain` reports nothing for + * it). A file that already has an uncommitted user edit is unsafe: without + * this check `selfCommitContextFiles` cannot tell that edit apart from the + * stats refresh `analyze` is about to write, and would silently sweep both + * into one generated-looking commit. Fails closed — a git failure marks the + * file unsafe rather than assuming it's clean. See #2639 review round 2. + */ +export const snapshotSelfCommitSafety = ( + repoPath: string, + candidateFiles: string[], +): Map => { + const safety = new Map(); + for (const name of candidateFiles) { + if (!existsSync(path.join(repoPath, name))) { + safety.set(name, true); + continue; + } + try { + const status = execFileSync('git', ['status', '--porcelain', '--', name], { + cwd: repoPath, + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + encoding: 'utf8', + }); + safety.set(name, status.trim().length === 0); + } catch { + safety.set(name, false); + } + } + return safety; +}; + +/** + * Best-effort auto-commit for the AGENTS.md/CLAUDE.md files `analyze --self-commit` + * just (re)wrote. Filters `candidateFiles` down to the ones that actually exist + * under `repoPath` AND were marked safe by `snapshotSelfCommitSafety` — a file + * that already had an uncommitted edit before this run is skipped (logged), + * never swept into the generated commit. Never `git add -A`. `git status + * --porcelain` (not `diff --quiet`) is deliberate: a first-time `analyze` run + * creates AGENTS.md/CLAUDE.md fresh, and untracked files never show up in + * `git diff`, only in `git status` — the same reason `isWorkingTreeDirty` + * above uses `--porcelain`. If `git commit` fails after `git add` already + * staged the safe files (e.g. missing git identity), the staged files are + * reset back to unstaged so the user's index isn't silently left mutated. + * No-ops silently (never throws) when: none of the candidate files exist or + * are safe, none changed, or any git step fails. Must never fail the + * surrounding `analyze` run. See #2639. + */ +export const selfCommitContextFiles = ( + repoPath: string, + candidateFiles: string[], + preRunSafety: Map, +): void => { + const existing = candidateFiles.filter((name) => existsSync(path.join(repoPath, name))); + if (existing.length === 0) return; + + const safe = existing.filter((name) => preRunSafety.get(name) === true); + const skippedDirty = existing.filter((name) => preRunSafety.get(name) !== true); + if (skippedDirty.length > 0) { + logger.warn( + { files: skippedDirty }, + 'gitnexus: --self-commit skipping file(s) with uncommitted changes from before this analyze run', + ); + } + if (safe.length === 0) return; + + try { + const status = execFileSync('git', ['status', '--porcelain', '--', ...safe], { + cwd: repoPath, + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + encoding: 'utf8', + }); + if (status.trim().length === 0) return; // nothing to commit + } catch { + return; // git failed (not a repo, git missing, etc.) — nothing to do + } + + try { + execFileSync('git', ['add', '--', ...safe], { + cwd: repoPath, + stdio: 'ignore', + windowsHide: true, + }); + } catch (err) { + logger.warn({ err, files: safe }, 'gitnexus: --self-commit failed to stage context files'); + return; + } + + try { + execFileSync( + 'git', + ['commit', '-m', 'chore(gitnexus): refresh index stats [skip ci]', '--', ...safe], + { cwd: repoPath, stdio: 'ignore', windowsHide: true }, + ); + } catch (err) { + // Commit failed after `git add` already staged `safe` (e.g. missing git + // identity). Restore the index to its pre-add state for exactly those + // files rather than leaving them silently staged — `analyze` reporting + // "success" must not leave the user's index mutated. + try { + execFileSync('git', ['reset', '--', ...safe], { + cwd: repoPath, + stdio: 'ignore', + windowsHide: true, + }); + } catch { + /* best-effort restore; nothing more we can do */ + } + logger.warn({ err, files: safe }, 'gitnexus: --self-commit failed to commit context files'); + } +}; + export const isGitRepo = (repoPath: string): boolean => { try { execSync('git rev-parse --is-inside-work-tree', { diff --git a/gitnexus/test/unit/analyze-self-commit-bridge.test.ts b/gitnexus/test/unit/analyze-self-commit-bridge.test.ts new file mode 100644 index 0000000000..d8c69dcb9c --- /dev/null +++ b/gitnexus/test/unit/analyze-self-commit-bridge.test.ts @@ -0,0 +1,203 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + runFullAnalysisMock, + generateAIContextFilesMock, + generateSkillFilesMock, + cliErrorMock, + selfCommitContextFilesMock, + snapshotSelfCommitSafetyMock, +} = vi.hoisted(() => { + const runFullAnalysisMock = vi.fn(); + const generateAIContextFilesMock = vi.fn(async () => ({ files: [] as string[] })); + const generateSkillFilesMock = vi.fn(async () => ({ + skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }], + outputPath: '/repo/.claude/skills', + })); + const cliErrorMock = vi.fn(); + const selfCommitContextFilesMock = vi.fn(); + const snapshotSelfCommitSafetyMock = vi.fn( + () => + new Map([ + ['AGENTS.md', true], + ['CLAUDE.md', true], + ]), + ); + return { + runFullAnalysisMock, + generateAIContextFilesMock, + generateSkillFilesMock, + cliErrorMock, + selfCommitContextFilesMock, + snapshotSelfCommitSafetyMock, + }; +}); + +vi.mock('../../src/core/run-analyze.js', () => ({ + runFullAnalysis: runFullAnalysisMock, +})); + +vi.mock('../../src/cli/ai-context.js', () => ({ + generateAIContextFiles: generateAIContextFilesMock, +})); + +vi.mock('../../src/cli/skill-gen.js', () => ({ + generateSkillFiles: generateSkillFilesMock, +})); + +vi.mock('../../src/cli/cli-message.js', () => ({ + cliError: cliErrorMock, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ + closeLbug: vi.fn(async () => undefined), + closeLbugBeforeExit: vi.fn(async () => undefined), + isLbugReady: vi.fn(() => false), +})); + +vi.mock('../../src/storage/repo-manager.js', () => ({ + getStoragePaths: vi.fn(() => ({ storagePath: '.gitnexus', lbugPath: '.gitnexus/lbug' })), + getGlobalRegistryPath: vi.fn(() => 'registry.json'), + RegistryNameCollisionError: class RegistryNameCollisionError extends Error {}, + AnalysisNotFinalizedError: class AnalysisNotFinalizedError extends Error {}, + assertAnalysisFinalized: vi.fn(async () => undefined), +})); + +vi.mock('../../src/storage/git.js', () => ({ + getGitRoot: vi.fn(() => '/repo'), + hasGitDir: vi.fn(() => true), + getDefaultBranch: vi.fn(() => null), + selfCommitContextFiles: selfCommitContextFilesMock, + snapshotSelfCommitSafety: snapshotSelfCommitSafetyMock, +})); + +vi.mock('../../src/core/ingestion/utils/max-file-size.js', () => ({ + getMaxFileSizeBannerMessage: vi.fn(() => null), +})); + +describe('analyzeCommand --self-commit bridge (#2639)', () => { + beforeEach(() => { + vi.resetModules(); + runFullAnalysisMock.mockReset(); + runFullAnalysisMock.mockResolvedValue({ + repoName: 'repo', + repoPath: '/repo', + stats: {}, + alreadyUpToDate: true, + }); + generateAIContextFilesMock.mockReset(); + generateAIContextFilesMock.mockResolvedValue({ files: [] }); + generateSkillFilesMock.mockReset(); + generateSkillFilesMock.mockResolvedValue({ + skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }], + outputPath: '/repo/.claude/skills', + }); + cliErrorMock.mockReset(); + selfCommitContextFilesMock.mockReset(); + snapshotSelfCommitSafetyMock.mockClear(); + process.exitCode = undefined; + process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); + }); + + it('does not call selfCommitContextFiles when --self-commit is omitted (default off)', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(selfCommitContextFilesMock).not.toHaveBeenCalled(); + }); + + it('does not call selfCommitContextFiles when --self-commit is explicitly false', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { selfCommit: false }); + + expect(selfCommitContextFilesMock).not.toHaveBeenCalled(); + }); + + it('calls selfCommitContextFiles scoped to AGENTS.md/CLAUDE.md on the already-up-to-date fast path', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { selfCommit: true }); + + expect(selfCommitContextFilesMock).toHaveBeenCalledTimes(1); + expect(selfCommitContextFilesMock).toHaveBeenCalledWith( + '/repo', + ['AGENTS.md', 'CLAUDE.md'], + expect.any(Map), + ); + }); + + it('calls selfCommitContextFiles on the primary (non-fast-path) analyze run', async () => { + runFullAnalysisMock.mockResolvedValueOnce({ + repoName: 'repo', + repoPath: '/repo', + stats: { + files: 1, + nodes: 10, + edges: 20, + communities: 0, + processes: 5, + }, + alreadyUpToDate: false, + pipelineResult: { communityResult: undefined }, + }); + + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + try { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { selfCommit: true }); + + expect(selfCommitContextFilesMock).toHaveBeenCalledTimes(1); + expect(selfCommitContextFilesMock).toHaveBeenCalledWith( + '/repo', + ['AGENTS.md', 'CLAUDE.md'], + expect.any(Map), + ); + } finally { + exitSpy.mockRestore(); + } + }); + + it('does not call selfCommitContextFiles on the primary run when --self-commit is omitted', async () => { + runFullAnalysisMock.mockResolvedValueOnce({ + repoName: 'repo', + repoPath: '/repo', + stats: { + files: 1, + nodes: 10, + edges: 20, + communities: 0, + processes: 5, + }, + alreadyUpToDate: false, + pipelineResult: { communityResult: undefined }, + }); + + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + try { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(selfCommitContextFilesMock).not.toHaveBeenCalled(); + } finally { + exitSpy.mockRestore(); + } + }); + + it('composes with --no-stats (both flags threaded independently)', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { selfCommit: true, stats: false }); + + const opts = runFullAnalysisMock.mock.calls[0][1]; + expect(opts.noStats).toBe(true); + expect(selfCommitContextFilesMock).toHaveBeenCalledWith( + '/repo', + ['AGENTS.md', 'CLAUDE.md'], + expect.any(Map), + ); + }); +}); diff --git a/gitnexus/test/unit/git-utils.test.ts b/gitnexus/test/unit/git-utils.test.ts index ae0277d9ba..b1fc8f7bd4 100644 --- a/gitnexus/test/unit/git-utils.test.ts +++ b/gitnexus/test/unit/git-utils.test.ts @@ -342,6 +342,336 @@ describe('getCanonicalRepoRoot', () => { }); }); +// ─── selfCommitContextFiles (#2639) ──────────────────────────────────────── + +describe('selfCommitContextFiles', () => { + const initRepo = (): string => { + const repoDir = makeIsolatedTempDir('gitnexus-self-commit-'); + execFileSync(gitExecutable, ['init', '-q'], { cwd: repoDir, stdio: 'ignore' }); + execSync('git config user.email "test@example.com"', { cwd: repoDir }); + execSync('git config user.name "Test"', { cwd: repoDir }); + return repoDir; + }; + + const lastCommitMessage = (repoDir: string): string => + execSync('git log -1 --format=%s', { cwd: repoDir, encoding: 'utf8' }).trim(); + + const commitCount = (repoDir: string): number => + Number(execSync('git rev-list --count HEAD', { cwd: repoDir, encoding: 'utf8' }).trim()); + + const stagedFiles = (repoDir: string): string[] => + execSync('git diff --cached --name-only', { cwd: repoDir, encoding: 'utf8' }) + .split('\n') + .map((s) => s.trim()) + .filter(Boolean); + + // Most tests below aren't exercising snapshotSelfCommitSafety itself (that + // has its own describe block); they just need "everything is safe to + // commit," matching a normal run where nothing was dirty beforehand. + const allSafe = (names: string[]): Map => + new Map(names.map((name) => [name, true])); + + it('commits only the changed candidate file, scoped by name (never git add -A)', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v1\n'); + execSync('git add AGENTS.md', { cwd: repoDir }); + execSync('git commit -q -m "initial"', { cwd: repoDir }); + + // Dirty AGENTS.md (candidate) plus an unrelated untracked file that + // must never be swept in. + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v2\n'); + fs.writeFileSync(path.join(repoDir, 'unrelated.txt'), 'should stay untouched\n'); + + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ); + + expect(commitCount(repoDir)).toBe(2); + expect(lastCommitMessage(repoDir)).toBe('chore(gitnexus): refresh index stats [skip ci]'); + const status = execSync('git status --porcelain', { cwd: repoDir, encoding: 'utf8' }); + // unrelated.txt is still untracked/dirty — proves the commit was scoped. + expect(status).toContain('unrelated.txt'); + expect(status).not.toContain('AGENTS.md'); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('no-ops (no new commit) when neither candidate file changed', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v1\n'); + execSync('git add AGENTS.md', { cwd: repoDir }); + execSync('git commit -q -m "initial"', { cwd: repoDir }); + + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ); + + expect(commitCount(repoDir)).toBe(1); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('commits newly-created (untracked) candidate files, not just modified ones', async () => { + // Regression guard: a first-time `analyze --self-commit` run creates + // AGENTS.md/CLAUDE.md fresh — they are untracked, not modified. An + // implementation based on `git diff --quiet` misses untracked files + // entirely and would silently skip this case. + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + execSync('git commit -q --allow-empty -m "initial"', { cwd: repoDir }); + + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'fresh from analyze\n'); + + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ); + + expect(commitCount(repoDir)).toBe(2); + expect(lastCommitMessage(repoDir)).toBe('chore(gitnexus): refresh index stats [skip ci]'); + const status = execSync('git status --porcelain', { cwd: repoDir, encoding: 'utf8' }); + expect(status.trim()).toBe(''); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('no-ops when neither candidate file exists on disk', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + execSync('git commit -q --allow-empty -m "initial"', { cwd: repoDir }); + + expect(() => + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ), + ).not.toThrow(); + expect(commitCount(repoDir)).toBe(1); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('never throws when repoPath is not a git repository', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const tmpDir = makeIsolatedTempDir('gitnexus-self-commit-nongit-'); + try { + fs.writeFileSync(path.join(tmpDir, 'AGENTS.md'), 'not a git repo\n'); + expect(() => + selfCommitContextFiles( + tmpDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ), + ).not.toThrow(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('commits both files when both changed, still scoped (no -A)', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v1\n'); + fs.writeFileSync(path.join(repoDir, 'CLAUDE.md'), 'v1\n'); + execSync('git add AGENTS.md CLAUDE.md', { cwd: repoDir }); + execSync('git commit -q -m "initial"', { cwd: repoDir }); + + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v2\n'); + fs.writeFileSync(path.join(repoDir, 'CLAUDE.md'), 'v2\n'); + + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ); + + expect(commitCount(repoDir)).toBe(2); + const status = execSync('git status --porcelain', { cwd: repoDir, encoding: 'utf8' }); + expect(status.trim()).toBe(''); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('logs a warning (never throws) when the commit step fails, e.g. no git identity', async () => { + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const { _captureLogger } = await import('../../src/core/logger.js'); + const repoDir = initRepo(); + try { + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v1\n'); + execSync('git add AGENTS.md', { cwd: repoDir }); + execSync('git commit -q -m "initial"', { cwd: repoDir }); + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v2\n'); + + // useConfigOnly forces git to error on a missing identity instead of + // guessing from OS user/hostname; HOME/XDG_CONFIG_HOME are redirected + // and GIT_CONFIG_NOSYSTEM disables the system config, so no ambient + // global identity on the CI runner can leak in and make git succeed + // anyway. Together these deterministically reproduce "no git identity + // configured" regardless of the machine running the test. + execSync('git config user.useConfigOnly true', { cwd: repoDir }); + execSync('git config --unset user.name', { cwd: repoDir }); + execSync('git config --unset user.email', { cwd: repoDir }); + + const savedHome = process.env.HOME; + const savedXdg = process.env.XDG_CONFIG_HOME; + const savedNoSystem = process.env.GIT_CONFIG_NOSYSTEM; + process.env.HOME = makeIsolatedTempDir('gitnexus-self-commit-noidentity-home-'); + process.env.XDG_CONFIG_HOME = process.env.HOME; + process.env.GIT_CONFIG_NOSYSTEM = '1'; + + const cap = _captureLogger(); + try { + expect(() => + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ), + ).not.toThrow(); + } finally { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedXdg === undefined) delete process.env.XDG_CONFIG_HOME; + else process.env.XDG_CONFIG_HOME = savedXdg; + if (savedNoSystem === undefined) delete process.env.GIT_CONFIG_NOSYSTEM; + else process.env.GIT_CONFIG_NOSYSTEM = savedNoSystem; + } + const warning = cap + .records() + .find((r) => r.msg.includes('--self-commit failed to commit context files')); + cap.restore(); + + expect(warning).toBeDefined(); + expect(commitCount(repoDir)).toBe(1); // commit never landed + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('skips (and logs) a candidate that already had an uncommitted edit before this run, never sweeping it into the generated commit', async () => { + // Regression for #2640 review round 1/2: without snapshotSelfCommitSafety, + // a pre-existing unstaged user edit in AGENTS.md and this run's stats + // refresh are indistinguishable — both just show up as "AGENTS.md is + // dirty" — so the old implementation silently committed both together. + const { selfCommitContextFiles, snapshotSelfCommitSafety } = + await import('../../src/storage/git.js'); + const { _captureLogger } = await import('../../src/core/logger.js'); + const repoDir = initRepo(); + try { + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'v1\n'); + fs.writeFileSync(path.join(repoDir, 'CLAUDE.md'), 'v1\n'); + execSync('git add AGENTS.md CLAUDE.md', { cwd: repoDir }); + execSync('git commit -q -m "initial"', { cwd: repoDir }); + + // A user edit lands in AGENTS.md BEFORE analyze/self-commit ever runs. + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'user note\n'); + + // The real call sequence: snapshot safety first (this is what + // analyze.ts does before writing), THEN simulate analyze's own write + // on top of the user's pre-existing edit, for both candidates. + const safety = snapshotSelfCommitSafety(repoDir, ['AGENTS.md', 'CLAUDE.md']); + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'user note\ngenerated stats refresh\n'); + fs.writeFileSync(path.join(repoDir, 'CLAUDE.md'), 'generated stats refresh\n'); + + const cap = _captureLogger(); + selfCommitContextFiles(repoDir, ['AGENTS.md', 'CLAUDE.md'], safety); + const warning = cap + .records() + .find((r) => r.msg.includes('skipping file(s) with uncommitted changes')); + cap.restore(); + + expect(warning).toBeDefined(); + // CLAUDE.md was clean pre-run (safe) and got committed; AGENTS.md was + // already dirty pre-run (unsafe) and must stay out of the commit and + // out of the index entirely — proving its edit wasn't swept in. + expect(commitCount(repoDir)).toBe(2); + expect(lastCommitMessage(repoDir)).toBe('chore(gitnexus): refresh index stats [skip ci]'); + const diffTreeFiles = execSync('git diff-tree --no-commit-id --name-only -r HEAD', { + cwd: repoDir, + encoding: 'utf8', + }) + .split('\n') + .map((s) => s.trim()) + .filter(Boolean); + expect(diffTreeFiles).toEqual(['CLAUDE.md']); + const status = execSync('git status --porcelain -- AGENTS.md', { + cwd: repoDir, + encoding: 'utf8', + }); + expect(status.trim()).not.toBe(''); // AGENTS.md's edit is still there, untouched + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('restores the index for exactly the staged files when commit fails after git add (no leftover staged state)', async () => { + // Regression for #2640 review round 2: `git add` runs before `git commit`; + // if commit then fails (e.g. missing identity), the old implementation + // left the candidate staged even though it reported nothing happened — + // silently mutating the user's index on a run that "did nothing." + const { selfCommitContextFiles } = await import('../../src/storage/git.js'); + const repoDir = initRepo(); + try { + execSync('git commit -q --allow-empty -m "initial"', { cwd: repoDir }); + fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), 'fresh from analyze\n'); + + execSync('git config user.useConfigOnly true', { cwd: repoDir }); + execSync('git config --unset user.name', { cwd: repoDir }); + execSync('git config --unset user.email', { cwd: repoDir }); + + const savedHome = process.env.HOME; + const savedXdg = process.env.XDG_CONFIG_HOME; + const savedNoSystem = process.env.GIT_CONFIG_NOSYSTEM; + process.env.HOME = makeIsolatedTempDir('gitnexus-self-commit-noidentity-home2-'); + process.env.XDG_CONFIG_HOME = process.env.HOME; + process.env.GIT_CONFIG_NOSYSTEM = '1'; + try { + selfCommitContextFiles( + repoDir, + ['AGENTS.md', 'CLAUDE.md'], + allSafe(['AGENTS.md', 'CLAUDE.md']), + ); + } finally { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedXdg === undefined) delete process.env.XDG_CONFIG_HOME; + else process.env.XDG_CONFIG_HOME = savedXdg; + if (savedNoSystem === undefined) delete process.env.GIT_CONFIG_NOSYSTEM; + else process.env.GIT_CONFIG_NOSYSTEM = savedNoSystem; + } + + expect(commitCount(repoDir)).toBe(1); // commit never landed + expect(stagedFiles(repoDir)).toEqual([]); // and nothing was left staged + // The file itself is still there, unstaged, exactly as analyze left it. + const status = execSync('git status --porcelain -- AGENTS.md', { + cwd: repoDir, + encoding: 'utf8', + }); + expect(status.trim()).toBe('?? AGENTS.md'); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); +}); + // ─── isWorkingTreeDirty ─────────────────────────────────────────────────── // // analyze's fast-path gate. GitNexus writes to .gitnexus/, .claude/, .cursor/,