fix(hook): resolve canonical repo root + guard read-only FTS ensure

[sburdges-eng]

Closes #1224

This is my first contribution — thanks for building GitNexus. Two small, focused fixes for the Claude Code hook + query layer integration. Happy to iterate on style, scope, or naming.

Summary

Bug 1 — findGitNexusDir silently fails in linked git worktrees

The PreToolUse / PostToolUse hook (in both gitnexus/hooks/claude/gitnexus-hook.cjs and gitnexus-claude-plugin/hooks/gitnexus-hook.js) walks the filesystem upward from cwd looking for a non-registry .gitnexus/. In a linked worktree created via git worktree add ../<repo>-worktrees/feature-x, the canonical repo's .gitnexus/ never sits above the worktree path, so the walk silently fails — neither pattern augmentation nor staleness notifications fire.

Fix: Keep the cwd-walk as the fast path, then fall back to git rev-parse --git-common-dir. The --git-common-dir output is the shared .git/ directory across all linked worktrees (it always lives inside the canonical repo root), so its parent is the canonical repo. Returns null cleanly when git isn't on PATH or cwd isn't inside any working tree, preserving the existing graceful-failure contract.

function findGitNexusDir(startDir) {
  const cwd = startDir || process.cwd();

  // Fast path: the cwd is inside the canonical repo (most common case).
  const fromCwd = walkForGitNexusDir(cwd);
  if (fromCwd) return fromCwd;

  // Fallback: cwd may be inside a linked git worktree whose `.gitnexus/`
  // only lives in the canonical repo root.
  const canonicalRoot = findCanonicalRepoRoot(cwd);
  if (canonicalRoot && canonicalRoot !== cwd) {
    return walkForGitNexusDir(canonicalRoot);
  }
  return null;
}

Bug 2 — FTS index ensure fails on read-only DB connection

The MCP query pool opens LadybugDB read-only (correct by design — multiple processes read concurrently and analyze holds the write lock). Defensive callers of ensureFTSIndex from that pool surfaced five Cannot execute write operations in a read-only database warnings per call (one each for File / Function / Class / Method / Interface).

[gitnexus] FTS index ensure failed for repo "lariat" table "File" (index "file_fts"): Connection exception: Cannot execute write operations in a read-only database!. Will retry on next query.
[gitnexus] FTS index ensure failed for repo "lariat" table "Function" (index "function_fts"): ...
[gitnexus] FTS index ensure failed for repo "lariat" table "Class" (index "class_fts"): ...
[gitnexus] FTS index ensure failed for repo "lariat" table "Method" (index "method_fts"): ...
[gitnexus] FTS index ensure failed for repo "lariat" table "Interface" (index "interface_fts"): ...

Fix (lower-risk option): Add the read-only error to ensureFTSIndex's catch list. Index creation is owned by gitnexus analyze on a writable connection, so the ensure call is safely a no-op on the read pool. The fix mirrors the existing isDbBusyError discriminator pattern with a new isReadOnlyDbError, keeping the lock / busy / "already exists" / schema-error paths intact. After this PR there is no stderr noise on this path.

export const isReadOnlyDbError = (err: unknown): boolean => {
  const msg = err instanceof Error ? err.message : String(err);
  return /read-only database/i.test(msg);
};

// inside ensureFTSIndex:
try {
  await createFTSIndex(tableName, indexName, properties, stemmer);
  ensuredFTSIndexes.add(key);
} catch (e) {
  if (isReadOnlyDbError(e)) {
    ensuredFTSIndexes.add(key);
    return;
  }
  throw e;
}

Note: I checked that the current query path on main no longer calls ensureFTSIndex from a read-only connection (searchFTSFromLbug only invokes queryFTS / queryFTSViaExecutor, and pool-adapter only loads the FTS extension). The reproduction comes from gitnexus@1.6.3 (the published npm package), which still has the older path. This PR adds defense-in-depth so any future call site — or a re-introduction during refactoring — can't regress to the noisy behavior.

Test plan

