Skip to content

fix(cli): keep GitNexus ignores inside .gitnexus#1248

Merged
magyargergo merged 4 commits into
mainfrom
fix/gitnexus-internal-gitignore-1233
May 1, 2026
Merged

fix(cli): keep GitNexus ignores inside .gitnexus#1248
magyargergo merged 4 commits into
mainfrom
fix/gitnexus-internal-gitignore-1233

Conversation

@magyargergo

Copy link
Copy Markdown
Collaborator

Summary

  • Stop mutating analyzed repositories' root .gitignore during analyze and index.
  • Create .gitnexus/.gitignore with * so generated GitNexus storage stays untracked from inside the storage directory.
  • Cover full analyze, index, and already-up-to-date analyze fast-path behavior.

Closes #1233

Test plan

  • cd gitnexus && npx prettier --check src/storage/repo-manager.ts src/core/run-analyze.ts src/cli/index-repo.ts test/unit/repo-manager.test.ts test/unit/index-repo-command.test.ts test/unit/run-analyze.test.ts test/integration/cli-e2e.test.ts
  • cd gitnexus && npx vitest run test/unit/repo-manager.test.ts test/unit/index-repo-command.test.ts test/unit/run-analyze.test.ts test/integration/cli-e2e.test.ts -t "ensureGitNexusInternalGitignore|already-up-to-date|analyze command runs pipeline|registers successfully"
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus && npm test (final rerun passed: 254 files, 7505 tests)
  • cd gitnexus && npx tsx src/cli/index.ts detect-changes --repo GitNexus (low risk, 0 affected processes)

Avoid mutating analyzed repositories' root .gitignore while keeping generated GitNexus state untracked via .gitnexus/.gitignore.

Made-with: Cursor
@vercel

vercel Bot commented May 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitnexus Ready Ready Preview, Comment May 1, 2026 0:52am

Request Review

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 4m 6s —— View job


Deep Production-Readiness Review: PR #1248 — fix(cli): keep GitNexus ignores inside .gitnexus


Review bar for this PR

  • No GitNexus analyze/index path modifies the analyzed repository's root .gitignore
  • .gitnexus/.gitignore is created with exactly *\n whenever GitNexus creates or uses .gitnexus storage in a git repo
  • Existing root .gitignore files remain byte-for-byte unchanged
  • Re-running analyze/index is idempotent and does not create noisy diffs
  • Already-up-to-date analyze still ensures .gitnexus/.gitignore exists
  • index command and full analyze command both use the same helper
  • The change does not break registry, metadata, assertAnalysisFinalized, or detect-changes behavior
  • Tests prove root .gitignore is untouched and git status --short stays clean
  • CI, typecheck, focused tests, and full relevant test suite are green before approval

Issue and PR context considered

#1233 symptom: GitNexus was appending .gitnexus to the analyzed repository's root .gitignore during every analyze and index run, forcing users to either commit that line or mark the file with skip-worktree.

Intended fix: Stop writing to the root .gitignore entirely. Instead, create .gitnexus/.gitignore containing *, which causes Git to ignore all generated files under .gitnexus/ (including the .gitignore itself) without touching user-owned files.

Changed files: repo-manager.ts, run-analyze.ts, index-repo.ts, cli-e2e.test.ts, repo-manager.test.ts, run-analyze.test.ts, index-repo-command.test.ts

Current CI status: CI workflow is in_progress; Docker Build & Push is in_progress. This blocks merge per the DoD.

Core risk areas: (a) non-git and --skip-git/parent-git-subdir behavior, (b) full analyze path guard condition, (c) helper idempotency, (d) tests proving git status cleanliness.


Understanding of the change

What #1233 reported: Every analyze or index run appended .gitnexus to the repo's root .gitignore, mutating a source-controlled file the user didn't ask GitNexus to touch.

Why that's undesirable: It creates unwanted dirty state in every analyzed repo, forcing developers to either commit GitNexus-specific lines into their project's .gitignore or use git update-index --skip-worktree. This breaks the principle that a tool should not mutate files outside its own storage.

How .gitnexus/.gitignore with * solves it: A .gitignore inside a directory applies to that directory and all descendants. Git traverses into untracked directories looking for .gitignore files, so <repo>/.gitnexus/.gitignore containing * is sufficient to keep all generated content (including the .gitignore itself) invisible to git status, even if the root .gitignore doesn't exist.

Paths changed: Full analyze (post-register, guarded by hasGitDir), already-up-to-date fast path (guarded by currentCommit !== ''), and index command (unconditional after register).

What must remain unchanged: Registry, meta schema, assertAnalysisFinalized, detect-changes, embeddings, AI context file generation — all untouched.

CI status: In progress — cannot approve.


Findings

[critical] CI still in progress

  • Category: Validation gate
  • Files: N/A
  • Issue: Both the CI workflow and Docker Build & Push are in_progress at review time. No conclusion for either.
  • Why it matters here: The DoD requires CI green before approval. A typecheck or test regression introduced by this change cannot be ruled out until the run completes.
  • Recommended fix: Wait for CI to complete and produce success on both workflows before merging.
  • Blocks merge: yes

[high] Full analyze skips .gitnexus/.gitignore creation for non-git paths, including --skip-git subfolders inside a parent git repo

  • Category: Correctness / storage hygiene

  • Files: gitnexus/src/core/run-analyze.ts:452

  • Issue: The full analyze path wraps the helper call in if (hasGitDir(repoPath)):

    if (hasGitDir(repoPath)) {
      await ensureGitNexusInternalGitignore(repoPath);
    }

    hasGitDir checks for .git in the exact directory passed (statSync(path.join(dirPath, '.git'))). It returns false for any path that has no .git directly at its root — including subdirectories that are inside a parent git repo.

    Scenario: gitnexus analyze /parent/repo/subdir --skip-git where subdir has no .git of its own. hasGitDir('/parent/repo/subdir') returns false. The full pipeline runs, creates /parent/repo/subdir/.gitnexus/, but never creates .gitnexus/.gitignore. The parent git repo at /parent/repo will see subdir/.gitnexus/ as untracked files — exactly the problem Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233 describes.

  • Why it matters here: This is the --skip-git use case explicitly called out in the issue. The index command has no such guard — it calls the helper unconditionally. That inconsistency means the same subfolder analyzed with analyze leaks into the parent repo's git status, but the same subfolder registered via index does not. Half the issue is fixed; the --skip-git path is not.

  • Recommended fix:
    Remove the hasGitDir guard on the full analyze path. Call ensureGitNexusInternalGitignore(repoPath) unconditionally after saveMeta / registerRepo, matching the index command's behavior:

    // Before (full analyze path, run-analyze.ts ~line 452):
    if (hasGitDir(repoPath)) {
      await ensureGitNexusInternalGitignore(repoPath);
    }
    
    // After:
    await ensureGitNexusInternalGitignore(repoPath);

    Creating a .gitignore inside .gitnexus for a genuinely non-git folder is harmless. When the folder is a subdirectory of a parent git repo, it is beneficial. There is no downside to unconditional creation.

    Add a test exercising ensureGitNexusInternalGitignore called from a path without .git that lives inside a real git repo, confirming git status --short in the parent stays empty.

  • Blocks merge: yes — the parent-git-subdir case is reachable via --skip-git and represents a real regression relative to the fix's stated goal. Fix this →


