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

[Avicennasis]

Summary

Fixes the documented Docker workflow described in issue #1549: gitnexus index /workspace/<repo> against a :ro bind mount (per .env.example "Optional read-only mount" + the README example) currently fails with EROFS: read-only file system, open '/workspace/.gitnexus/.gitignore'.

Root cause is a regression introduced by PR #1248 ("fix(cli): keep GitNexus ignores inside .gitnexus"). That PR — intentionally and correctly — moved the gitignore write out of the user's root .gitignore and into <repo>/.gitnexus/.gitignore. The new code unconditionally writes that path inside the workspace, which fails on a read-only bind mount even when the host has already created an identical file during a prior analyze.

What this PR does

Two complementary changes inside ensureGitNexusIgnored and ensureGitInfoExclude:

1. Idempotent fast path — read the existing .gitnexus/.gitignore content first; if it already matches the desired value (*\n), skip the write entirely. This covers the common Docker workflow case (host did analyze, container does index).

2. EROFS/EACCES tolerance — when a write is genuinely needed but the filesystem refuses it, log a structured logger.warn(...) 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 failure propagate leaves the user with a registered-but-error-exited command (visible in /api/repos, but gitnexus index exits non-zero — confusing).

Both code paths still write on a normal writable filesystem and still produce the same content as before. Behaviour only changes for the read-only / permission-denied case.

Test plan

• cd gitnexus && npx tsc --noEmit — clean.
• cd gitnexus && npx vitest run test/unit/repo-manager.test.ts — PASS (61) FAIL (0). Three new tests cover idempotent fast-path and :ro parent dir (simulated via chmod 0o555).
• cd gitnexus && npx vitest run test/unit/index-repo-command.test.ts — PASS (11) FAIL (0).
• cd gitnexus && npx vitest run test/unit/run-analyze.test.ts — PASS (18) FAIL (0).
• npx prettier --check gitnexus/src/storage/repo-manager.ts gitnexus/test/unit/repo-manager.test.ts — clean.

Reproducer for the Docker workflow itself (verified before/after the fix):

# Before this PR — fails with EROFS:
rm -rf .gitnexus ~/.gitnexus
docker compose down -v
npx -y gitnexus@1.6.5-rc.19 analyze
WORKSPACE_DIR="$PWD" SERVER_IMAGE=ghcr.io/abhigyanpatwari/gitnexus:1.6.5-rc.19 \
  docker compose up -d
docker compose exec gitnexus-server node /app/gitnexus/dist/cli/index.js index /workspace
# → Error: EROFS: read-only file system,
#   open '/workspace/.gitnexus/.gitignore'

# After this PR (built locally from the branch), the same command exits 0
# and the repo is registered cleanly in the global registry inside the
# named volume.

Notes

• The companion gitnexus PATH issue from Documented Docker workflow (gitnexus index /workspace/<repo> on :ro mount) is broken in two places #1549 is the subject of a separate, smaller PR — I kept the two changes apart so each can be reviewed/reverted on its own.
• No public CLI/MCP contract changes. The behaviour for writable workspaces (the host-side analyze workflow) is byte-identical to before.

Closes #1549 (in conjunction with the Dockerfile.cli PATH PR).

[vercel]

@Avicennasis is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

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
9031	9030	0	1	440s

✅ All 9030 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	78.18%	28430/36361	N/A%	—	🟢 ███████████████░░░░░
Branches	66.55%	18021/27078	N/A%	—	🟢 █████████████░░░░░░░
Functions	83.02%	2857/3441	N/A%	—	🟢 ████████████████░░░░
Lines	81.47%	25678/31515	N/A%	—	🟢 ████████████████░░░░

---

_{📋 View full run · Generated by CI}

[github-actions]

Claude finished @magyargergo's task in 5m 46s —— View job

---

Adversarial Production-Readiness Review — PR #1550

Todo list:

• Post initial comment
• Read required repo context files (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, ARCHITECTURE.md, DoD.md, TESTING.md)
• Run branch hygiene git commands
• Read changed files (repo-manager.ts, repo-manager.test.ts)
• Read related files (index-repo.ts, run-analyze.ts, logger.ts, Dockerfile.cli, docker-compose.yaml, .env.example)
• Search for relevant terms (ensureGitNexusIgnored, EROFS, EACCES, EPERM, etc.)
• Perform Lane A–G analysis
• Post final review

---

1. Review Bar for This PR

Reviewed against:

• Repo DoD.md (v2.0.0, last reviewed 2026-04-23) — §2.7 Tests, §2.8 Observability, §2.5 Security
• GUARDRAILS.md — scope, escalation
• CONTRIBUTING.md — PR format, validation baseline
• Trigger comment's PR-specific DoD — all seven lanes