• gitnexus/test/unit/hooks.test.ts — new "Linked git worktree resolution" describe block runs against both hook variants. Creates a real git worktree add linked tree and asserts PostToolUse fires the stale notification when the canonical repo has .gitnexus/, and stays silent when it doesn't.
• gitnexus/test/unit/lbug-readonly-error.test.ts — new file unit-tests isReadOnlyDbError (canonical message, wrapped prefix, case-insensitivity, non-Error inputs, plus negative cases for lock contention, "already exists", missing tables, and generic transient errors that must still surface).
• gitnexus/test/integration/lbug-core-adapter.test.ts — extended FTS coverage with an idempotency assertion for ensureFTSIndex to pin the read-only guard's success-path contract.
• npx tsc --noEmit clean.
• Targeted vitest run on the affected files (hooks + readonly + lbug-core-adapter + bm25-search + lbug-extension-loader + lbug-embedding-hashes — 136 tests pass).
• npm run build succeeds end-to-end.
• prettier --check clean on touched files; eslint adds no new errors (one new non-null-assertion warning that matches the existing test file's pattern).

Out of scope / observations

• I did not touch package versions, dependencies, or lockfiles.
• The gitnexus-claude-plugin/hooks/gitnexus-hook.js and gitnexus/hooks/claude/gitnexus-hook.cjs files share substantial logic and drift apart. Not in this PR's scope, but a future refactor to a single source-of-truth (with a small build step that emits the .cjs variant) would prevent the next "fix one, miss the other" issue.
• For Bug 1 I went with the git rev-parse approach rather than the ~/.gitnexus/registry.json lookup since the existing hook already shells out to git for HEAD checks — this keeps it consistent and avoids a new file-format dependency in the hook. Happy to switch approach if you'd prefer the registry path.

[vercel]

@sburdges-eng is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

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

---

PR #1181 Production Readiness Review

fix(local-backend): skip vector index query on unsupported platforms

Tasks

• Initialize review
• Load reference docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
• Inspect local-backend.ts — guard implementation, fallback path, diagnostics
• Inspect capabilities.ts — platform policy consistency
• Inspect calltool-dispatch.test.ts — test quality and coverage
• Check CI status / validation evidence
• Produce final review

---

Review bar for this PR

• On unsupported VECTOR platforms, LocalBackend.semanticSearch() must not issue CALL QUERY_VECTOR_INDEX(...) — no try/catch rescue from a native crash once entered.
• On unsupported platforms, semantic search must proceed to the real exact-scan CodeEmbedding fallback path (not just the count query).
• On supported platforms, CALL QUERY_VECTOR_INDEX(...) must still be issued first; failure fallback behavior is unchanged.
• The guard must use the same isVectorExtensionSupportedByPlatform() as core embedding code — no duplicate platform policy.
• No new extension-loading or install behavior introduced.
• Diagnostics must go to stderr only, fire at most once per session, and not be misleading about why the extension is unavailable.
• Tests must prove the actual scan Cypher was issued on unsupported platforms, not only the count query.
• TypeScript clean; CI passes on all 3 platforms.

---

Understanding of the change

Issue #1178: LocalBackend.semanticSearch() unconditionally called CALL QUERY_VECTOR_INDEX(...) regardless of platform. On Windows (and any platform where isVectorExtensionSupportedByPlatform() returns false), the LadybugDB native VECTOR extension is not available and can crash the process. A try/catch cannot protect against a native crash once that unsafe path is entered — the guard must prevent entry entirely.

Why the local backend was a gap: The core embedding pipeline already checked isVectorExtensionSupportedByPlatform() before VECTOR usage. LocalBackend.semanticSearch() had its own inline try { collectBestChunks(...QUERY_VECTOR_INDEX...) } catch { bestChunks = new Map(); }. The catch protects against a recoverable query error, not a native crash.

What changes on unsupported platforms: CALL QUERY_VECTOR_INDEX(...) is never issued. bestChunks stays empty. The existing if (bestChunks.size === 0) branch falls through to the exact-scan path. A one-shot console.error fires on the first call per LocalBackend instance.

What stays unchanged on supported platforms: CALL QUERY_VECTOR_INDEX(...) is still attempted first, inside the existing try/catch. Empty results or query failure still fall back to exact scan. No behavioral difference except one cheap boolean check.

Out of scope: The underlying LadybugDB native VECTOR crash on unsupported platforms. Extension lifecycle or install policy. No lockfile or dependency changes.

---

Findings

[medium] No explicit test asserting QUERY_VECTOR_INDEX still fires on supported platforms

• Category: Test coverage / regression risk
• Files: gitnexus/test/unit/calltool-dispatch.test.ts
• Issue: The beforeEach resets platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true), and existing query tests exercise callTool('query', ...). However, no test explicitly asserts that QUERY_VECTOR_INDEX appears in the query list when the platform returns true. If the guard were accidentally inverted (!isVectorExtensionSupportedByPlatform()), the existing tests would not fail — they would just silently use exact scan and the default executeQuery mock returning [] would still let the test pass.
• Why it matters here: The PR's primary job is a targeted guard. Regressing supported platforms by accidentally inverting the condition is the most plausible future breakage, and it would go undetected.
• Recommended fix: Add one test alongside the new unsupported-platform test:

  it('issues QUERY_VECTOR_INDEX on supported platforms', async () => {
    platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true);
    (executeQuery as any).mockImplementation(async (_: string, cypher: string) => {
      if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }];
      return [];
    });
    await backend.callTool('query', { query: 'auth' });
    const queries = (executeQuery as any).mock.calls.map(([, c]: [string, string]) => c);
    expect(queries.some((c: string) => c.includes('QUERY_VECTOR_INDEX'))).toBe(true);
  });

  Fix this →

[low] Diagnostic wording implies extension is missing rather than deliberately unsupported

• Category: Observability / operator experience
• Files: gitnexus/src/mcp/local/local-backend.ts:1118
• Issue: 'GitNexus [query:vector]: VECTOR index unavailable for this runtime; using exact scan fallback' — the word "unavailable" can be read as "the extension isn't installed" rather than "this platform doesn't support VECTOR by policy." Compare with capabilities.ts which says "LadybugDB VECTOR is disabled on this platform". Operators on Windows may open a support issue thinking they need to install a native dependency.
• Why it matters here: MCP server output is often the first and only diagnostic signal for operators debugging CI or cloud environments. Misleading it creates avoidable noise.
• Recommended fix: Align wording with the existing capability reason string:

  console.error(
    'GitNexus [query:vector]: VECTOR extension not supported on this platform; using exact scan fallback',
  );