[medium] No test proves git status cleanliness when analyze creates .gitnexus from scratch (full pipeline path)

  • Category: Test coverage
  • Files: gitnexus/test/unit/run-analyze.test.ts, gitnexus/test/integration/cli-e2e.test.ts
  • Issue: The unit test for the fast path (already-up-to-date) proves .gitnexus/.gitignore exists but does not assert git status --short is empty. The repo-manager helper test does assert git status --short is empty, but that's a unit test of the helper in isolation, not the full runFullAnalysis() wiring. The E2E test asserts .gitnexus/.gitignore content ('*\n') but not git status --short output — it only indirectly implies cleanliness. No test explicitly verifies that after a full runFullAnalysis() completes, git status --short is empty on the target repo.
  • Why it matters here: The DoD requires "tests prove root .gitignore is untouched and git status --short stays clean." A git status --short assertion on the E2E test's MINI_REPO after analyze would catch any path that accidentally mutates the working tree.
  • Recommended fix: In cli-e2e.test.ts, after the analyze command runs pipeline on mini-repo test, add:
    const statusOut = spawnSync('git', ['status', '--short'], { cwd: MINI_REPO, encoding: 'utf8' });
    expect(statusOut.stdout).toBe('');
    This is the strongest possible assertion for the DoD bar.
  • Blocks merge: no — existing coverage (helper unit test + E2E content assertion) is materially close; this is a strengthening recommendation.

[low] E2E test assertion is brittle against future fixture evolution

  • Category: Test design
  • Files: gitnexus/test/integration/cli-e2e.test.ts:201
  • Issue:
    expect(fs.existsSync(path.join(MINI_REPO, '.gitignore'))).toBe(false);
    This asserts the root .gitignore does not exist. It is correct today because the mini-repo fixture has no root .gitignore. If the fixture legitimately gains a .gitignore later (e.g. to exclude IDE files from the test fixture itself), this test will fail even if the behavior is correct.
  • Why it matters here: Tests should prove behavior, not rely on fixture state. The intent is that analyze does not create or modify the root .gitignore, not that one cannot exist. A more robust assertion would read the file content before and after and assert it is unchanged.
  • Recommended fix: Either record the before-content and assert it is byte-for-byte identical after, or document in the test comment that the fixture intentionally has no root .gitignore.
  • Blocks merge: no — the current assertion is correct for the current fixture.

[low] Helper overwrites .gitnexus/.gitignore on every call without a content check

  • Category: Performance / intent documentation
  • Files: gitnexus/src/storage/repo-manager.ts:247
  • Issue: ensureGitNexusInternalGitignore always writes *\n, overwriting any existing content. The name ensure... implies idempotency, but it's actually "always-overwrite."
    await fs.writeFile(gitignorePath, '*\n', 'utf-8');
    Any user who manually edits .gitnexus/.gitignore for a legitimate purpose (e.g. un-ignoring a generated index to share with a team) will have their edit silently overwritten on the next analyze.
  • Why it matters here: .gitnexus is internal generated storage, so overwriting is probably acceptable. But it should be documented as intentional so future maintainers don't introduce a content-check without understanding the design intent.
  • Recommended fix: Add a one-line comment: // Always overwrite: .gitnexus is generated storage; this file is owned by GitNexus.
  • Blocks merge: no

[low] Migration: existing repos with old root .gitignore entries are not cleaned up

  • Category: Compatibility / release notes
  • Files: N/A (by design)
  • Issue: Repos that previously ran gitnexus analyze will have .gitnexus in their root .gitignore. The new behavior does not remove it. This is the correct policy (never delete user-owned file content), but users are not informed.
  • Recommended fix: Add a release note:

    GitNexus no longer writes to your repository's root .gitignore. Generated files are now ignored from inside .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it.

  • Blocks merge: no

Definition of Done check

DoD item Status
No GitNexus analyze/index path modifies the analyzed repository's root .gitignore satisfied — zero addToGitignore references remain; helper writes only to .gitnexus/.gitignore
.gitnexus/.gitignore is created with exactly *\n whenever GitNexus creates or uses .gitnexus in a git repo partially satisfiedindex command: satisfied unconditionally; analyze full path: only when hasGitDir(repoPath) is true (misses --skip-git + parent-git-subdir case)
Existing root .gitignore files remain byte-for-byte unchanged satisfied — confirmed by code inspection and test at repo-manager.test.ts:87-93
Re-running analyze/index is idempotent and does not create noisy diffs satisfied — helper overwrites *\n with *\n on every call; no diff created
Already-up-to-date analyze still ensures .gitnexus/.gitignore exists satisfied — fast path at run-analyze.ts:169 calls helper inside if (currentCommit !== '')
index command and full analyze command both use the same helper partially satisfied — both use ensureGitNexusInternalGitignore, but analyze guards with hasGitDir; index does not
The change does not break registry, metadata, assertAnalysisFinalized, or detect-changes satisfied — no registry or meta schema changes; assertAnalysisFinalized logic untouched
Tests prove root .gitignore is untouched and git status --short stays clean partially satisfiedgit status --short is proven clean by helper unit test; E2E test asserts .gitignore content but not git status --short on full analyze path
CI, typecheck, focused tests, and full relevant test suite are green before approval not satisfied — CI is in_progress

Storage / gitignore behavior assessment

Root .gitignore mutation: Fully eliminated. addToGitignore has zero remaining references in the codebase. ignore-service.ts references are reads-only (file discovery), not writes. No production path writes to the root .gitignore.

.gitnexus/.gitignore creation: ensureGitNexusInternalGitignore correctly computes path.join(getStoragePath(repoPath), '.gitignore') = <repoPath>/.gitnexus/.gitignore. Uses fs.mkdir(..., { recursive: true }) to create the .gitnexus dir. Writes exactly '*\n'. Does not read or mutate the root .gitignore. Path is scoped to the local repo, not global ~/.gitnexus (which uses getGlobalDir()os.homedir()/.gitnexus — a completely separate code path).

Idempotency: Writing *\n over *\n is a no-op from a behavioral standpoint. The file is never appended to, only overwritten. Git sees no diff.

Git status cleanliness: * in a .gitignore inside a directory causes Git to ignore all files in that directory, including the .gitignore itself. Proven by the repo-manager.test.ts test that runs git status --short after creating the gitignore and a dummy meta.json, asserting empty output.

Existing root .gitignore preservation: The helper never opens the root .gitignore path. Confirmed by test at repo-manager.test.ts:87-93 which writes node_modules/\n to root .gitignore, calls the helper, and asserts content is unchanged.

Old root .gitignore cleanup policy: Not auto-removed (correct). Release note should mention manual cleanup option.


Analyze/index path assessment

Full analyze: ensureGitNexusInternalGitignore(repoPath) is called after saveMeta and registerRepo, inside the try block, before AI context files. It is gated by if (hasGitDir(repoPath))this misses non-git paths, including --skip-git subfolders inside parent git repos.

Already-up-to-date fast path: Helper called inside if (currentCommit !== ''), which is correct — non-git repos have empty commit and fall through to full rebuild. Git repos on the fast path get the gitignore created/refreshed. ✓

Index command: Helper called unconditionally after registerRepo. No hasGitDir guard. Works for git repos, --allow-non-git paths, and cwd-discovered repos. ✓

Non-git / --skip-git behavior (full analyze): hasGitDir checks for .git directly in repoPath only (statSync(path.join(dirPath, '.git'))). A subdirectory without its own .git returns false. The helper is skipped. The parent git repo sees .gitnexus as untracked. This is the open issue.

Parent-git-subdir behavior: Described above. Not covered; index handles it, analyze does not.

