diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index dcc26d57d..82b76f23e 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -45,7 +45,12 @@ import { cliError } from './cli-message.js'; import { EMBEDDING_DIMS_ERROR, normalizeEmbeddingDims } from './embedding-dims.js'; import { formatElapsed } from './format-elapsed.js'; import { isHfDownloadFailure } from '../core/embeddings/hf-env.js'; -import { isHttpMode, safeUrl } from '../core/embeddings/http-client.js'; +import { + isHttpEmbeddingDimsError, + isHttpEmbeddingError, + isHttpMode, + safeUrl, +} from '../core/embeddings/http-client.js'; import { isLocalEmbeddingRuntimeBlockerMessage, isMissingLocalEmbeddingStackMessage, @@ -1641,10 +1646,62 @@ const analyzeCommandImpl = async ( return; } + // Malformed GITNEXUS_EMBEDDING_DIMS env var (#2385). readConfig() throws a + // plain Error (a config mistake, not an endpoint failure), surfacing here from + // httpEmbed()->readConfig() inside the analysis run. Show a clean config + // message rather than a raw stack dump. The --embedding-dims CLI flag is + // validated up front (EMBEDDING_DIMS_ERROR); this covers the env-var path. + // Checked before the endpoint/HF branches: it is a plain Error, so + // isHttpEmbeddingError() is false and the HF network heuristic must not claim it. + if (isHttpEmbeddingDimsError(msg)) { + cliError(` ${msg.replace(/\n/g, '\n ')}\n`, { + recoveryHint: 'embedding-dims-invalid', + }); + process.exitCode = 1; + return; + } + + // Custom HTTP embedding endpoint failure (#2385). When a `--embedding-base-url` + // is configured, HTTP mode never downloads a model — so a failure talking to + // that endpoint must NOT show the huggingface-download guidance. Keyed on the + // error *type* (HttpEmbeddingError), not its message text, so it stays correct + // regardless of locale or wording. Checked before the HF branch, whose network + // heuristic (`fetch failed` / `ECONNREFUSED`) would otherwise also match a + // wrapped endpoint-connection error. The header is deliberately neutral: this + // type covers both never-reached failures (connection/timeout/DNS) and + // reached-but-failed ones (4xx/5xx, dimension/shape mismatch), so it must not + // assert "unreachable". The thrown `msg` carries the specific reason (and the + // masked URL where one applies), so it is surfaced verbatim. + if (isHttpEmbeddingError(err)) { + cliError( + ` The custom embedding endpoint request failed.\n` + + ` ${msg.replace(/\n/g, '\n ')}\n` + + ` Suggestions:\n` + + ` 1. Verify the endpoint URL is reachable and running ` + + `(--embedding-base-url / GITNEXUS_EMBEDDING_URL: host, port, /v1 path).\n` + + ` 2. Confirm the model name and embedding dimensions match what the endpoint serves.\n` + + ` 3. Re-run without --embeddings to index without vectors.\n`, + { recoveryHint: 'http-embedding-endpoint-error' }, + ); + process.exitCode = 1; + return; + } + + // isHttpMode() is a pure presence probe (URL+MODEL) that never throws — a + // malformed GITNEXUS_EMBEDDING_DIMS is handled by the dims branch above — so + // no defensive try/catch is needed here (#2385). + const inHttpMode = isHttpMode(); + // HF download failure — show clean guidance without the raw stack trace. // Checked before writeFatalToStderr so the user sees one focused message // rather than a stack-trace dump followed by a second remediation block. - if (isHfDownloadFailure(msg) || msg.includes('Failed to download embedding model')) { + // Gated on !inHttpMode: with a custom endpoint configured no model download + // is ever attempted, so a network error there is the endpoint's, handled by + // the HttpEmbeddingError branch above — never HF's (#2385). + if ( + (isHfDownloadFailure(msg) || msg.includes('Failed to download embedding model')) && + !inHttpMode + ) { cliError( ` The embedding model could not be downloaded.\n` + ` huggingface.co may be unreachable from your network\n` + diff --git a/gitnexus/src/cli/cli-message.ts b/gitnexus/src/cli/cli-message.ts index cf4693b91..fb5331e45 100644 --- a/gitnexus/src/cli/cli-message.ts +++ b/gitnexus/src/cli/cli-message.ts @@ -49,6 +49,8 @@ export type RecoveryHint = | 'heap-oom-respawn' | 'native-worker-abort' | 'hf-endpoint-unreachable' + | 'http-embedding-endpoint-error' + | 'embedding-dims-invalid' | 'local-embedding-unsupported' | 'local-embedding-stack-missing' | 'large-repo' diff --git a/gitnexus/src/core/embeddings/http-client.ts b/gitnexus/src/core/embeddings/http-client.ts index cab2de8ce..121790251 100644 --- a/gitnexus/src/core/embeddings/http-client.ts +++ b/gitnexus/src/core/embeddings/http-client.ts @@ -27,10 +27,29 @@ interface HttpConfig { dimensions?: number; } +/** + * Stable lead of the {@link readConfig} malformed-`GITNEXUS_EMBEDDING_DIMS` + * error. `readConfig` throws a plain `Error` (not an {@link HttpEmbeddingError}) + * because this is a *config* mistake, not an endpoint failure — so the CLI + * recognizes it by this lead ({@link isHttpEmbeddingDimsError}) and prints a + * clean config message instead of a raw stack dump. See #2385. + */ +const EMBEDDING_DIMS_ENV_ERROR_LEAD = 'GITNEXUS_EMBEDDING_DIMS must be a positive integer'; + +/** + * @internal Exported for the CLI analyze error handler. True when `message` is + * the {@link readConfig} malformed-DIMS config error (a plain `Error`). + */ +export const isHttpEmbeddingDimsError = (message: string): boolean => + message.includes(EMBEDDING_DIMS_ENV_ERROR_LEAD); + /** * Build config from the current process.env snapshot. * Returns null when GITNEXUS_EMBEDDING_URL + GITNEXUS_EMBEDDING_MODEL are unset. * Not cached — env vars are read fresh so late configuration takes effect. + * Validates GITNEXUS_EMBEDDING_DIMS and throws on a malformed value; callers + * that only need to know whether HTTP mode is *configured* must use + * {@link isHttpMode} (a presence probe that never throws), not this. */ const readConfig = (): HttpConfig | null => { const baseUrl = process.env.GITNEXUS_EMBEDDING_URL; @@ -41,11 +60,11 @@ const readConfig = (): HttpConfig | null => { let dimensions: number | undefined; if (rawDims !== undefined) { if (!/^\d+$/.test(rawDims)) { - throw new Error(`GITNEXUS_EMBEDDING_DIMS must be a positive integer, got "${rawDims}"`); + throw new Error(`${EMBEDDING_DIMS_ENV_ERROR_LEAD}, got "${rawDims}"`); } const parsed = parseInt(rawDims, 10); if (parsed <= 0) { - throw new Error(`GITNEXUS_EMBEDDING_DIMS must be a positive integer, got "${rawDims}"`); + throw new Error(`${EMBEDDING_DIMS_ENV_ERROR_LEAD}, got "${rawDims}"`); } dimensions = parsed; } @@ -59,9 +78,16 @@ const readConfig = (): HttpConfig | null => { }; /** - * Check whether HTTP embedding mode is active (env vars are set). + * Whether HTTP embedding mode is active — i.e. both `GITNEXUS_EMBEDDING_URL` and + * `GITNEXUS_EMBEDDING_MODEL` are set. A pure presence probe: it deliberately does + * NOT call {@link readConfig}, so it never throws on a malformed + * `GITNEXUS_EMBEDDING_DIMS`. This lets its ~13 call sites (analyze, doctor, + * run-analyze, embedder, mcp) probe the mode without a defensive try/catch; the + * DIMS value is validated where it is actually used (`readConfig` in + * `httpEmbed`/`httpEmbedQuery`), surfacing a recognizable config error. See #2385. */ -export const isHttpMode = (): boolean => readConfig() !== null; +export const isHttpMode = (): boolean => + Boolean(process.env.GITNEXUS_EMBEDDING_URL && process.env.GITNEXUS_EMBEDDING_MODEL); /** * Return the configured embedding dimensions for HTTP mode, or undefined @@ -84,10 +110,74 @@ export const safeUrl = (url: string): string => { } }; +/** + * Strip credentials from an underlying transport error message before it is + * surfaced. A credential-bearing endpoint URL (`https://user:secret@host/v1`) + * makes undici throw `TypeError: Request cannot be constructed from a URL that + * includes credentials: `; interpolating `err.message` verbatim + * would re-leak the secret to stderr + logs even though the URL argument is + * already masked with {@link safeUrl}. First swap the exact configured `url` for + * its masked form, then strip any residual `scheme://userinfo@` the transport may + * have echoed in a normalized (non-exact) form. See #2385. + */ +const sanitizeReason = (reason: string, url: string): string => + reason + .split(url) + .join(safeUrl(url)) + .replace(/([a-z][a-z0-9+.-]*:\/\/)[^/@\s]*@/gi, '$1'); + +/** + * Error thrown by this module's HTTP embedding path (`httpEmbedBatch` / + * `httpEmbed` / `httpEmbedQuery`) for any endpoint failure — a + * connection/timeout/DNS error, an open circuit, a non-OK status, an + * unparseable or wrong-shape response body, an empty response, or a dimension + * mismatch. + * + * Carrying a distinct type (rather than a plain `Error`) lets the CLI tell a + * *custom endpoint* failure apart from a HuggingFace *model download* failure + * without matching message text: the two share the same underlying network + * substrings (`fetch failed`, `ECONNREFUSED`, …), which is exactly why + * `isNetworkFetchError` in `hf-env.ts` cannot tell them apart. Keying on the + * type instead of the message is also locale-proof and survives message + * rewording. The human-readable `.message` (built with `safeUrl` and the + * underlying reason) is what the CLI surfaces to the user. See #2385. + */ +export class HttpEmbeddingError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options?.cause !== undefined ? { cause: options.cause } : undefined); + this.name = 'HttpEmbeddingError'; + } +} + +/** + * @internal Exported for the CLI analyze error handler and unit tests. + * + * Type-guard for {@link HttpEmbeddingError}. The `name` fallback keeps the + * check working across module-realm boundaries where `instanceof` can fail + * (two loaded copies of the class) — mirroring the codebase's existing + * `err.name === 'TimeoutError'` idiom. Matches on the stable class + * discriminator, never on the human-readable (potentially localized) message. + */ +export const isHttpEmbeddingError = (err: unknown): boolean => + err instanceof HttpEmbeddingError || (err instanceof Error && err.name === 'HttpEmbeddingError'); + interface EmbeddingItem { embedding: number[]; } +/** + * Runtime guard for a single response item. The `Array.isArray(data.data)` shape + * check only validates the outer array — a 200 body like `{"data":[null]}` passes + * it, then crashes at `new Float32Array(item.embedding)` (`httpEmbed`) or + * `items[0].embedding` (`httpEmbedQuery`) with a raw `TypeError` that escapes the + * typed boundary, landing on the CLI's generic stack-dump path — the exact class + * #2385 closes. Validate each item so every wrong-shape body stays classifiable. + */ +const isEmbeddingItem = (item: unknown): item is EmbeddingItem => + typeof item === 'object' && + item !== null && + Array.isArray((item as { embedding?: unknown }).embedding); + /** * Send a single batch of texts to the embedding endpoint with retry. * @@ -140,33 +230,56 @@ const httpEmbedBatch = async ( ); } catch (err) { if (err instanceof CircuitOpenError) { - throw new Error( + throw new HttpEmbeddingError( `Embedding endpoint circuit open (${safeUrl(url)}, batch ${batchIndex}): retry in ${Math.ceil(err.retryAfterMs / 1000)}s`, + { cause: err }, ); } if (err instanceof DOMException && err.name === 'TimeoutError') { - throw new Error( + throw new HttpEmbeddingError( `Embedding request timed out after ${HTTP_TIMEOUT_MS}ms (${safeUrl(url)}, batch ${batchIndex})`, + { cause: err }, ); } if (err instanceof ResilientFetchExhaustedError) { - throw new Error( + throw new HttpEmbeddingError( `Embedding endpoint returned ${err.response.status} (${safeUrl(url)}, batch ${batchIndex})`, + { cause: err }, ); } - const reason = err instanceof Error ? err.message : String(err); - throw new Error(`Embedding request failed (${safeUrl(url)}, batch ${batchIndex}): ${reason}`); + const reason = sanitizeReason(err instanceof Error ? err.message : String(err), url); + throw new HttpEmbeddingError( + `Embedding request failed (${safeUrl(url)}, batch ${batchIndex}): ${reason}`, + { cause: err }, + ); } if (!resp.ok) { // resilientFetch already retried 5xx/429; any non-OK response here is // a terminal client error (4xx other than 429). - throw new Error( + throw new HttpEmbeddingError( `Embedding endpoint returned ${resp.status} (${safeUrl(url)}, batch ${batchIndex})`, ); } - const data = (await resp.json()) as { data: EmbeddingItem[] }; + // A reachable-but-wrong endpoint (e.g. a captive portal or a non-embeddings + // service) can answer 200 with an HTML/truncated body. Parse inside the + // typed-error boundary so that lands as an endpoint failure the CLI can + // classify, not a raw SyntaxError/TypeError on the generic stack-dump path. + let data: { data: EmbeddingItem[] }; + try { + data = (await resp.json()) as { data: EmbeddingItem[] }; + } catch (err) { + throw new HttpEmbeddingError( + `Embedding endpoint returned an unparseable response (${safeUrl(url)}, batch ${batchIndex})`, + { cause: err }, + ); + } + if (!Array.isArray(data?.data) || !data.data.every(isEmbeddingItem)) { + throw new HttpEmbeddingError( + `Embedding endpoint returned an unexpected response shape (${safeUrl(url)}, batch ${batchIndex})`, + ); + } return data.data; }; @@ -199,7 +312,7 @@ export const httpEmbed = async (texts: string[]): Promise => { ); if (items.length !== batch.length) { - throw new Error( + throw new HttpEmbeddingError( `Embedding endpoint returned ${items.length} vectors for ${batch.length} texts ` + `(${safeUrl(url)}, batch ${batchIndex})`, ); @@ -214,7 +327,7 @@ export const httpEmbed = async (texts: string[]): Promise => { const hint = config.dimensions ? 'Update GITNEXUS_EMBEDDING_DIMS to match your model output.' : `Set GITNEXUS_EMBEDDING_DIMS=${vec.length} to match your model output.`; - throw new Error( + throw new HttpEmbeddingError( `Embedding dimension mismatch: endpoint returned ${vec.length}d vector, ` + `but expected ${expected}d. ${hint}`, ); @@ -248,7 +361,7 @@ export const httpEmbedQuery = async (text: string): Promise => { config.dimensions, ); if (!items.length) { - throw new Error(`Embedding endpoint returned empty response (${safeUrl(url)})`); + throw new HttpEmbeddingError(`Embedding endpoint returned empty response (${safeUrl(url)})`); } const embedding = items[0].embedding; @@ -259,7 +372,7 @@ export const httpEmbedQuery = async (text: string): Promise => { const hint = config.dimensions ? 'Update GITNEXUS_EMBEDDING_DIMS to match your model output.' : `Set GITNEXUS_EMBEDDING_DIMS=${embedding.length} to match your model output.`; - throw new Error( + throw new HttpEmbeddingError( `Embedding dimension mismatch: endpoint returned ${embedding.length}d vector, ` + `but expected ${expected}d. ${hint}`, ); diff --git a/gitnexus/test/unit/analyze-http-endpoint-error.test.ts b/gitnexus/test/unit/analyze-http-endpoint-error.test.ts new file mode 100644 index 000000000..2dcd0b3d9 --- /dev/null +++ b/gitnexus/test/unit/analyze-http-endpoint-error.test.ts @@ -0,0 +1,253 @@ +/** + * Tests for the custom HTTP embedding endpoint failure path in the + * `analyzeCommand` CLI (#2385). + * + * When a `--embedding-base-url` is configured, HTTP mode never downloads a + * model. A connection/timeout/DNS failure to that endpoint must surface an + * endpoint-specific message — NOT the huggingface.co download remediation, + * whose network heuristic (`fetch failed` / `ECONNREFUSED`) would otherwise + * also match the wrapped endpoint error. The analyze handler discriminates on + * the error *type* (`HttpEmbeddingError`), not its message text. + * + * Mirrors analyze-local-embedding-error.test.ts: + * - vi.mock the heavy dependencies so no real DB / git is touched + * - drive `analyzeCommand` with a mocked `runFullAnalysis` that rejects + * - assert on process.exitCode and the captured logger records + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const runFullAnalysisMock = vi.fn(); +// Controls the HF network heuristic so the gate/ordering scenarios can force it +// to also claim a plain network error and prove the endpoint branch / mode gate +// still win. +const isHfDownloadFailureMock = vi.fn(() => false); +// Controls isHttpMode so the HF-branch gate (`!isHttpMode()`) can be exercised +// in both states without setting real env vars. The real HttpEmbeddingError / +// isHttpEmbeddingError / safeUrl are preserved via importOriginal. +const isHttpModeMock = vi.fn(() => true); + +const resolveEmbeddingRuntimeMock = vi.fn<() => { source: string } | null>(() => ({ + source: 'package', +})); +const isPrefixRuntimeLoadableMock = vi.fn(() => true); +const installEmbeddingRuntimeMock = vi.fn(async () => undefined); +vi.mock('../../src/core/embeddings/runtime-install.js', async (importOriginal) => ({ + ...(await importOriginal()), + resolveEmbeddingRuntime: () => resolveEmbeddingRuntimeMock(), + isPrefixRuntimeLoadable: () => isPrefixRuntimeLoadableMock(), + installEmbeddingRuntime: (...args: unknown[]) => installEmbeddingRuntimeMock(...args), + getEmbeddingRuntimeDir: () => '/fake/embedding-runtime', +})); + +vi.mock('../../src/core/run-analyze.js', () => ({ + runFullAnalysis: runFullAnalysisMock, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ + closeLbug: vi.fn(async () => undefined), + closeLbugBeforeExit: vi.fn(async () => undefined), + isLbugReady: vi.fn(() => false), +})); + +vi.mock('../../src/storage/repo-manager.js', () => ({ + getStoragePaths: vi.fn(() => ({ storagePath: '.gitnexus', lbugPath: '.gitnexus/lbug' })), + getGlobalRegistryPath: vi.fn(() => 'registry.json'), + RegistryNameCollisionError: class RegistryNameCollisionError extends Error {}, + AnalysisNotFinalizedError: class AnalysisNotFinalizedError extends Error {}, + assertAnalysisFinalized: vi.fn(async () => undefined), +})); + +vi.mock('../../src/storage/git.js', () => ({ + getGitRoot: vi.fn(() => '/repo'), + hasGitDir: vi.fn(() => true), +})); + +vi.mock('../../src/core/ingestion/utils/max-file-size.js', () => ({ + getMaxFileSizeBannerMessage: vi.fn(() => null), +})); + +// analyze.ts imports isHfDownloadFailure from hf-env.js. Mock it to break the +// transitive gitnexus-shared chain and to drive the HF-heuristic scenarios. +vi.mock('../../src/core/embeddings/hf-env.js', () => ({ + isHfDownloadFailure: isHfDownloadFailureMock, +})); + +// Preserve the real HttpEmbeddingError / isHttpEmbeddingError / safeUrl; only +// override isHttpMode so the mode gate can be flipped per test. +vi.mock('../../src/core/embeddings/http-client.js', async (importOriginal) => ({ + ...(await importOriginal()), + isHttpMode: () => isHttpModeMock(), +})); + +describe('analyzeCommand custom HTTP endpoint error handling (#2385)', () => { + beforeEach(() => { + vi.resetModules(); + runFullAnalysisMock.mockReset(); + isHfDownloadFailureMock.mockReset().mockReturnValue(false); + isHttpModeMock.mockReset().mockReturnValue(true); + resolveEmbeddingRuntimeMock.mockReset().mockReturnValue({ source: 'package' }); + isPrefixRuntimeLoadableMock.mockReset().mockReturnValue(true); + installEmbeddingRuntimeMock.mockReset().mockResolvedValue(undefined); + process.exitCode = undefined; + process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); + }); + + it('routes an endpoint connection failure to a clean endpoint message (R1)', async () => { + const { HttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + runFullAnalysisMock.mockRejectedValue( + new HttpEmbeddingError( + 'Embedding request failed (http://127.0.0.1:1/v1/embeddings, batch 0): fetch failed', + ), + ); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(process.exitCode).toBe(1); + const record = cap.records().find((r) => r.recoveryHint === 'http-embedding-endpoint-error'); + expect(record).toBeDefined(); + // The masked URL from the thrown message is surfaced verbatim. + expect(typeof record?.msg === 'string' && record.msg).toContain('127.0.0.1:1'); + cap.restore(); + }); + + it('routes a malformed GITNEXUS_EMBEDDING_DIMS to a clean config message, not endpoint/HF (R3)', async () => { + // readConfig() throws a plain Error on a malformed env DIMS; it surfaces from + // the embedding pipeline into this catch. It is a config mistake, not an + // endpoint failure, so it must get its own clean message — never the endpoint + // or HF branch. (isHttpMode() no longer throws, so the crash at analyze:1109 + // that this used to be is gone; the error now reaches here.) + runFullAnalysisMock.mockRejectedValue( + new Error('GITNEXUS_EMBEDDING_DIMS must be a positive integer, got "1024abc"'), + ); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + expect(records.some((r) => r.recoveryHint === 'embedding-dims-invalid')).toBe(true); + expect(records.some((r) => r.recoveryHint === 'http-embedding-endpoint-error')).toBe(false); + expect(records.some((r) => r.recoveryHint === 'hf-endpoint-unreachable')).toBe(false); + const record = records.find((r) => r.recoveryHint === 'embedding-dims-invalid'); + expect(typeof record?.msg === 'string' && record.msg).toContain('GITNEXUS_EMBEDDING_DIMS'); + cap.restore(); + }); + + it('does not mislabel a reached-but-failed endpoint as "could not be reached"', async () => { + // A dimension mismatch means the endpoint WAS reached and answered — the + // message must not assert unreachability, and must surface the real reason + // (which itself carries the fix hint). Regression guard for the #2385 fix. + const { HttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + runFullAnalysisMock.mockRejectedValue( + new HttpEmbeddingError( + 'Embedding dimension mismatch: endpoint returned 512d vector, but expected 1024d. ' + + 'Update GITNEXUS_EMBEDDING_DIMS to match your model output.', + ), + ); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(process.exitCode).toBe(1); + const record = cap.records().find((r) => r.recoveryHint === 'http-embedding-endpoint-error'); + expect(record).toBeDefined(); + const text = typeof record?.msg === 'string' ? record.msg : ''; + // Surfaces the real reason... + expect(text).toContain('dimension mismatch'); + // ...without falsely claiming the endpoint was unreachable. + expect(text).not.toMatch(/could not be reached|unreachable/i); + cap.restore(); + }); + + it('never mentions huggingface for an endpoint failure, even if the HF heuristic matches (R2, R3)', async () => { + // Force the HF network heuristic to also claim this error. The typed + // endpoint branch is ordered first, so HF guidance must not appear. + isHfDownloadFailureMock.mockReturnValue(true); + const { HttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + runFullAnalysisMock.mockRejectedValue( + new HttpEmbeddingError( + 'Embedding request failed (http://127.0.0.1:1/v1/embeddings, batch 0): fetch failed', + ), + ); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + const records = cap.records(); + expect(records.some((r) => r.recoveryHint === 'http-embedding-endpoint-error')).toBe(true); + expect(records.some((r) => r.recoveryHint === 'hf-endpoint-unreachable')).toBe(false); + expect(records.every((r) => !(typeof r.msg === 'string' && /huggingface/i.test(r.msg)))).toBe( + true, + ); + cap.restore(); + }); + + it('suppresses the HF branch for a raw network error while in HTTP mode (R3 gate)', async () => { + // A plain (untyped) network error while a custom endpoint is configured: + // the endpoint branch keys on the type so it does not fire, and the HF + // branch is gated on !isHttpMode() so it must not fire either. + isHttpModeMock.mockReturnValue(true); + isHfDownloadFailureMock.mockReturnValue(true); + runFullAnalysisMock.mockRejectedValue(new Error('fetch failed')); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + const records = cap.records(); + expect(records.some((r) => r.recoveryHint === 'hf-endpoint-unreachable')).toBe(false); + expect(records.some((r) => r.recoveryHint === 'http-embedding-endpoint-error')).toBe(false); + cap.restore(); + }); + + it('leaves the real HF-download path unchanged when HTTP mode is inactive (R4)', async () => { + // Local embedder (no custom endpoint): a genuine HF download network error + // must still show the huggingface guidance. + isHttpModeMock.mockReturnValue(false); + isHfDownloadFailureMock.mockReturnValue(true); + runFullAnalysisMock.mockRejectedValue(new Error('TypeError: fetch failed')); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + expect(records.some((r) => r.recoveryHint === 'hf-endpoint-unreachable')).toBe(true); + expect(records.some((r) => r.recoveryHint === 'http-embedding-endpoint-error')).toBe(false); + cap.restore(); + }); + + it('does not capture unrelated HTTP-mode errors in the endpoint branch (R5)', async () => { + isHttpModeMock.mockReturnValue(true); + runFullAnalysisMock.mockRejectedValue(new Error('LadybugDB write failed')); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + expect(records.some((r) => r.recoveryHint === 'http-embedding-endpoint-error')).toBe(false); + cap.restore(); + }); +}); diff --git a/gitnexus/test/unit/doctor-format.test.ts b/gitnexus/test/unit/doctor-format.test.ts index e3b2ff219..11f3bac59 100644 --- a/gitnexus/test/unit/doctor-format.test.ts +++ b/gitnexus/test/unit/doctor-format.test.ts @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest'; -import { displayWidth, localEmbeddingDoctorStatus, padDisplayEnd } from '../../src/cli/doctor.js'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + displayWidth, + doctorCommand, + localEmbeddingDoctorStatus, + padDisplayEnd, +} from '../../src/cli/doctor.js'; describe('doctor output formatting', () => { it('keeps ASCII padding equivalent to String.padEnd', () => { @@ -121,3 +126,35 @@ describe('doctor embedding-runtime support status', () => { expect(detail).toBeNull(); }); }); + +describe('doctor survives a malformed GITNEXUS_EMBEDDING_DIMS (#2385)', () => { + const ENV_KEYS = [ + 'GITNEXUS_EMBEDDING_URL', + 'GITNEXUS_EMBEDDING_MODEL', + 'GITNEXUS_EMBEDDING_DIMS', + ] as const; + const savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + + afterEach(() => { + vi.restoreAllMocks(); + for (const key of ENV_KEYS) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; + } + } + }); + + it('does not crash at the unguarded isHttpMode() call sites', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + process.env.GITNEXUS_EMBEDDING_DIMS = '1024abc'; + vi.spyOn(console, 'log').mockImplementation(() => undefined); + + // Before the isHttpMode() root-cause fix (#2385) this threw at doctor.ts:167 + // (isHttpMode -> readConfig -> throw on the malformed DIMS); now the presence + // probe never throws, so `gitnexus doctor` completes and reports the backend. + await expect(doctorCommand()).resolves.toBeUndefined(); + }); +}); diff --git a/gitnexus/test/unit/http-embedder.test.ts b/gitnexus/test/unit/http-embedder.test.ts index c19bb4825..b99cef182 100644 --- a/gitnexus/test/unit/http-embedder.test.ts +++ b/gitnexus/test/unit/http-embedder.test.ts @@ -200,7 +200,103 @@ describe('HTTP embedding backend', () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 500 })); const { embedText } = await import('../../src/core/embeddings/embedder.js'); - await expect(embedText('test')).rejects.toThrow('500'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(String(err)).toContain('500'); + // Type-completeness fence: a non-OK-status failure must stay classifiable + // so the CLI routes it to the endpoint branch, not the HF branch (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); + }); + + it('classifies a terminal 4xx (404) as a typed endpoint error without retrying (#2385)', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + // The most common --embedding-base-url misconfiguration: wrong path -> 404, + // bad key -> 401/403. resilientFetch returns a terminal 4xx (other than 429) + // without retrying, so httpEmbedBatch's !resp.ok branch is the sole + // classifier — distinct from 500 (ResilientFetchExhaustedError) and 429/503. + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 404 })); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(String(err)).toContain('404'); + expect(isHttpEmbeddingError(err)).toBe(true); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('classifies a reachable endpoint that returns a non-JSON 200 body', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + // A captive portal / wrong service answers 200 with HTML — resp.json() throws. + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: async () => { + throw new SyntaxError('Unexpected token < in JSON at position 0'); + }, + }), + ); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(isHttpEmbeddingError(err)).toBe(true); + expect(String(err)).toContain('unparseable response'); + }); + + it('surfaces a connection failure as a typed HttpEmbeddingError (the #2385 case)', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://127.0.0.1:1/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + // Node's undici throws `TypeError: fetch failed` on a terminal connect error. + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('fetch failed'))); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + + const err = await embedText('test').catch((e: unknown) => e); + // The endpoint failure carries the type — no message-text matching needed. + expect(isHttpEmbeddingError(err)).toBe(true); + // The masked URL is preserved for the CLI message; no HuggingFace text. + expect(String(err)).toContain('127.0.0.1:1'); + expect(String(err)).not.toMatch(/huggingface/i); + }); + + // A reachable-but-wrong endpoint can answer 200 with a well-formed outer array + // whose items are malformed. The outer Array.isArray(data.data) guard passes; + // without per-item validation these crash at new Float32Array(item.embedding) + // (batch) / items[0].embedding (query) with a raw TypeError that escapes the + // typed boundary — the exact #2385 stack-dump class. (#2385) + it.each([ + { label: 'a null item', body: { data: [null] } }, + { label: 'an item with no embedding', body: { data: [{}] } }, + { label: 'an item whose embedding is not an array', body: { data: [{ embedding: 'nope' }] } }, + ])('types a malformed response item ($label) on the batch path', async ({ body }) => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => body })); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(isHttpEmbeddingError(err)).toBe(true); + expect(String(err)).toContain('unexpected response shape'); + }); + + it('types a null item on the query path (httpEmbedQuery, #2385)', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ ok: true, json: async () => ({ data: [null] }) }), + ); + + const { httpEmbedQuery, isHttpEmbeddingError } = + await import('../../src/core/embeddings/http-client.js'); + const err = await httpEmbedQuery('test').catch((e: unknown) => e); + expect(isHttpEmbeddingError(err)).toBe(true); + expect(String(err)).toContain('unexpected response shape'); }); it('excludes API key from error messages', async () => { @@ -220,6 +316,42 @@ describe('HTTP embedding backend', () => { } }); + it('scrubs credentials embedded in the endpoint URL from the error message (#2385)', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'https://user:secret@host.example/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + // undici rejects a credential-bearing URL at Request construction, echoing + // the full URL (incl. user:secret) verbatim in err.message. + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockRejectedValue( + new TypeError( + 'Request cannot be constructed from a URL that includes credentials: ' + + 'https://user:secret@host.example/v1/embeddings', + ), + ), + ); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(isHttpEmbeddingError(err)).toBe(true); + // The secret is gone; the masked host is retained so the message stays useful. + expect(String(err)).not.toContain('secret'); + expect(String(err)).toContain('host.example'); + }); + + it('leaves a non-credential reason unchanged (no over-scrubbing)', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('fetch failed'))); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(String(err)).toContain('fetch failed'); + }); + it('includes abort signal for timeout', async () => { process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; @@ -333,7 +465,12 @@ describe('HTTP embedding backend', () => { ); const mod = await import('../../src/mcp/core/embedder.js'); - await expect(mod.embedQuery('test')).rejects.toThrow('empty response'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await mod.embedQuery('test').catch((e: unknown) => e); + expect(String(err)).toContain('empty response'); + // Type-completeness fence: this conversion must stay typed so the CLI + // routes it to the endpoint branch, not the HF branch (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); }); it('throws when endpoint returns fewer embeddings than texts', async () => { @@ -349,9 +486,11 @@ describe('HTTP embedding backend', () => { ); const { embedBatch } = await import('../../src/core/embeddings/embedder.js'); - await expect(embedBatch(['text1', 'text2', 'text3'])).rejects.toThrow( - '1 vectors for 3 texts', - ); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedBatch(['text1', 'text2', 'text3']).catch((e: unknown) => e); + expect(String(err)).toContain('1 vectors for 3 texts'); + // Type-completeness fence (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); }); it('throws on dimension mismatch when GITNEXUS_EMBEDDING_DIMS is set', async () => { @@ -368,7 +507,11 @@ describe('HTTP embedding backend', () => { ); const { embedText } = await import('../../src/core/embeddings/embedder.js'); - await expect(embedText('test')).rejects.toThrow('Embedding dimension mismatch'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(String(err)).toContain('Embedding dimension mismatch'); + // Type-completeness fence (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); }); }); @@ -397,7 +540,11 @@ describe('HTTP embedding backend', () => { vi.stubGlobal('fetch', vi.fn().mockRejectedValue(timeoutErr)); const { embedText } = await import('../../src/core/embeddings/embedder.js'); - await expect(embedText('test')).rejects.toThrow('timed out'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + expect(String(err)).toContain('timed out'); + // Type-completeness fence: a timeout must stay classifiable (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); expect(fetch).toHaveBeenCalledTimes(1); }); @@ -433,7 +580,11 @@ describe('HTTP embedding backend', () => { ); const mod = await import('../../src/mcp/core/embedder.js'); - await expect(mod.embedQuery('test')).rejects.toThrow('dimension mismatch'); + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + const err = await mod.embedQuery('test').catch((e: unknown) => e); + expect(String(err)).toContain('dimension mismatch'); + // Type-completeness fence: the query-path conversion must stay typed (#2385). + expect(isHttpEmbeddingError(err)).toBe(true); }); it('throws with Set hint when GITNEXUS_EMBEDDING_DIMS is unset', async () => { @@ -454,3 +605,77 @@ describe('HTTP embedding backend', () => { }); }); }); + +describe('HttpEmbeddingError classification', () => { + it('recognises an HttpEmbeddingError instance', async () => { + const { HttpEmbeddingError, isHttpEmbeddingError } = + await import('../../src/core/embeddings/http-client.js'); + expect(isHttpEmbeddingError(new HttpEmbeddingError('anything at all'))).toBe(true); + }); + + it('recognises a cross-realm error by name even when instanceof fails', async () => { + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + // Simulates an error that crossed a module boundary and lost its prototype + // chain: instanceof would be false, but the stable `name` still identifies it. + const crossRealm = new Error('endpoint down'); + crossRealm.name = 'HttpEmbeddingError'; + expect(isHttpEmbeddingError(crossRealm)).toBe(true); + }); + + it.each([ + new Error('TypeError: fetch failed'), + new Error('Failed to download embedding model'), + new Error('connect ECONNREFUSED 127.0.0.1:443'), + 'not even an error', + undefined, + ])('does not claim non-endpoint value: %s', async (value) => { + const { isHttpEmbeddingError } = await import('../../src/core/embeddings/http-client.js'); + expect(isHttpEmbeddingError(value)).toBe(false); + }); +}); + +describe('HTTP mode config probe (#2385)', () => { + const ENV_KEYS = [ + 'GITNEXUS_EMBEDDING_URL', + 'GITNEXUS_EMBEDDING_MODEL', + 'GITNEXUS_EMBEDDING_DIMS', + ] as const; + const savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + + afterEach(() => { + vi.resetModules(); + for (const key of ENV_KEYS) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; + } + } + }); + + it('isHttpMode() is a presence probe that does NOT throw on a malformed DIMS', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + process.env.GITNEXUS_EMBEDDING_DIMS = '1024abc'; + + const { isHttpMode } = await import('../../src/core/embeddings/http-client.js'); + // Root-cause fix: the mode probe must not validate DIMS, so ~13 unguarded + // call sites (analyze:1109, doctor, run-analyze, embedder, mcp) don't crash. + expect(isHttpMode()).toBe(true); + }); + + it('surfaces a malformed DIMS as a recognizable plain config error, not an endpoint error', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + process.env.GITNEXUS_EMBEDDING_DIMS = '1024abc'; + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const { isHttpEmbeddingDimsError, isHttpEmbeddingError } = + await import('../../src/core/embeddings/http-client.js'); + const err = await embedText('test').catch((e: unknown) => e); + // Validated where it's used (readConfig in httpEmbed) and recognizable... + expect(isHttpEmbeddingDimsError(String(err))).toBe(true); + // ...as a plain config Error, NOT an HttpEmbeddingError endpoint failure. + expect(isHttpEmbeddingError(err)).toBe(false); + }); +});