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/,