Registry/meta/finalization impact: Zero. registerRepo, saveMeta, assertAnalysisFinalized, and assertSafeStoragePath are all untouched. The new helper runs after registration — same ordering risk as the old addToGitignore, not a regression.


Test assessment

Tests added/updated:

  • repo-manager.test.ts: 3 new tests for ensureGitNexusInternalGitignore — creates .gitnexus/.gitignore with *\n, preserves existing root .gitignore, proves git status --short is empty after helper + written storage files. Uses real git init and real execSync. ✓
  • run-analyze.test.ts: 1 new test for already-up-to-date fast path — real git repo, saves meta with current commit, runs runFullAnalysis(), asserts alreadyUpToDate: true and .gitnexus/.gitignore content. ✓
  • index-repo-command.test.ts: Updated to mock ensureGitNexusInternalGitignore; asserts it is called on successful register and not called on validation failures. ✓
  • cli-e2e.test.ts: 2 new assertions in the analyze command runs pipeline test — root .gitignore absent, .gitnexus/.gitignore content is *\n. ✓

Helper tests: Real-filesystem, real-git tests. Not mock-only. ✓

CLI E2E tests: Assertions cover existence and content. Do not assert git status --short explicitly. Gap noted above.

Git status test: Present in repo-manager.test.ts for the helper in isolation. Not present for the full runFullAnalysis() path.

Missing cases:

  1. Full analyze via runFullAnalysis() + git status --short assertion
  2. --skip-git on a path without .git that is inside a parent git repo (helper is not called; .gitnexus/.gitignore is not created)

Would old behavior fail? Yes — repo-manager.test.ts:79-84 asserts .gitnexus/.gitignore content is *\n; the old addToGitignore approach would write to root .gitignore instead, which would fail the git status test and the content assertion.


Validation assessment

Latest CI status: CIin_progress; Docker Build & Pushin_progress. Three informational workflows (PR Conventional Labeler ×2, PR Description Check) passed.

Typecheck (tsc --noEmit): Cannot confirm until CI completes. Import/export changes (ensureGitNexusInternalGitignore added to exports, addToGitignore removed) are correctly reflected across all importing files based on code inspection.

Focused tests: Correct selection (ensureGitNexusInternalGitignore|already-up-to-date|analyze command runs pipeline|registers successfully) covers the new and changed paths.

Full test suite: PR body claims 254 files, 7505 tests passing locally. Cannot verify until CI reports.

Detect-changes smoke: Listed in PR body as npx tsx src/cli/index.ts detect-changes --repo GitNexus. This is smoke, not proof.

Pending CI blocks approval: Yes.


Compatibility and rollout assessment

Existing repos with old root .gitignore entry: Unaffected. The new code never reads or writes the root .gitignore. Old entries remain until the user removes them manually.

Existing .gitnexus storage: Fully compatible. No schema change. No meta format change. On next analyze (including the fast path), .gitnexus/.gitignore is created. On next index, same.

User cleanup expectations: Users may optionally remove the .gitnexus entry from their root .gitignore. No action required for correctness — the internal gitignore takes over. If both exist, git applies both (redundant but harmless).

Release note recommendation:

GitNexus no longer writes to your repository's root .gitignore. Starting with this release, generated index files are kept untracked via .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it manually.


Final verdict

not production-ready

CI is still in progress — this alone is a hard block per the DoD. Beyond CI, the analyze full path contains a hasGitDir(repoPath) guard that prevents .gitnexus/.gitignore from being created when --skip-git is used on a subdirectory inside a parent git repo. The index command has no such guard and handles this case correctly, but the inconsistency means the most common scenario for --skip-git usage (analyzing a monorepo subfolder with no .git) leaves .gitnexus visible to the parent repo's git status — which is precisely what #1233 was filed to prevent. The fix is a one-line removal of the if (hasGitDir(repoPath)) condition on the full analyze path.

The remaining DoD items (root mutation elimination, idempotency, already-up-to-date path, registry/meta integrity, root .gitignore preservation) are all satisfied. Once CI is green and the hasGitDir guard is removed from the full analyze path (with a matching test for the --skip-git + parent-git-subdir case), this PR is otherwise in good shape to merge.


View CI run · View PR

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
7740 7739 0 1 354s

✅ All 7739 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 77.14% 22895/29677 77.02% 📈 +0.1 🟢 ███████████████░░░░░
Branches 66.05% 14714/22276 65.97% 📈 +0.1 🟢 █████████████░░░░░░░
Functions 82.1% 2253/2744 81.86% 📈 +0.2 🟢 ████████████████░░░░
Lines 80.03% 20664/25820 79.89% 📈 +0.1 🟢 ████████████████░░░░

📋 View full run · Generated by CI

When an analyzed repo has a real .git directory, add .gitnexus/ to .git/info/exclude so local Git metadata ignores generated storage without touching root .gitignore.

Made-with: Cursor
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 3m 44s —— View job


Production-Readiness Review: PR #1248 — fix(cli): keep GitNexus ignores inside .gitnexus


Review bar for this PR

  • No GitNexus analyze/index path modifies the analyzed repository's root .gitignore
  • .gitnexus/.gitignore is created with exactly *\n on every path that creates .gitnexus storage
  • Existing root .gitignore files remain byte-for-byte unchanged
  • Re-running analyze/index is idempotent and does not create noisy diffs
  • Already-up-to-date analyze still ensures .gitnexus/.gitignore exists
  • Full analyze and index command use the same helper with consistent guards
  • Parent-git/subdir --skip-git analysis keeps the parent working tree clean
  • CI, typecheck, focused tests, and relevant full tests are green before approval

Issue and PR context considered

#1233 symptom: GitNexus appended .gitnexus to the analyzed repository's root .gitignore on every analyze and index run, creating unwanted dirty working tree state and forcing users to commit a GitNexus-specific line or use git update-index --skip-worktree.

Intended fix: Stop writing to the root .gitignore entirely. Create .gitnexus/.gitignore containing *, causing Git to ignore all generated content inside .gitnexus/ (including the ignore file itself) without touching user-owned files. Additionally add .gitnexus/ to .git/info/exclude for direct-git-repo cases as a belt-and-suspenders layer.

Changed files: repo-manager.ts, run-analyze.ts, index-repo.ts, cli-e2e.test.ts, repo-manager.test.ts, run-analyze.test.ts, index-repo-command.test.ts

Current CI status: CI workflow FAILED (run 25212892995, created 2026-05-01T11:35:44Z). This run appears to correspond to the second commit on this branch (5971d07 fix(cli): also use git info exclude for GitNexus storage), pushed after the earlier passing run (which covered only the first commit e3c122c). Docker Build & Push completed successfully.

Core risk areas: (a) hasGitDir guard on full analyze path misses --skip-git/parent-git-subdir; (b) CI failing on current HEAD; (c) missing parent-git/subdir regression test.


Understanding of the change

What #1233 reported: Every analyze or index run appended .gitnexus to the repo's root .gitignore, mutating a source-controlled file the user didn't ask GitNexus to touch.

Why undesirable: Modifying a project's .gitignore is hostile — it makes the tool invisible to git status only at the cost of polluting a shared project file, forcing either a committed lint-noise entry or a per-machine skip-worktree workaround.

How .gitnexus/.gitignore with * solves it: A .gitignore placed inside a directory applies to that directory and all descendants. Git traverses into untracked directories looking for .gitignore files, so <repo>/.gitnexus/.gitignore containing * is sufficient to keep all generated content (including itself) invisible to git status without any root-level change.