TESTING.md: Present and read. No platform-specific skip policy for chmod tests was found there.

---

2. Current PR State

Item	Status
PR branch	Avicennasis:fix/index-tolerate-readonly-workspace → main
Effective changed files	gitnexus/src/storage/repo-manager.ts, gitnexus/test/unit/repo-manager.test.ts
CI	All checks passed (8,846 tests, 1 skipped) on Linux/macOS/Windows
Typecheck	Clean
Companion PR #1551	Merged — gitnexus symlinked on $PATH in runtime image
Vercel	Requires team authorization — irrelevant to this CLI/storage-only change

---

3. Branch Hygiene Assessment

Verdict: merge-from-main commit present but harmless and merge-safe

The PR contains two merge-from-main commits:

• 9812d38 — merged 507f84b (Docker PATH fix, fix(docker): symlink gitnexus binary onto $PATH in runtime image (#1549) #1551) and 38ff736 (ca-certificates fix)
• 38ff351 — merged 88d3df7 (wiki --timeout/--retries flags)

Running git show --name-status 41a6345 confirmed the fix commit touches only the two intended files. The diff of 41a6345..HEAD shows 6 files from the merge commits (Dockerfile.cli, README.md, wiki.ts, etc.), but these are all already present on main; the effective merge diff versus origin/main is restricted to repo-manager.ts and repo-manager.test.ts.

No whitespace errors (git diff --check clean). The wiki commit (88d3df7) carries an unconventional PR title format (feat:(wiki) should be feat(wiki)) but that is a pre-existing issue on main, not introduced by this PR.

---

4. Understanding of the Change

Two complementary changes in ensureGitNexusIgnored and ensureGitInfoExclude (repo-manager.ts:288–360):

1. Idempotent fast path (lines 296–303): Read .gitnexus/.gitignore; if content already equals *\n, skip the write and proceed to ensureGitInfoExclude. ENOENT is treated as "file absent, proceed to write path"; any other read error (e.g. EACCES on an unreadable file) is re-thrown — this is intentional.
2. EROFS/EACCES write tolerance (lines 305–317 and 347–358): Wrap the mkdir+writeFile call for both .gitnexus/.gitignore and the .git/info/exclude append in a narrowed catch. Only EROFS and EACCES are downgraded to logger.warn; all other error codes re-throw.
3. Logger import (line 14): import { logger } from '../core/logger.js' — no circular dependency (verified: logger.ts does not import from repo-manager.ts).

indexCommand call order (index-repo.ts:117–118): registerRepo first, then ensureGitNexusIgnored. This is correct — registry mutation is committed before the best-effort ignore writes.

---

5. Findings

Finding 1 — ensureGitInfoExclude write-failure path is untested

Severity	High
File/line	gitnexus/src/storage/repo-manager.ts:347–358, gitnexus/test/unit/repo-manager.test.ts:155–191
Risk	The three new tests (lines 155–191) never exercise the EROFS/EACCES catch in ensureGitInfoExclude. All new tests either omit a .git directory entirely (causing ensureGitInfoExclude to return early at the stat check, line 327–331) or use the fast path (file already correct). The new code at lines 347–358 is reachable only when .git is a real directory, .git/info/exclude needs to be created/updated, AND the write fails. No test covers that path.
Evidence	grep -n 'ensureGitInfoExclude' gitnexus/test → 0 matches. Tests at lines 168–190 chmod 0o555 the .gitnexus storage dir, which does not affect .git/info/exclude reachability. Lines 96–116 test the happy-path .git/info/exclude write only.
Recommended fix	Add a test that: (a) creates a real .git/info/exclude path; (b) makes .git/info or the target file unwritable (or mocks fs.writeFile to reject with { code: 'EROFS' }); (c) asserts not.toThrow() AND that logger.warn was called with the expected path and code. _captureLogger() from gitnexus/src/core/logger.ts is already available for assertion.
Blocks merge	Yes — DoD.md §2.7: "Tests cover the real changed path — they would fail if behavior, wiring, or contracts were broken." The EROFS/EACCES tolerance in ensureGitInfoExclude is a changed path with no test that would catch a regression. The trigger prompt's PR-specific DoD also explicitly requires: "Unit tests must cover ensureGitInfoExclude read-only or permission-denied write path."

---

Finding 2 — EPERM not in the tolerance list

Severity	Medium
File/line	repo-manager.ts:309, repo-manager.ts:351
Risk	Both catch blocks gate on err?.code === 'EROFS' || err?.code === 'EACCES'. Node.js on Windows uses EPERM for many write-denied scenarios (e.g. writing to a file marked read-only via icacls, or a volume mounted with a read-only driver). The rest of this codebase already handles EPERM in analogous contexts: lbug-config.ts:140 has const HANDLE_RELEASE_LOCK_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']) and bridge-db.ts:261 has const RETRY_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']). A Windows user whose workspace is on a read-only share would receive an unhandled EPERM thrown from ensureGitNexusIgnored, causing gitnexus index to exit non-zero after a successful registerRepo — the exact "registered-but-error-exited" state this PR is meant to fix.
Evidence	grep -n 'EPERM' gitnexus/src/storage/repo-manager.ts → 0 matches. grep -n 'EPERM' gitnexus/src/core/lbug/lbug-config.ts → line 140. The Docker workflow is Linux-only, but gitnexus index is also documented as a host-side command.
Recommended fix	Add || err?.code === 'EPERM' to both catch conditions (lines 309 and 351). Add a unit test that injects { code: 'EPERM' } via vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(...) to assert the path is tolerated.
Blocks merge	Maybe — directly blocks if Windows CLI support is a shipping requirement. The documented Docker use case is Linux-only, but the CLI is cross-platform (CI runs on Windows). Given the established pattern in this codebase of handling EPERM alongside EACCES, omitting it here looks like an accidental gap rather than a deliberate decision.