---

Definition of Done check

DoD item	Status
On unsupported platforms, semanticSearch() does not issue CALL QUERY_VECTOR_INDEX(...)	satisfied — guard prevents entry entirely; test proves no QUERY_VECTOR_INDEX call
On unsupported platforms, proceeds to real exact-scan CodeEmbedding fallback	satisfied — bestChunks stays empty; test asserts RETURN e.nodeId AS nodeId + e.embedding AS embedding in issued queries, distinguishing from count query
On supported platforms, vector-index behavior unchanged	partially satisfied — code is provably unchanged; test defaults to supported mode; but no explicit assertion that QUERY_VECTOR_INDEX still fires (see finding above)
Uses same isVectorExtensionSupportedByPlatform() as core embedding code	satisfied — imported directly from canonical ../../core/platform/capabilities.js; no duplicate policy
No new extension-loading or install behavior	satisfied
Diagnostics: stderr-only, one-shot per session, safe for MCP JSON-RPC stdout	satisfied — console.error, guarded by warnedVectorUnsupported per-instance flag, mirrors warnedSiblingDrift pattern; stdout is never touched
Diagnostics: wording accurate and not misleading	partially satisfied — "unavailable" is imprecise; see low finding above
Tests prove actual scan query issued (not only count query)	satisfied — assertion checks for RETURN e.nodeId AS nodeId AND e.embedding AS embedding, which only appear in the scan query, not the count query
TypeScript clean; CI passes on 3 platforms	satisfied — PR claims tsc --noEmit clean; CI reported 7,667 tests passed on 3 platforms

---

Validation assessment

Tests added / updated:

• New test 'skips vector index query when VECTOR is unsupported by the platform' in calltool-dispatch.test.ts
• Mock infrastructure for isVectorExtensionSupportedByPlatform added to vi.hoisted() block; module mock properly uses async (importOriginal) to spread real exports
• beforeEach resets platform mock to true — isolation is correct
• console.error spied and restored in finally ✓

Validation claimed by the PR:

• npx vitest run --project default "test/unit/platform-capabilities.test.ts" — 2 tests passed
• npx vitest run --project default "test/unit/calltool-dispatch.test.ts" — 67 tests passed
• npx tsc --noEmit clean
• CI: typecheck + unit tests on 3 platforms (ubuntu, macOS, Windows), 7,667 tests, 2 skipped

Verified from diff / CI evidence:

• Diff is correct and matches the claimed description end-to-end
• The weak assertion concern from earlier review feedback is resolved: the test checks RETURN e.nodeId AS nodeId + e.embedding AS embedding, which only appear in the scan projection, not the count query
• Diagnostic is asserted in the test with stringContaining(...) ✓
• PR fix(local-backend): #1178 skip vector index query on unsupported pla… #1181 was merged as commit 9cd8c36 — CI for that SHA is not directly inspectable in the current context, but the PR description is consistent with the diff

Missing / not verifiable:

• No test explicitly asserting QUERY_VECTOR_INDEX still fires on supported platforms (see medium finding)
• No end-to-end MCP smoke test with a real Windows DB and real embeddings — unit mocks are adequate for this targeted fix, but a real Windows integration test would close the gap entirely
• CI logs for the merged 9cd8c36 SHA are not available to verify platform-specific run artifacts

Does missing validation block production readiness? No. The missing supported-platform assertion is a follow-up risk, not a current regression. The unit mock approach is proportionate for a platform guard that wraps unchanged code.

---

Final verdict

production-ready with minor follow-ups

The core correctness of this PR is sound: the guard correctly prevents CALL QUERY_VECTOR_INDEX(...) on unsupported platforms using the canonical platform capability function, bestChunks is never populated on the unsafe path (ensuring fallback executes), the diagnostic is safely routed to stderr and deduplicated per instance, and the test strengthening over the prior weak assertion is real — the scan query assertion specifically targets RETURN e.nodeId AS nodeId and e.embedding AS embedding, which only appear in the full scan, not the count query. The two follow-ups — adding a supported-platform regression test and tightening the diagnostic wording — are low-risk cleanups that don't block the crash-avoidance goal of this fix. The change is small, correctly scoped, easy to revert, and consistent with platform capability policy across the codebase.

[magyargergo]

@sburdges-eng thank you for your contribution and nice work! Could you please tackle with the findings in this PR? 🙏

[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
7719	7718	0	1	316s

✅ All 7718 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.05%	22771/29553	77.02%	📈 +0.0	🟢 ███████████████░░░░░
Branches	65.99%	14674/22236	65.97%	📈 +0.0	🟢 █████████████░░░░░░░
Functions	81.9%	2222/2713	81.82%	📈 +0.1	🟢 ████████████████░░░░
Lines	79.91%	20546/25709	79.89%	📈 +0.0	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}