Why .git/info/exclude was added: Belt-and-suspenders: .git/info/exclude is a per-clone, non-source-controlled exclude file. Adding .gitnexus/ there makes the directory invisible to git even before Git descends to read .gitnexus/.gitignore. The helper (ensureGitInfoExclude) only mutates this when .git is a real directory (not a worktree file), is idempotent, and preserves existing content.

Paths changed: Full analyze (guarded by hasGitDir), already-up-to-date fast path (unconditional inside if (currentCommit !== '')), and index command (unconditional). The helper does not touch the root .gitignore on any path.

What must remain unchanged: Registry, meta schema, assertAnalysisFinalized, detect-changes, embeddings, AI context file generation — all untouched.

CI status: Failing on current HEAD.


Findings

[critical] CI is failing on the current HEAD commit

  • Category: Validation gate
  • Files: N/A
  • Issue: The CI workflow (25212892995) shows conclusion: failure for the current HEAD commit (5971d07 fix(cli): also use git info exclude for GitNexus storage, created 2026-05-01T11:35:44Z). The earlier passing CI report in the comment thread covered only the first commit (e3c122c). The second commit introduced ensureGitInfoExclude and triggered a new CI run which failed. Job details were unavailable to retrieve via API, so the exact failing step is unconfirmed — but the failure conclusion is certain.
  • Why it matters: Any typecheck or test regression introduced by the .git/info/exclude logic cannot be ruled out. The DoD requires CI green before approval.
  • Recommended fix: Diagnose and fix the CI failure. Re-run the focused test plan from the PR body against the current HEAD. Do not merge until CI shows success.
  • Blocks merge: yes

[high] Full analyze path skips .gitnexus/.gitignore creation for non-git paths, including --skip-git subfolders inside a parent git repo

  • Category: Correctness / storage hygiene

  • Files: gitnexus/src/core/run-analyze.ts:452–454

  • Issue: The full analyze path guards the helper call with:

    // run-analyze.ts line 452
    if (hasGitDir(repoPath)) {
      await ensureGitNexusIgnored(repoPath);
    }

    hasGitDir checks for .git directly in repoPath — it returns false for any path without its own .git root. The scenario gitnexus analyze --skip-git on parent-repo/app/ (where app/ has no .git but parent-repo/ does) hits this gap:

    1. hasGitDir('parent-repo/app/')false
    2. repoHasGit = false, currentCommit = '' → full pipeline runs (not fast-path)
    3. .gitnexus/ is created and populated during the pipeline
    4. if (hasGitDir(...)) → skipped; .gitnexus/.gitignore is never created
    5. parent-repo/ git sees app/.gitnexus/ as untracked — the original Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233 problem persists

    The index command (line 118) calls ensureGitNexusIgnored(repoPath) unconditionally. This inconsistency means the same folder analyzed via analyze leaks into the parent repo's git status, but the same folder registered via index does not.

  • Why it matters here: --skip-git is explicitly the use case for analyzing monorepo subfolders and non-git projects. This is the most common scenario that triggered Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233 in multi-project workspaces. The half-fix means the problem is only resolved for direct git repos; the reported case (subfolder inside a parent git repo) is unresolved on the full analyze path.

  • Recommended fix:
    Remove the if (hasGitDir(repoPath)) guard. Call ensureGitNexusIgnored(repoPath) unconditionally, matching the index command:

    // run-analyze.ts ~line 452 — replace the guarded block with:
    await ensureGitNexusIgnored(repoPath);

    The ensureGitInfoExclude sub-helper inside ensureGitNexusIgnored already returns early when .git is not a real directory (line 259: if (!gitDir.isDirectory()) return;), so it is already safe for non-git paths. Creating .gitnexus/.gitignore for a genuinely non-git folder is harmless; for a subfolder of a parent git repo, it is essential. Fix this →

  • Blocks merge: yes


[medium] No test proves the parent-git/subdir --skip-git scenario is clean

  • Category: Test coverage

  • Files: gitnexus/test/unit/run-analyze.test.ts, gitnexus/test/integration/cli-e2e.test.ts

  • Issue: No test exercises the scenario:

    parent/
      .git/
      app/         ← no .git
    

    Run analyze (or runFullAnalysis with skipGit: true) on app/, then assert:

    • app/.gitnexus/.gitignore exists and contains *\n
    • git status --short from parent/ is empty

    The existing repo-manager.test.ts helper tests prove the helper works in isolation; the E2E test proves the happy-path direct-git-repo case. Neither covers the subdir case. Without a fix to the production code (finding 2) and a regression test for it, this scenario is silently broken.

  • Recommended fix: Add a unit test in run-analyze.test.ts:

    it('creates .gitnexus/.gitignore on full analyze of --skip-git subdir inside parent git repo', async () => {
      // create parent with .git, then a subdirectory without .git
      // call runFullAnalysis(..., { skipGit: true }, ...)
      // assert app/.gitnexus/.gitignore === '*\n'
      // assert git status --short from parent is ''
    });
  • Blocks merge: yes (contingent on production code fix; without the fix, this test would fail and should be added alongside the code change)


[low] E2E test asserts root .gitignore is absent — brittle against fixture evolution

  • Category: Test design
  • Files: gitnexus/test/integration/cli-e2e.test.ts:201
  • Issue:
    expect(fs.existsSync(path.join(MINI_REPO, '.gitignore'))).toBe(false);
    This passes today because mini-repo has no root .gitignore. If the fixture gains one legitimately (e.g. IDE excludes), this assertion fails even if behavior is correct. The intent is to prove analyze does not create or modify the root .gitignore, not that one cannot exist.
  • Recommended fix: Either snapshot the file content before/after analyze and assert equality, or add a comment: // mini-repo fixture intentionally has no root .gitignore; this asserts analyze did not create one.
  • Blocks merge: no

[low] .gitnexus/.gitignore is silently overwritten on every call without documenting intent

  • Category: Maintainability
  • Files: gitnexus/src/storage/repo-manager.ts:248
  • Issue: ensureGitNexusIgnored always writes *\n, overwriting any existing content. The name ensure… implies idempotency via a content check, but the implementation is "always-overwrite." Any user who edits .gitnexus/.gitignore for a legitimate purpose (e.g. un-ignoring a generated index to share with a team) will have their edit silently dropped on the next analyze.
  • Recommended fix: Add one comment: // Always overwrite: .gitnexus/ is generated storage owned by GitNexus.
  • Blocks merge: no

[low] Old root .gitignore entries from previous runs are not cleaned up — release note needed

  • Category: Compatibility / release notes
  • Files: N/A (by design — removing user file content is risky)
  • Issue: Repos that ran gitnexus analyze before this fix will have .gitnexus or .gitnexus/ in their root .gitignore. The new behavior leaves those entries in place (correct policy). Users are not informed.
  • Recommended fix: Add to release notes:

    GitNexus no longer writes to your repository's root .gitignore. Generated files are now ignored via .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it manually.

  • Blocks merge: no

Definition of Done check