---

Finding 3 — logger.warn call not asserted in tests

Severity	Low
File/line	repo-manager.test.ts:180–190
Risk	The test titled "warns and continues when the storage dir is read-only and the file does not yet exist" only asserts resolves.not.toThrow(). It does not assert that logger.warn was called, the call's payload shape, or the path/code fields. If the catch block were accidentally changed to silently return (dropping the warn), this test would still pass. The warning is part of the stated PR contract and is the user's only signal that a write was skipped.
Evidence	grep -n '_captureLogger|logger|warn' gitnexus/test/unit/repo-manager.test.ts → line 180 contains only the description string "warns"; no import of _captureLogger and no assertion on warn calls.
Recommended fix	Import _captureLogger from logger.ts; add a beforeEach/afterEach capture; in the read-only test assert cap.records().some(r => r.msg?.includes('.gitnexus/.gitignore') && r.level === 40) (pino level 40 = warn). The same pattern is used in logger.test.ts.
Blocks merge	No — but this weakens test fidelity and the PR's stated contract. Recommend as a follow-up or part of the same commit.

---

Finding 4 — chmod 0o555 tests do not verify the catch branch on Windows or under root

Severity	Low
File/line	repo-manager.test.ts:172, repo-manager.test.ts:183
Risk	chmod 0o555 is a no-op on Windows (fs.chmod silently succeeds but the write is still allowed). This means the catch block in ensureGitNexusIgnored is never hit on Windows CI; both tests pass because the write succeeds, not because the tolerance works. Similarly, processes running as root on Linux ignore directory mode bits and writes succeed through chmod-denied paths. The Windows CI run in this PR's CI report ("all checks passed") confirms the tests pass, but doesn't confirm the catch branch fires on Windows.
Evidence	CI says tests pass on all 3 platforms. On Windows, chmod 0o555 does not actually deny writes; Node.js fs.writeFile succeeds regardless.
Recommended fix	Keep the chmod test as an integration-style sanity check on Linux/macOS (add skipIf(process.platform === 'win32') to avoid false confidence). Add a complementary mock-based test using vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(Object.assign(new Error('read-only'), { code: 'EROFS' })) to force the catch branch in a platform-neutral way. This is the only reliable cross-platform method short of actual bind mounts.
Blocks merge	Maybe — the fix works on Linux (the Docker target), but the Windows catch branch is untested. If EPERM is also added (Finding 2), a mock-based test is required to verify it, making this blocking in conjunction with Finding 2.

---

6. PR-Specific Assessment Sections

Storage ignore behavior (Lane A)

PASS with caveat (EPERM). ensureGitNexusIgnored correctly reads before writing, short-circuits on exact content match *\n, handles ENOENT by falling through to the write path, and calls ensureGitInfoExclude on both code paths. The mkdir call is inside the same try block as writeFile, so an EROFS on mkdir (cold :ro mount) is also tolerated. Non-ENOENT read errors correctly re-throw. EROFS/EACCES are narrowly scoped to the two best-effort ignore files only. EPERM is absent (see Finding 2). No parent directory creation occurs before the fast-path read.

Git info exclude behavior (Lane B)

PASS for logic, FAIL for test coverage. ensureGitInfoExclude correctly returns early when .git does not exist or is a file (worktree). Preserves existing content, avoids duplicates (checks both .gitnexus and .gitnexus/), handles correct newline termination. Read failures on non-ENOENT re-throw (intentional). EROFS/EACCES write tolerance is implemented correctly. Missing test for the write-failure path (see Finding 1).

