From 41a6345e643f424098b652bbd407bccfa77daf4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9on=20=22Avic=22=20Simmons?= Date: Wed, 13 May 2026 10:45:06 -0400 Subject: [PATCH 1/3] fix(cli): tolerate read-only workspace in ensureGitNexusIgnored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documented Docker workflow mounts the host workspace at /workspace:ro and runs `gitnexus index /workspace/` 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 `/.gitnexus/.gitignore` and `/.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. --- gitnexus/src/storage/repo-manager.ts | 46 ++++++++++++++++++++--- gitnexus/test/unit/repo-manager.test.ts | 49 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 456a6c1435..06783d4706 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -11,6 +11,7 @@ import { realpathSync } from 'fs'; import path from 'path'; import os from 'os'; import { getInferredRepoName, resolveRepoIdentityRoot } from './git.js'; +import { logger } from '../core/logger.js'; /** * Normalise a repo path for registry comparison across platforms @@ -286,9 +287,34 @@ export const findRepo = async (startPath: string): Promise = */ export const ensureGitNexusIgnored = async (repoPath: string): Promise => { const gitignorePath = path.join(getStoragePath(repoPath), '.gitignore'); + const desired = '*\n'; - await fs.mkdir(path.dirname(gitignorePath), { recursive: true }); - await fs.writeFile(gitignorePath, '*\n', 'utf-8'); + // Idempotent fast path: skip the write entirely when the file already has + // the expected content. Lets this run cleanly on read-only mounts (e.g. + // the documented Docker workflow with WORKSPACE_DIR bound :ro) when an + // earlier `analyze` already created the file. See issue #1549. + try { + if ((await fs.readFile(gitignorePath, 'utf-8')) === desired) { + await ensureGitInfoExclude(repoPath); + return; + } + } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; + } + + try { + await fs.mkdir(path.dirname(gitignorePath), { recursive: true }); + await fs.writeFile(gitignorePath, desired, 'utf-8'); + } catch (err: any) { + if (err?.code === 'EROFS' || err?.code === 'EACCES') { + logger.warn( + { path: gitignorePath, code: err.code }, + 'GitNexus storage filesystem is not writable; skipping .gitnexus/.gitignore. Generated files may appear as untracked in this repo locally.', + ); + } else { + throw err; + } + } await ensureGitInfoExclude(repoPath); }; @@ -304,8 +330,6 @@ const ensureGitInfoExclude = async (repoPath: string): Promise => { return; } - await fs.mkdir(path.dirname(excludePath), { recursive: true }); - let content = ''; try { content = await fs.readFile(excludePath, 'utf-8'); @@ -320,7 +344,19 @@ const ensureGitInfoExclude = async (repoPath: string): Promise => { if (excludes.includes(GITNEXUS_DIR) || excludes.includes(GITNEXUS_EXCLUDE_ENTRY)) return; const separator = content.length === 0 || content.endsWith('\n') ? '' : '\n'; - await fs.writeFile(excludePath, `${content}${separator}${GITNEXUS_EXCLUDE_ENTRY}\n`, 'utf-8'); + try { + await fs.mkdir(path.dirname(excludePath), { recursive: true }); + await fs.writeFile(excludePath, `${content}${separator}${GITNEXUS_EXCLUDE_ENTRY}\n`, 'utf-8'); + } catch (err: any) { + if (err?.code === 'EROFS' || err?.code === 'EACCES') { + logger.warn( + { path: excludePath, code: err.code }, + 'GitNexus storage filesystem is not writable; skipping .git/info/exclude update. .gitnexus/ may appear as untracked in `git status` locally.', + ); + } else { + throw err; + } + } }; // ─── Global Registry (~/.gitnexus/registry.json) ─────────────────────── diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index bbd0fd50eb..9e1c12f626 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -139,6 +139,55 @@ describe('ensureGitNexusIgnored (#1233)', () => { }); expect(status).toBe(''); }); + + // ─ Read-only workspace tolerance (#1549) ──────────────────────────── + // The documented Docker workflow mounts the host workspace at /workspace:ro + // and runs `gitnexus index /workspace/`. The host has already created + // the .gitnexus dir during a prior `analyze`, so the gitignore file already + // exists with the correct content — there's no real work to do. The tests + // below pin two pieces of behaviour that make that workflow work: + // (a) the function short-circuits when the file is already correct + // (no write attempt, no mtime bump); + // (b) when a write *is* needed but the FS is not writable + // (EROFS / EACCES), the function logs and continues instead of + // throwing — so the caller's `registerRepo` work stays committed. + + it('does not re-write .gitnexus/.gitignore when it already has the desired content', async () => { + await ensureGitNexusIgnored(tmpRepo.dbPath); + const gitignorePath = path.join(tmpRepo.dbPath, '.gitnexus', '.gitignore'); + const before = await fs.stat(gitignorePath); + + await new Promise((resolve) => setTimeout(resolve, 25)); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + + const after = await fs.stat(gitignorePath); + expect(after.mtimeMs).toBe(before.mtimeMs); + }); + + it('does not throw when .gitnexus/.gitignore is already correct and the storage dir is read-only', async () => { + await ensureGitNexusIgnored(tmpRepo.dbPath); + const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); + + await fs.chmod(storagePath, 0o555); + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + } finally { + await fs.chmod(storagePath, 0o755); + } + }); + + it('warns and continues when the storage dir is read-only and the file does not yet exist', async () => { + const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); + await fs.mkdir(storagePath, { recursive: true }); + await fs.chmod(storagePath, 0o555); + + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + } finally { + await fs.chmod(storagePath, 0o755); + } + }); }); // ─── readRegistry ──────────────────────────────────────────────────── From 3117f701782465f7be4f279b2c326f7728c54dc7 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 14 May 2026 09:19:19 +0100 Subject: [PATCH 2/3] 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 --- gitnexus/src/storage/repo-manager.ts | 9 +- ...epo-manager-ensure-ignore-readonly.test.ts | 109 ++++++++++++++++++ gitnexus/test/unit/repo-manager.test.ts | 64 ++++++---- 3 files changed, 158 insertions(+), 24 deletions(-) create mode 100644 gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 06783d4706..44446e0ec0 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -282,6 +282,11 @@ export const findRepo = async (startPath: string): Promise = return null; }; +function isReadOnlyFilesystemError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException)?.code; + return code === 'EROFS' || code === 'EACCES' || code === 'EPERM'; +} + /** * Keep generated index files ignored without modifying the user's root .gitignore. */ @@ -306,7 +311,7 @@ export const ensureGitNexusIgnored = async (repoPath: string): Promise => await fs.mkdir(path.dirname(gitignorePath), { recursive: true }); await fs.writeFile(gitignorePath, desired, 'utf-8'); } catch (err: any) { - if (err?.code === 'EROFS' || err?.code === 'EACCES') { + if (isReadOnlyFilesystemError(err)) { logger.warn( { path: gitignorePath, code: err.code }, 'GitNexus storage filesystem is not writable; skipping .gitnexus/.gitignore. Generated files may appear as untracked in this repo locally.', @@ -348,7 +353,7 @@ const ensureGitInfoExclude = async (repoPath: string): Promise => { await fs.mkdir(path.dirname(excludePath), { recursive: true }); await fs.writeFile(excludePath, `${content}${separator}${GITNEXUS_EXCLUDE_ENTRY}\n`, 'utf-8'); } catch (err: any) { - if (err?.code === 'EROFS' || err?.code === 'EACCES') { + if (isReadOnlyFilesystemError(err)) { logger.warn( { path: excludePath, code: err.code }, 'GitNexus storage filesystem is not writable; skipping .git/info/exclude update. .gitnexus/ may appear as untracked in `git status` locally.', diff --git a/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts b/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts new file mode 100644 index 0000000000..8f187f6b7a --- /dev/null +++ b/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts @@ -0,0 +1,109 @@ +/** + * Read-only / permission-denied write paths for ensureGitNexusIgnored (#1549, PR #1550). + * Separate from repo-manager.test.ts: Vitest cannot vi.spyOn ESM namespace exports of + * fs/promises; a delegating vi.mock is required for cross-platform mock rejects. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import path from 'path'; + +const fswCtx = vi.hoisted(() => ({ + writeFileMock: vi.fn(), + realWrite: null as ((...args: unknown[]) => Promise) | null, +})); + +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const d = actual.default; + fswCtx.realWrite = d.writeFile.bind(d); + fswCtx.writeFileMock.mockImplementation((...args) => fswCtx.realWrite!(...args)); + return { + default: new Proxy(d, { + get(target, prop) { + if (prop === 'writeFile') return fswCtx.writeFileMock; + const v = Reflect.get(target, prop, target) as unknown; + return typeof v === 'function' ? (v as (...args: unknown[]) => unknown).bind(target) : v; + }, + }), + }; +}); + +import fs from 'fs/promises'; +import { ensureGitNexusIgnored } from '../../src/storage/repo-manager.js'; +import { _captureLogger } from '../../src/core/logger.js'; +import { createTempDir } from '../helpers/test-db.js'; + +const samePath = (a: string, b: string) => path.normalize(a) === path.normalize(b); + +describe('ensureGitNexusIgnored — mocked writeFile (EROFS / EACCES / EPERM)', () => { + let tmpRepo: Awaited>; + + beforeEach(async () => { + tmpRepo = await createTempDir('gitnexus-ro-ignore-mock-'); + fswCtx.writeFileMock.mockClear(); + fswCtx.writeFileMock.mockImplementation((...args) => fswCtx.realWrite!(...args)); + }); + + afterEach(async () => { + await tmpRepo.cleanup(); + }); + + it.each(['EROFS', 'EACCES', 'EPERM'] as const)( + 'tolerates %s on .git/info/exclude write and logs a warn', + async (code) => { + const gitignorePath = path.join(tmpRepo.dbPath, '.gitnexus', '.gitignore'); + await fs.mkdir(path.dirname(gitignorePath), { recursive: true }); + await fs.writeFile(gitignorePath, '*\n', 'utf-8'); + + const excludePath = path.join(tmpRepo.dbPath, '.git', 'info', 'exclude'); + await fs.mkdir(path.dirname(excludePath), { recursive: true }); + await fs.writeFile(excludePath, '# empty\n', 'utf-8'); + + const cap = _captureLogger(); + fswCtx.writeFileMock.mockRejectedValueOnce(Object.assign(new Error('mock ro'), { code })); + + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + expect(fswCtx.writeFileMock).toHaveBeenCalled(); + expect( + cap.records().some( + (r) => + r.level === 40 && + r.code === code && + typeof r.path === 'string' && + samePath(String(r.path), excludePath) && + String(r.msg ?? '').includes('.git/info/exclude'), + ), + ).toBe(true); + } finally { + cap.restore(); + } + }, + ); + + it.each(['EROFS', 'EACCES', 'EPERM'] as const)( + 'tolerates %s on .gitnexus/.gitignore write and logs a warn', + async (code) => { + const cap = _captureLogger(); + const gitignorePath = path.join(tmpRepo.dbPath, '.gitnexus', '.gitignore'); + + fswCtx.writeFileMock.mockRejectedValueOnce(Object.assign(new Error('mock ro'), { code })); + + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + expect(fswCtx.writeFileMock).toHaveBeenCalled(); + expect( + cap.records().some( + (r) => + r.level === 40 && + r.code === code && + typeof r.path === 'string' && + samePath(String(r.path), gitignorePath) && + String(r.msg ?? '').includes('.gitnexus/.gitignore'), + ), + ).toBe(true); + } finally { + cap.restore(); + } + }, + ); +}); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 9e1c12f626..22d8e5f232 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -4,10 +4,11 @@ * Tests: getStoragePath, getStoragePaths, readRegistry, registerRepo, unregisterRepo * Covers hardening fixes #29 (API key file permissions) and #30 (case-insensitive paths on Windows) */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'path'; import os from 'os'; import fs from 'fs/promises'; +import { _captureLogger } from '../../src/core/logger.js'; import { getStoragePath, getStoragePaths, @@ -73,6 +74,7 @@ describe('ensureGitNexusIgnored (#1233)', () => { }); afterEach(async () => { + vi.restoreAllMocks(); await tmpRepo.cleanup(); }); @@ -149,7 +151,7 @@ describe('ensureGitNexusIgnored (#1233)', () => { // (a) the function short-circuits when the file is already correct // (no write attempt, no mtime bump); // (b) when a write *is* needed but the FS is not writable - // (EROFS / EACCES), the function logs and continues instead of + // (EROFS / EACCES / EPERM), the function logs and continues instead of // throwing — so the caller's `registerRepo` work stays committed. it('does not re-write .gitnexus/.gitignore when it already has the desired content', async () => { @@ -165,29 +167,47 @@ describe('ensureGitNexusIgnored (#1233)', () => { expect(after.mtimeMs).toBe(before.mtimeMs); }); - it('does not throw when .gitnexus/.gitignore is already correct and the storage dir is read-only', async () => { - await ensureGitNexusIgnored(tmpRepo.dbPath); - const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'does not throw when .gitnexus/.gitignore is already correct and the storage dir is read-only', + async () => { + await ensureGitNexusIgnored(tmpRepo.dbPath); + const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); - await fs.chmod(storagePath, 0o555); - try { - await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); - } finally { - await fs.chmod(storagePath, 0o755); - } - }); + await fs.chmod(storagePath, 0o555); + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + } finally { + await fs.chmod(storagePath, 0o755); + } + }, + ); - it('warns and continues when the storage dir is read-only and the file does not yet exist', async () => { - const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); - await fs.mkdir(storagePath, { recursive: true }); - await fs.chmod(storagePath, 0o555); + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'warns and continues when the storage dir is read-only and the file does not yet exist', + async () => { + const storagePath = path.join(tmpRepo.dbPath, '.gitnexus'); + await fs.mkdir(storagePath, { recursive: true }); + await fs.chmod(storagePath, 0o555); + + const cap = _captureLogger(); + try { + await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); + expect( + cap.records().some( + (r) => + r.level === 40 && + (r.code === 'EACCES' || r.code === 'EPERM') && + String(r.msg ?? '').includes('.gitnexus/.gitignore') && + String(r.path ?? '').includes('.gitnexus'), + ), + ).toBe(true); + } finally { + cap.restore(); + await fs.chmod(storagePath, 0o755); + } + }, + ); - try { - await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); - } finally { - await fs.chmod(storagePath, 0o755); - } - }); }); // ─── readRegistry ──────────────────────────────────────────────────── From 43d864e9ce54b3db8b9a754b8561ed2ff18a88ba Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 14 May 2026 08:38:38 +0000 Subject: [PATCH 3/3] chore(autofix): apply prettier + eslint fixes via /autofix command --- ...epo-manager-ensure-ignore-readonly.test.ts | 36 ++++++++++--------- gitnexus/test/unit/repo-manager.test.ts | 17 ++++----- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts b/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts index 8f187f6b7a..8e49a01f35 100644 --- a/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts +++ b/gitnexus/test/unit/repo-manager-ensure-ignore-readonly.test.ts @@ -65,14 +65,16 @@ describe('ensureGitNexusIgnored — mocked writeFile (EROFS / EACCES / EPERM)', await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); expect(fswCtx.writeFileMock).toHaveBeenCalled(); expect( - cap.records().some( - (r) => - r.level === 40 && - r.code === code && - typeof r.path === 'string' && - samePath(String(r.path), excludePath) && - String(r.msg ?? '').includes('.git/info/exclude'), - ), + cap + .records() + .some( + (r) => + r.level === 40 && + r.code === code && + typeof r.path === 'string' && + samePath(String(r.path), excludePath) && + String(r.msg ?? '').includes('.git/info/exclude'), + ), ).toBe(true); } finally { cap.restore(); @@ -92,14 +94,16 @@ describe('ensureGitNexusIgnored — mocked writeFile (EROFS / EACCES / EPERM)', await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); expect(fswCtx.writeFileMock).toHaveBeenCalled(); expect( - cap.records().some( - (r) => - r.level === 40 && - r.code === code && - typeof r.path === 'string' && - samePath(String(r.path), gitignorePath) && - String(r.msg ?? '').includes('.gitnexus/.gitignore'), - ), + cap + .records() + .some( + (r) => + r.level === 40 && + r.code === code && + typeof r.path === 'string' && + samePath(String(r.path), gitignorePath) && + String(r.msg ?? '').includes('.gitnexus/.gitignore'), + ), ).toBe(true); } finally { cap.restore(); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 22d8e5f232..d3dd65a278 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -193,13 +193,15 @@ describe('ensureGitNexusIgnored (#1233)', () => { try { await expect(ensureGitNexusIgnored(tmpRepo.dbPath)).resolves.not.toThrow(); expect( - cap.records().some( - (r) => - r.level === 40 && - (r.code === 'EACCES' || r.code === 'EPERM') && - String(r.msg ?? '').includes('.gitnexus/.gitignore') && - String(r.path ?? '').includes('.gitnexus'), - ), + cap + .records() + .some( + (r) => + r.level === 40 && + (r.code === 'EACCES' || r.code === 'EPERM') && + String(r.msg ?? '').includes('.gitnexus/.gitignore') && + String(r.path ?? '').includes('.gitnexus'), + ), ).toBe(true); } finally { cap.restore(); @@ -207,7 +209,6 @@ describe('ensureGitNexusIgnored (#1233)', () => { } }, ); - }); // ─── readRegistry ────────────────────────────────────────────────────