DoD item Status
No analyze/index path modifies the root .gitignore satisfied — zero addToGitignore references remain in production code; ignore-service.ts references are reads-only
.gitnexus/.gitignore created with *\n on every path that creates .gitnexus partially satisfiedindex: unconditional ✓; fast-path analyze: unconditional (guarded by currentCommit !== '', correct) ✓; full analyze: only when hasGitDir — misses --skip-git/parent-git-subdir
Existing root .gitignore files remain byte-for-byte unchanged satisfied — confirmed by code inspection and repo-manager.test.ts:87–93
Re-running is idempotent satisfied — always-overwrite of *\n produces no diff; .git/info/exclude append is guarded with duplicate-entry check
Already-up-to-date analyze ensures .gitnexus/.gitignore exists satisfied — fast path calls ensureGitNexusIgnored unconditionally inside if (currentCommit !== '')
Full analyze and index command use consistent ignore behavior partially satisfied — both use ensureGitNexusIgnored; index is unconditional, full analyze guards with hasGitDir — inconsistent
Parent-git/subdir --skip-git keeps parent working tree clean not satisfiedhasGitDir guard blocks helper call; no test covers this
git status --short stays clean after analyze partially satisfied — proven for helper in isolation (repo-manager.test.ts:126–141) and implied by E2E content assertion; not proven for full runFullAnalysis() or parent-git/subdir path
CI, typecheck, focused tests, full suite green not satisfied — CI is failing on current HEAD

Storage / ignore behavior assessment

Root .gitignore mutation: Fully eliminated. addToGitignore has zero references in production code. ignore-service.ts references are read-only (file discovery). No production path writes to the root .gitignore. ✓

.gitnexus/.gitignore creation: ensureGitNexusIgnored correctly computes path.join(getStoragePath(repoPath), '.gitignore') = <repoPath>/.gitnexus/.gitignore. Uses fs.mkdir(..., { recursive: true }). Writes exactly '*\n'. Scoped to the local repo, not global ~/.gitnexus (separate path via getGlobalDir()). ✓

.git/info/exclude: ensureGitInfoExclude only runs when stat('.git').isDirectory() — correctly skips worktrees and non-git paths. Preserves existing content. Appends .gitnexus/ at most once (duplicate check on lines and trailing-slash forms). Idempotent. The mutation is local-only git metadata, not source-controlled. Acceptable. ✓

Idempotency: Writing *\n over *\n produces no observable diff. .git/info/exclude append is guarded. ✓

Git status cleanliness: Proven at the helper level (repo-manager.test.ts:126–141, real git init + execSync). Not proven for the full runFullAnalysis() path or the parent-git/subdir path. Partial.

Old root .gitignore cleanup policy: Not auto-removed (correct). No release note present yet.


Analyze/index path assessment

Full analyze (main path): ensureGitNexusIgnored(repoPath) is called at line 452–454, after saveMeta and registerRepo, inside the main try block, before AI context generation. Gated by if (hasGitDir(repoPath)) — this is the open issue. For direct git repos, works. For --skip-git / non-git paths / subfolders of parent repos, skipped.

Already-up-to-date fast path: ensureGitNexusIgnored(repoPath) called at line 169, unconditionally inside if (currentCommit !== ''). Non-git repos have currentCommit = '' and fall through to the full rebuild path. This is correct — the fast path only fires for git repos with a matching commit, and calling the helper there is right. ✓

Index command: ensureGitNexusIgnored(repoPath) called unconditionally at line 118 after registerRepo. No hasGitDir guard. Handles git repos, --allow-non-git paths, and cwd-discovered repos. ✓ This correctness is what makes the full-analyze inconsistency stand out.

Non-git / --skip-git behavior (full analyze): hasGitDir checks .git in repoPath only. Returns false for subfolders without own .git. Helper is skipped. Parent git repo sees .gitnexus/ as untracked. Open blocker.

Registry/meta/finalization impact: Zero. registerRepo, saveMeta, assertAnalysisFinalized, and assertSafeStoragePath are all untouched. ✓


Test assessment

repo-manager.test.ts (5 new tests): Creates .gitnexus/.gitignore with *\n ✓; preserves root .gitignore ✓; appends to .git/info/exclude ✓; .git/info/exclude idempotency ✓; .git-as-file (worktree) does not create exclude ✓; git status --short clean ✓. Strong real-git, real-fs coverage for the helper.

run-analyze.test.ts (1 new test): Already-up-to-date fast path → asserts alreadyUpToDate: true and .gitnexus/.gitignore content *\n. Real git repo. ✓ Missing: full analyze path and --skip-git/parent-git-subdir.

index-repo-command.test.ts (updated): Mocked; asserts ensureGitNexusIgnored called on success, not called on validation failures. ✓

cli-e2e.test.ts (2 new assertions): Root .gitignore absent, .gitnexus/.gitignore content *\n. Implies cleanliness but does not assert git status --short. Missing explicit status assertion and parent-git/subdir coverage.

Missing tests:

  1. Full runFullAnalysis() path: git status --short clean after .gitnexus/ created
  2. --skip-git on a subdirectory inside a parent git repo: git status --short from parent is empty

Would old behavior fail new tests? Yes — repo-manager.test.ts:79–84 asserts content *\n in .gitnexus/.gitignore; old addToGitignore wrote to root instead. The git status --short test would also fail on old code. ✓


Validation assessment

CI: Failing on current HEAD (5971d07). Run 25212892995 conclusion: failure. Blocks approval.

Typecheck: Cannot confirm for current HEAD until CI is fixed.

Focused tests: Correct selection in PR body. Cannot confirm for current HEAD.

Full test suite: PR body claims 254 files, 7505 tests locally — but that was for the first commit. The failing CI run covers the second commit which added ensureGitInfoExclude.

Docker/build: success. Not blocking.

CI blocks approval: Yes.


Compatibility and rollout assessment

Existing repos with old root .gitignore entry: Unaffected — the helper never reads or writes the root .gitignore. Old .gitnexus entries are left in place (correct policy). On next analyze/index, .gitnexus/.gitignore is created independently.

Existing .gitnexus storage: Fully compatible. No schema change. No meta format change. Storage created before this fix continues to work; next analyze adds .gitnexus/.gitignore.

User cleanup: Optional. The old root .gitignore entry is redundant once the internal gitignore exists — both applying is harmless. Users should be advised they may remove it.

Release note recommendation:

GitNexus no longer writes to your repository's root .gitignore. Generated index files are now kept untracked via .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it manually.


Final verdict

not production-ready

Two hard blockers prevent merge: (1) CI is failing on the current HEAD (5971d07), which introduced ensureGitInfoExclude — the exact step that is failing cannot be confirmed without job log access, but the failure conclusion is certain and the DoD requires CI green. (2) The full analyze path guards ensureGitNexusIgnored with if (hasGitDir(repoPath)), which means any use of gitnexus analyze --skip-git on a subdirectory inside a parent git repo creates .gitnexus/ without .gitnexus/.gitignore, leaving the parent repo's git status dirty — precisely the problem #1233 was filed to prevent. The index command has no such guard and handles this case correctly, making the inconsistency a behavioral regression on the most common --skip-git use case. The fix is a one-line removal of the if (hasGitDir) condition on the full analyze path (the ensureGitInfoExclude sub-helper already short-circuits safely for non-git paths). Beyond these two blockers, the root mutation elimination, idempotency, already-up-to-date path, .git/info/exclude behavior, and registry/meta integrity are all well-implemented and the helper-level test coverage is genuinely strong.


View CI failure · View PR branch ·

Ensure full analyze always writes the internal GitNexus ignore file so parent Git repositories stay clean for --skip-git subdirectory indexes.

Made-with: Cursor
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 3m 37s —— View job


Production-Readiness Review: PR #1248 — fix(cli): keep GitNexus ignores inside .gitnexus