CLI/index transaction semantics (Lane C)

PASS. registerRepo is called at index-repo.ts:117 before ensureGitNexusIgnored at line 118. The fix ensures that a EROFS/EACCES failure in ensureGitNexusIgnored no longer propagates — the command exits 0 and the registry entry is coherent. Real errors (missing .gitnexus/, missing lbug, meta load failure, registerRepo throws) are not downgraded. GITNEXUS_HOME=/data/gitnexus in Dockerfile.cli:73 correctly routes global registry writes to the container's writable volume, not the :ro workspace. The documented workflow is coherent after #1551 (PATH fix) is included.

Cross-platform filesystem semantics (Lane D)

CONDITIONAL PASS. Linux :ro bind mounts produce EROFS — handled. Normal permission denial produces EACCES — handled. EPERM is not handled — Windows CLI users with read-only workspaces would still get a thrown error (Finding 2). chmod 0o555 tests work on Linux/macOS but are no-ops on Windows (Finding 4). The Docker use case is Linux-only and works correctly with the current fix.

Tests and CI fidelity (Lane F)

NOT FULLY SATISFACTORY.

• ✅ Idempotent fast-path: mtime preservation test (lines 155–166) — good.
• ✅ Cold write on read-only dir (lines 180–190) — exercises the EACCES catch in practice on Linux/macOS.
• ❌ ensureGitInfoExclude write-failure: not tested (Finding 1).
• ❌ logger.warn payload: not asserted (Finding 3).
• ⚠️ chmod-based tests don't fire the catch on Windows (Finding 4).
• ✅ CI passed on 3 platforms (8,846 tests).
• Docker end-to-end validation is manual-only (PR description); no automated E2E test. Acceptable given the nature of the fix.
• Vercel authorization is unrelated to this CLI/storage change.

Logging/observability (Lane E)

PASS with minor gap. No circular dependency between repo-manager.ts and logger.ts. Warning messages are structured with { path, code } and actionable text describing the impact. Local paths only — no secrets disclosed. Warnings fire at most once per cold invocation (subsequent calls hit the fast path). The _captureLogger() helper is available but not used in the new tests to assert warn payloads (Finding 3).

Hidden Unicode and hygiene (Lane G)

CLEAN. No hidden or bidirectional Unicode control characters found in either changed file. Non-ASCII characters present (─, —, →) are box-drawing and typographic characters used consistently throughout the existing codebase as section dividers and comment punctuation. git diff --check produced no output. DoD.md, CONTRIBUTING.md, and GUARDRAILS.md are unchanged. No unrelated files in the effective diff.

---

7. Final Verdict

not production-ready

The implementation logic is correct and the primary documented Docker workflow (host analyze then container index on :ro mount) works as intended. However, two issues prevent this from meeting the repo's DoD and the PR-specific bar:

1. The EROFS/EACCES tolerance added to ensureGitInfoExclude (lines 347–358) has zero test coverage — no test exercises this code path. DoD.md §2.7 requires that changed paths have tests that would fail on regression, and the PR-specific DoD explicitly lists this test as required. This is the primary blocking finding.

2. EPERM is absent from both catch conditions, breaking the read-only-tolerance promise for Windows CLI users. The rest of the codebase (lbug-config.ts, bridge-db.ts) already handles EPERM alongside EACCES for analogous filesystem scenarios, making this an accidental omission rather than a deliberate choice.

Both gaps are small and mechanical to address (add a test with a mock/chmod for ensureGitInfoExclude; add EPERM to both catch lists; optionally assert logger.warn). The fix itself is sound, well-scoped, and architecturally correct — these are test coverage and error-code completeness issues only.

Actionable fixes needed before merge:

1. Add ensureGitInfoExclude write-failure test → — cover the ensureGitInfoExclude EROFS/EACCES path directly.
2. Add EPERM to both catch conditions → — match the EPERM handling already used in lbug-config.ts and bridge-db.ts.
3. Assert logger.warn is called in the read-only test → — verify the warning contract, not just absence of throw.

---

· PR branch

[github-actions]

✨ PR Autofix

Found fixable formatting / unused-import issues across 89 changed lines. Comment /autofix on this PR to apply them, or run npm run lint:fix && npm run format locally.

{"schema":"gitnexus.pr-autofix/v2","state":"fixes-available","pr_number":1550,"changed_lines":89,"head_sha":"f3078d699f688cb8dcbd6561222873d4449d10e4","run_id":"25850317290","apply_command":"/autofix"}

[magyargergo]

/autofix

[github-actions]

✅ Applied autofix and pushed a commit. (apply run)