Tasks:

  • Post initial comment
  • Read AGENTS.md, GUARDRAILS.md, DoD.md context files
  • Inspect repo-manager.tsensureGitNexusIgnored + ensureGitInfoExclude
  • Inspect run-analyze.ts — full analyze + fast path
  • Inspect index-repo.ts — index command path
  • Search for remaining addToGitignore / root .gitignore mutation references
  • Review test files (repo-manager, run-analyze, index-repo-command, cli-e2e, skip-git-cli)
  • Verify CI/Docker status
  • Compile findings and post final review

Review bar for this PR

  • No GitNexus analyze/index path modifies the analyzed repository's root .gitignore
  • .gitnexus/.gitignore is created with exactly *\n on every path that creates .gitnexus storage — including --skip-git subdirs inside parent git repos
  • Existing root .gitignore files remain byte-for-byte unchanged
  • Re-running analyze/index is idempotent and does not create noisy diffs
  • Already-up-to-date analyze ensures .gitnexus/.gitignore exists
  • Full analyze and index command use consistent ignore behavior
  • Parent-git/subdir --skip-git keeps the parent working tree clean (the core Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233 scenario)
  • CI, Docker, typecheck, focused tests, and relevant full tests are green

Issue and PR context considered

#1233 symptom: Every gitnexus analyze or gitnexus index run appended .gitnexus to the analyzed repo's root .gitignore, dirtying a source-controlled file without user consent and forcing users to either commit a GitNexus-specific line or use git update-index --skip-worktree.

Intended fix: Stop writing to root .gitignore entirely. Create .gitnexus/.gitignore with *, which causes Git to ignore all generated content inside .gitnexus/ (including the gitignore itself) without touching user-owned files. Additionally write .gitnexus/ to .git/info/exclude as a belt-and-suspenders measure for repos with a real .git directory.

Changed files: repo-manager.ts, run-analyze.ts, index-repo.ts, cli-e2e.test.ts, index-repo-command.test.ts, repo-manager.test.ts, run-analyze.test.ts, skip-git-cli.test.ts

CI/Docker status: Both workflows completed success (CI run 25214920613, Docker run 25214920552, both triggered by latest HEAD). 7739/7739 tests passed.

Earlier blockers:

  • hasGitDir guard on full analyze path — previously blocked merge. Resolved: line 452 of run-analyze.ts now calls await ensureGitNexusIgnored(repoPath) unconditionally, with no if (hasGitDir) wrapper.
  • Parent-git/subdir regression test — previously missing. Resolved: skip-git-cli.test.ts line 163 tests this scenario end-to-end.

Understanding of the change

What #1233 reported: addToGitignore() was called from both analyze and index, appending .gitnexus to the repo's root .gitignore on every run — dirtying a user-owned, source-controlled file.

Why undesirable: Modifying a project's .gitignore is hostile to users. It creates unwanted dirty state in every analyzed repo and forces developers to either commit a GitNexus-specific line into their team's shared .gitignore or apply a per-machine skip-worktree workaround. The tool should not mutate files outside its own storage.

How .gitnexus/.gitignore with * solves it: Git traverses into untracked directories looking for .gitignore files. A .gitignore inside a directory applies to all contents of that directory, so <repo>/.gitnexus/.gitignore containing * keeps all generated files (including the .gitignore itself) invisible to git status with zero changes to user-owned files.

Why .git/info/exclude was added: Belt-and-suspenders: .git/info/exclude is local, per-clone, non-source-controlled. Adding .gitnexus/ there makes the directory invisible to Git at the directory-listing stage, before Git even descends to read .gitnexus/.gitignore. The sub-helper (ensureGitInfoExclude) only runs when .git is a real directory — it short-circuits for linked worktrees where .git is a file, ensuring no mutation of wrong git metadata.

Paths changed: Full analyze (unconditional after registerRepo); already-up-to-date fast path (inside if (currentCommit !== ''), correct — non-git repos have currentCommit = '' and fall to full rebuild); index command (unconditional after registerRepo). Zero remaining calls to root .gitignore mutation anywhere in production code.

What remains unchanged: Registry, meta schema, assertAnalysisFinalized, assertSafeStoragePath, detect-changes, embeddings, AI context file generation — all untouched.


Findings

No production-readiness issues found against the current DoD bar.

Three low-severity observations follow, none of which block merge.


[low] ensure naming without "always-overwrite" comment

  • Category: Maintainability / intent documentation
  • Files: gitnexus/src/storage/repo-manager.ts:248
  • Issue: ensureGitNexusIgnored always writes *\n, overwriting any existing content without a content check. The ensure… naming implies idempotency-via-check, but the implementation is always-overwrite. Any user who manually edits .gitnexus/.gitignore for a legitimate purpose (e.g. un-ignoring a generated index to share with a team) will have their edit silently dropped on the next analyze. This is almost certainly the correct policy — .gitnexus is GitNexus-owned generated storage — but the intent is undocumented.
  • Recommended fix: Add one comment above the writeFile call: // Always overwrite: .gitnexus/ is generated storage owned by GitNexus.
  • Blocks merge: no

[low] E2E fixture assertion is brittle against future .gitignore addition

  • Category: Test design
  • Files: gitnexus/test/integration/cli-e2e.test.ts:201
  • Issue:
    expect(fs.existsSync(path.join(MINI_REPO, '.gitignore'))).toBe(false);
    This passes today because mini-repo has no root .gitignore. If the fixture legitimately gains one (e.g. to exclude IDE files), this test fails even if the behavior is correct. The intent is to prove analyze did not create or modify the root .gitignore, not that one cannot exist.
  • Recommended fix: Either record and compare before/after content, or add a comment: // mini-repo fixture has no root .gitignore; this asserts analyze did not create one.
  • Blocks merge: no

[low] No release note mentioning manual cleanup of old root .gitignore entries

  • Category: Compatibility / migration guidance
  • Files: N/A (by design — removing user file content is risky and correct to skip)
  • Issue: Repos that ran gitnexus analyze before this fix will have .gitnexus in their root .gitignore. The new behavior does not remove it (correct). Users are not informed.
  • Recommended fix: Add to release notes:

    GitNexus no longer writes to your repository's root .gitignore. Generated files are now kept untracked via .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it manually.

  • Blocks merge: no

Definition of Done check

DoD item Status
No analyze/index path modifies root .gitignore satisfied — zero addToGitignore references remain in production code; ignore-service.ts references are read-only (file discovery, not writes)
.gitnexus/.gitignore created with *\n on every path that creates .gitnexus storage satisfied — full analyze: unconditional at run-analyze.ts:452; fast path: unconditional inside if (currentCommit !== ''); index: unconditional at index-repo.ts:118
Existing root .gitignore files remain byte-for-byte unchanged satisfied — confirmed by code inspection and repo-manager.test.ts:87–94 (writes node_modules/\n to root, calls helper, asserts content unchanged)
Re-running analyze/index is idempotent satisfied*\n overwrites *\n (no diff); .git/info/exclude append is guarded by duplicate-entry check
Already-up-to-date analyze ensures .gitnexus/.gitignore exists satisfied — fast path at run-analyze.ts:169 calls helper before returning alreadyUpToDate: true
Full analyze and index command use consistent behavior satisfied — both call ensureGitNexusIgnored unconditionally after registerRepo; no hasGitDir guard on either path
Parent-git/subdir --skip-git keeps parent working tree clean satisfiedskip-git-cli.test.ts:163 runs real CLI from COOLIO/ inside a parent git repo, asserts COOLIO/.gitnexus/.gitignore === '*\n' and git status --short === '' from parent
git status --short stays clean after generated .gitnexus files are written satisfied — proven at helper level (repo-manager.test.ts:126–141, real git init + real storage files + execSync('git status --short') asserts '') and at CLI/integration level (skip-git-cli.test.ts:163)
CI, Docker, typecheck, focused tests, full suite green satisfied — CI run 25214920613: success (7739/7739); Docker run 25214920552: success

Storage / ignore behavior assessment

Root .gitignore mutation: Fully eliminated. addToGitignore has zero matches in the entire repository (confirmed via codebase search). ignore-service.ts references are reads-only, used for file discovery during pipeline ingestion. No production path writes to the root .gitignore.

.gitnexus/.gitignore creation: ensureGitNexusIgnored computes path.join(getStoragePath(repoPath), '.gitignore') = <repoPath>/.gitnexus/.gitignore. Creates the .gitnexus directory recursively before writing. Writes exactly '*\n'. Uses path.resolve internally via getStoragePath — handles relative paths and Windows paths correctly. Scoped to the local repo, not ~/.gitnexus (which uses getGlobalDir()process.env.GITNEXUS_HOME || os.homedir()/.gitnexus — entirely separate code path).

.git/info/exclude: ensureGitInfoExclude (private, called only from ensureGitNexusIgnored) stats .git before any write. If stat throws (no .git exists) or .git is not a directory (linked worktree case), it returns early — no mutation. For real git repos: reads existing content, splits by newline, strips comments, checks for both .gitnexus and .gitnexus/ forms before appending. Separator logic handles missing trailing newlines. This is acceptable local-only git metadata — it is per-clone and non-source-controlled, consistent with tools like git update-index --skip-worktree.

Idempotency: Writing *\n over *\n is a no-op from git's perspective. .git/info/exclude append is guarded by duplicate-entry check on both the bare ('.gitnexus') and trailing-slash ('.gitnexus/') forms. Both paths are safe to call on every analyze/index run.

Git status cleanliness: Git traverses into untracked directories to read .gitignore files. * in .gitnexus/.gitignore causes Git to ignore all contents of .gitnexus/, including the .gitignore itself. Belt-and-suspenders: .git/info/exclude prevents even the .gitnexus/ directory name from appearing in git status output before Git descends into it. Proven empirically by repo-manager.test.ts:126–141 and skip-git-cli.test.ts:163.

Old root .gitignore cleanup policy: Not auto-removed (correct). The helper never opens the root .gitignore path. Redundant old entries (# .gitnexus) are harmless — both the root entry and the internal gitignore applying simultaneously is a no-op. Manual cleanup guidance should be in release notes.


Analyze/index path assessment

Full analyze (run-analyze.ts): ensureGitNexusIgnored(repoPath) is called at line 452, after saveMeta and registerRepo, inside the main try block (before AI context generation, before closeLbug). No hasGitDir guard — the previous blocker is resolved. Works for: direct git repos, --skip-git on non-git paths, and --skip-git on subdirs inside parent git repos (the core #1233 scenario). The ensureGitInfoExclude sub-helper short-circuits safely when no .git directory is present, so unconditional calling is safe for all path types.

Already-up-to-date fast path: ensureGitNexusIgnored(repoPath) called at line 169, inside if (currentCommit !== ''). Non-git repos always have currentCommit = '' (because repoHasGit is falsecurrentCommit = '') and fall through to the full rebuild — so the fast path is only reachable for git repos, where calling the helper is exactly right.

Index command (index-repo.ts): ensureGitNexusIgnored(repoPath) called unconditionally at line 118, after registerRepo. Not called on early-exit paths (non-git without --allow-non-git, missing .gitnexus/, missing lbug, missing meta.json without --force). Consistent with full analyze behavior.

Non-git / --skip-git (full analyze): Works correctly. hasGitDir(repoPath) is still consulted at lines 161–162 only to determine repoHasGit / currentCommit for the fast-path check — not to gate the ignore helper. The ignore helper is called unconditionally regardless of that determination.

Parent-git/subdir behavior: The scenario parent/.git/ + parent/app/ with gitnexus analyze --skip-git on app/:

  1. hasGitDir('app/')false; currentCommit = ''
  2. No fast path; full pipeline runs
  3. app/.gitnexus/ is created
  4. await ensureGitNexusIgnored('app/') — creates app/.gitnexus/.gitignore with *\n
  5. ensureGitInfoExclude('app/')stat('app/.git') throws; returns early (no mutation to parent/.git/info/exclude)
  6. Parent git status --short is clean because app/.gitnexus/.gitignore covers all generated content

Registry/meta/finalization impact: Zero. registerRepo, saveMeta, assertAnalysisFinalized, and assertSafeStoragePath are all untouched. The new helper runs after registration — same ordering risk as the old addToGitignore, not a regression. If ensureGitNexusIgnored fails after registerRepo, the repo is in the registry but lacks .gitnexus/.gitignore; the next analyze run will recreate it.


Test assessment

repo-manager.test.ts (5 new tests, ensureGitNexusIgnored block): Real-filesystem, real-git coverage. Creates .gitnexus/.gitignore with *\n ✓; preserves root .gitignore ✓; appends to .git/info/exclude ✓; .git/info/exclude idempotency (two calls, single append) ✓; .git as a file (linked worktree) does not create exclude ✓; git status --short is '' after helper + written meta.json ✓. The createTempDir helper creates a plain temp dir (not a git repo), and individual tests opt into git init as needed — correct isolation.

run-analyze.test.ts (1 new test): Already-up-to-date fast path with real git repo and real saveMeta. Asserts alreadyUpToDate: true and .gitnexus/.gitignore === '*\n'. ✓

index-repo-command.test.ts (updated): Mocked; asserts mockEnsureGitNexusIgnored is called exactly once on successful register and not called on any early-exit path. Mocking is appropriate here since the helper has its own deep real-fs tests. ✓

cli-e2e.test.ts (2 new assertions): Asserts MINI_REPO/.gitignore does not exist and MINI_REPO/.gitnexus/.gitignore === '*\n' after full analyze. Covers the real CLI output path on the fixture repo. ✓ (minor brittleness if fixture gains a .gitignore — noted under low findings, non-blocking)

skip-git-cli.test.ts (new test at line 163): The critical regression test for the previously-open blocker. Uses real parent git init, commits baseline fixtures (including a root .gitignore), runs the built CLI from COOLIO/ with --skip-git and isolated GITNEXUS_HOME, then asserts:

  • COOLIO/.gitnexus/.gitignore === '*\n'
  • git status --short from parent is ''
  • Cleanup in finally block ✓
  • Cannot pass if .gitnexus/.gitignore is missing (the assertion would fail) ✓
  • The parent repo has a tracked root .gitignore in the commit, so any modification would surface in git status --short — the assertion is adversarially strong.

Would old behavior fail new tests? Yes. repo-manager.test.ts:79–84 asserts content *\n in .gitnexus/.gitignore; old addToGitignore wrote to root .gitignore instead — the file would be absent and the test would fail. The git status --short test would also fail: old behavior would write to root .gitignore, making it show up as modified.


Validation assessment

CI: Run 25214920613 — success. 7739 tests passed, 0 failed, 1 skipped (documented Ruby block parameter limitation). Typecheck (tsc --noEmit) passed as part of the CI pipeline. This run covers the current HEAD.

Docker Build & Push: Run 25214920552 — success.

Typecheck: Passed in CI. Import/export changes (ensureGitNexusIgnored added to exports, addToGitignore removed) are reflected across all importing files (run-analyze.ts, index-repo.ts, test files).

Focused tests: PR body correctly selects ensureGitNexusInternalGitignore|already-up-to-date|analyze command runs pipeline|registers successfully. All run in the green full suite.

Detect-changes smoke: Listed in PR body as npx tsx src/cli/index.ts detect-changes --repo GitNexus (low risk, 0 affected processes). Smoke only, not verified here — but detect-changes touches no storage or ignore logic.


Compatibility and rollout assessment

Existing repos with old root .gitignore entry: Unaffected. The new code never reads or writes the root .gitignore. Old .gitnexus entries remain until the user removes them manually. Both the root entry and the internal gitignore coexisting is harmless — git applies both (redundant, no conflict).

Existing .gitnexus storage: Fully compatible. No schema change. No meta format change. Next analyze (including fast path) or index run creates .gitnexus/.gitignore. No re-index required.

User cleanup: Optional. Users may safely remove any # .gitnexus line from their root .gitignore. No action required for correctness.

Recommended release note:

GitNexus no longer writes to your repository's root .gitignore. Generated index files are now kept untracked via .gitnexus/.gitignore. If you have a .gitnexus line in your root .gitignore from a prior run, you may safely remove it manually.


Final verdict

production-ready with minor follow-ups

All previously identified blockers are resolved: the hasGitDir guard on the full analyze path is removed (line 452 calls ensureGitNexusIgnored unconditionally), the parent-git/subdir regression test is solid and adversarially strong, CI and Docker both completed successfully on the current HEAD, and zero addToGitignore references remain anywhere in the codebase. The core correctness invariant — no root .gitignore mutation, .gitnexus/.gitignore created on every storage-creating path including --skip-git subdirs — is fully satisfied and covered by real-git, real-filesystem tests. The three remaining observations (always-overwrite intent comment, brittle E2E fixture assertion, release note) are polish items that do not affect correctness, safety, or test strength, and can be addressed in follow-up issues or as part of the next release.


View CI run · View PR branch

@magyargergo magyargergo merged commit 0418cbb into main May 1, 2026
20 checks passed
@magyargergo magyargergo deleted the fix/gitnexus-internal-gitignore-1233 branch May 1, 2026 15:46
evander-wang pushed a commit to evander-wang/GitNexus that referenced this pull request May 3, 2026
* fix(cli): keep GitNexus ignores inside .gitnexus

Avoid mutating analyzed repositories' root .gitignore while keeping generated GitNexus state untracked via .gitnexus/.gitignore.

Made-with: Cursor

* fix(cli): also use git info exclude for GitNexus storage

When an analyzed repo has a real .git directory, add .gitnexus/ to .git/info/exclude so local Git metadata ignores generated storage without touching root .gitignore.

Made-with: Cursor

* fix(cli): keep skip-git subdir indexes ignored

Ensure full analyze always writes the internal GitNexus ignore file so parent Git repositories stay clean for --skip-git subdirectory indexes.

Made-with: Cursor
magyargergo added a commit that referenced this pull request May 14, 2026
) (#1550)

* fix(cli): tolerate read-only workspace in ensureGitNexusIgnored

The documented Docker workflow mounts the host workspace at /workspace:ro
and runs `gitnexus index /workspace/<repo>` against an index produced by
a prior host-side `analyze`. Since PR #1248 ("keep GitNexus ignores
inside .gitnexus") the index command has called `ensureGitNexusIgnored`,
which unconditionally writes `<repo>/.gitnexus/.gitignore` and
`<repo>/.git/info/exclude` — both fail with EROFS on the :ro bind mount
even though the host already wrote the correct file during `analyze`.

Two complementary changes:

1. Idempotent fast path. Read the existing .gitnexus/.gitignore content
   first; if it already matches the desired value (`*\n`), skip the
   write entirely. This is the common case for the Docker workflow and
   avoids touching the FS at all.

2. EROFS/EACCES tolerance. When a write is genuinely needed but the FS
   refuses it, log a structured warning via the existing pino logger
   and continue. `registerRepo` runs before `ensureGitNexusIgnored` in
   `indexCommand`, so the global-registry write is already committed
   when we get here — letting the gitignore-write failure propagate
   leaves the user with a registered-but-error-exited command.

Three new unit tests pin the behaviour:
- idempotent re-call leaves mtime untouched
- ENOENT-then-correct path on a writable parent succeeds
- :ro parent (simulated via chmod 0o555) does not throw, on the
  already-correct fast path and on the cold-create path

Existing tests (61) still pass.

Closes #1549.

* test(storage): cover read-only ignore paths and tolerate EPERM (#1550)

- Add isReadOnlyFilesystemError helper including EPERM alongside EROFS/EACCES
  for ensureGitNexusIgnored and ensureGitInfoExclude (Windows parity with
  lbug-config / bridge-db patterns).
- Skip chmod-based read-only tests on win32 and uid 0; assert logger.warn
  on POSIX chmod denial for missing .gitignore.
- Add repo-manager-ensure-ignore-readonly.test.ts with vi.mock fs/promises
  delegating writeFile so EROFS/EACCES/EPERM rejections are asserted with
  structured log path and message for both .gitignore and .git/info/exclude.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hohaivu pushed a commit to hohaivu/GitNexus that referenced this pull request May 19, 2026
…higyanpatwari#1549) (abhigyanpatwari#1550)

* fix(cli): tolerate read-only workspace in ensureGitNexusIgnored

The documented Docker workflow mounts the host workspace at /workspace:ro
and runs `gitnexus index /workspace/<repo>` against an index produced by
a prior host-side `analyze`. Since PR abhigyanpatwari#1248 ("keep GitNexus ignores
inside .gitnexus") the index command has called `ensureGitNexusIgnored`,
which unconditionally writes `<repo>/.gitnexus/.gitignore` and
`<repo>/.git/info/exclude` — both fail with EROFS on the :ro bind mount
even though the host already wrote the correct file during `analyze`.

Two complementary changes:

1. Idempotent fast path. Read the existing .gitnexus/.gitignore content
   first; if it already matches the desired value (`*\n`), skip the
   write entirely. This is the common case for the Docker workflow and
   avoids touching the FS at all.

2. EROFS/EACCES tolerance. When a write is genuinely needed but the FS
   refuses it, log a structured warning via the existing pino logger
   and continue. `registerRepo` runs before `ensureGitNexusIgnored` in
   `indexCommand`, so the global-registry write is already committed
   when we get here — letting the gitignore-write failure propagate
   leaves the user with a registered-but-error-exited command.

Three new unit tests pin the behaviour:
- idempotent re-call leaves mtime untouched
- ENOENT-then-correct path on a writable parent succeeds
- :ro parent (simulated via chmod 0o555) does not throw, on the
  already-correct fast path and on the cold-create path

Existing tests (61) still pass.

Closes abhigyanpatwari#1549.

* test(storage): cover read-only ignore paths and tolerate EPERM (abhigyanpatwari#1550)

- Add isReadOnlyFilesystemError helper including EPERM alongside EROFS/EACCES
  for ensureGitNexusIgnored and ensureGitInfoExclude (Windows parity with
  lbug-config / bridge-db patterns).
- Skip chmod-based read-only tests on win32 and uid 0; assert logger.warn
  on POSIX chmod denial for missing .gitignore.
- Add repo-manager-ensure-ignore-readonly.test.ts with vi.mock fs/promises
  delegating writeFile so EROFS/EACCES/EPERM rejections are asserted with
  structured log path and message for both .gitignore and .git/info/exclude.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead

1 participant