Compare commits

..
Author SHA1 Message Date
fffa779172 fix(rerank): classify missing auth before fallback (#2059)
Missing ZEROENTROPY_API_KEY threw AIConfigError from auth resolution, which
rerank.ts recorded as reason 'unknown' — and doctor's reranker_health had no
unknown bucket, so it reported ok while every rerank silently failed open.

- gateway.rerank wraps AIConfigError from applyResolveAuth as
  RerankError(reason: 'auth') before any HTTP call.
- checkRerankerHealth warns on >=3 'unknown' failures in the 7-day window
  (covers historical pre-fix audit rows), with a ZEROENTROPY_API_KEY setup
  hint when the error summary points at a missing key.
- Tests: RerankError(auth) classification, applyReranker fail-open + audit
  reason, doctor warn on repeated unknowns.

Takeover of #2070.

Co-authored-by: maxpetrusenkoagent <maxpetrusenkoagent@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:43:03 -07:00
11 changed files with 133 additions and 147 deletions
+18
View File
@@ -1551,6 +1551,24 @@ export async function checkRerankerHealth(engine: BrainEngine): Promise<Check> {
};
}
// Historical #2059 rows were logged as `unknown` before missing reranker
// auth was classified at the gateway. Surface repeated unknowns instead of
// reporting "ok" while every rerank fails open.
const unknownFails = failures.filter((f) => f.reason === 'unknown');
if (unknownFails.length >= 3) {
const setupHint = unknownFails.some((f) => {
const summary = String(f.error_summary ?? '');
return summary.includes('ZEROENTROPY_API_KEY') || summary.toLowerCase().includes('api key');
})
? ' Fix: verify ZEROENTROPY_API_KEY and run `gbrain models doctor`.'
: '';
return {
name: 'reranker_health',
status: 'warn',
message: `${unknownFails.length} unknown reranker failure(s) in last 7 days.${setupHint}`,
};
}
return {
name: 'reranker_health',
status: 'ok',
+12 -42
View File
@@ -161,7 +161,7 @@ interface ResolveAIOptionsArgs {
nonInteractive: boolean; // --non-interactive (forces D3 fail-loud, no picker)
}
export interface ResolvedAIOptions {
interface ResolvedAIOptions {
embedding_model?: string;
embedding_dimensions?: number;
expansion_model?: string;
@@ -170,41 +170,6 @@ export interface ResolvedAIOptions {
noEmbedding?: boolean;
}
/**
* Seed init's AI options from persisted config, falling back to the raw env
* vars when loadConfig() returned null (#1058). On a cold install (no
* config.json AND no DATABASE_URL) loadConfig short-circuits BEFORE its env
* merge, so GBRAIN_EMBEDDING_MODEL / GBRAIN_EMBEDDING_DIMENSIONS /
* GBRAIN_EXPANSION_MODEL / GBRAIN_CHAT_MODEL were silently ignored by init
* and Tier-3 detection auto-picked by API key instead. Exported for unit
* tests (env injectable).
*/
export function seedAIOptionsFromConfig(
cfg: GBrainConfig | null,
env: NodeJS.ProcessEnv = process.env,
): ResolvedAIOptions {
const envDims = env.GBRAIN_EMBEDDING_DIMENSIONS
? parseInt(env.GBRAIN_EMBEDDING_DIMENSIONS, 10)
: NaN;
const seed = cfg ?? {
embedding_disabled: undefined,
embedding_model: env.GBRAIN_EMBEDDING_MODEL,
embedding_dimensions: Number.isFinite(envDims) ? envDims : undefined,
expansion_model: env.GBRAIN_EXPANSION_MODEL,
chat_model: env.GBRAIN_CHAT_MODEL,
};
const out: ResolvedAIOptions = {};
if (seed.embedding_disabled) {
out.noEmbedding = true;
} else if (seed.embedding_model) {
out.embedding_model = seed.embedding_model;
if (seed.embedding_dimensions) out.embedding_dimensions = seed.embedding_dimensions;
}
if (seed.expansion_model) out.expansion_model = seed.expansion_model;
if (seed.chat_model) out.chat_model = seed.chat_model;
return out;
}
/**
* Resolve AI provider options for `gbrain init`.
*
@@ -238,13 +203,18 @@ async function resolveAIOptions(opts: ResolveAIOptionsArgs): Promise<ResolvedAIO
// user already opted into deferred mode.
try {
const { loadConfig } = await import('../core/config.ts');
// #1058: loadConfig() returns null on a cold install (no config.json AND
// no DATABASE_URL) — before it ever reaches its env merge. The seed helper
// falls back to the same GBRAIN_* env vars directly in that case.
Object.assign(out, seedAIOptionsFromConfig(loadConfig()));
const cfg = loadConfig();
if (cfg?.embedding_disabled) {
out.noEmbedding = true;
} else if (cfg?.embedding_model) {
out.embedding_model = cfg.embedding_model;
if (cfg.embedding_dimensions) out.embedding_dimensions = cfg.embedding_dimensions;
}
if (cfg?.expansion_model) out.expansion_model = cfg.expansion_model;
if (cfg?.chat_model) out.chat_model = cfg.chat_model;
} catch {
// loadConfig threw — treat as first-time install, fall through to env
// detection.
// loadConfig throws when no brain configured — first-time install, fall
// through to env detection.
}
// --- Tier 1+2: explicit flags ---------------------------------------------
+9 -1
View File
@@ -3656,7 +3656,15 @@ export async function rerank(input: RerankInput): Promise<RerankResult[]> {
// whose request/response shape differs from ZE/llama.cpp (e.g. Voyage with
// `top_k` / `data[]`) needs separate adapter hooks in a follow-up plan.
const url = `${compat.baseURL.replace(/\/$/, '')}${tp.path ?? '/models/rerank'}`;
const auth = applyResolveAuth(recipe, cfg, 'reranker');
let auth: { apiKey?: string; headers?: Record<string, string> };
try {
auth = applyResolveAuth(recipe, cfg, 'reranker');
} catch (err) {
if (err instanceof AIConfigError) {
throw new RerankError(err.message, 'auth');
}
throw err;
}
// applyResolveAuth returns { apiKey } for Bearer-style auth (SDK's native
// path) or { headers } for custom-header providers (Azure). v0.37.6.0:
// recipes can ALSO declare default_headers (attribution etc.) which flow
+3 -19
View File
@@ -332,15 +332,6 @@ export interface OperationContext {
* remote/untrusted (defense in depth in case the type is bypassed via cast).
*/
remote: boolean;
/**
* Transport marker for auth-less remote surfaces (#1061). The stdio MCP
* dispatch sets 'stdio' — it is deliberately `remote: true` (agent-facing,
* untrusted) but has no per-token auth (local pipe), so identity ops like
* whoami need a way to distinguish "known auth-less transport" from "a
* transport bug forgot to thread ctx.auth". Trust decisions MUST NOT key
* off this field — only `ctx.remote === false` grants trust.
*/
transport?: 'stdio';
/**
* Subagent runtime context (v0.16+). Set by the subagent tool dispatcher when
* dispatching an op as a tool call from an LLM loop. Used to enforce per-op
@@ -3722,10 +3713,9 @@ const whoami: Operation = {
'Introspect the calling identity. Returns one of three transport shapes: ' +
'{transport: "oauth", client_id, client_name, scopes, expires_at}, ' +
'{transport: "legacy", token_name, scopes, expires_at: null}, or ' +
'{transport: "local", scopes: []}, or {transport: "stdio", scopes: []} ' +
'for the auth-less stdio MCP pipe. Throws unknown_transport when the ' +
'context is ambiguous (remote=true without auth and no transport marker) ' +
'— fail-closed posture mirroring the v0.26.9 trust-boundary contract.',
'{transport: "local", scopes: []}. Throws unknown_transport when the ' +
'context is ambiguous (remote=true without auth) — fail-closed posture ' +
'mirroring the v0.26.9 trust-boundary contract.',
params: {},
scope: 'read',
handler: async (ctx) => {
@@ -3737,12 +3727,6 @@ const whoami: Operation = {
if (ctx.remote === false) {
return { transport: 'local', scopes: [] };
}
// #1061: stdio MCP is remote/untrusted by design but has no per-token
// auth (local pipe) — a known transport, not a bug. Report it instead of
// throwing. Empty scopes: nothing here may be used to gate anything.
if (!ctx.auth && ctx.transport === 'stdio') {
return { transport: 'stdio', scopes: [] };
}
if (!ctx.auth) {
throw new OperationError(
'unknown_transport',
-7
View File
@@ -32,12 +32,6 @@ export interface DispatchOpts {
remote?: boolean;
/** Override the default stderr logger (e.g. CLI uses console.* directly). */
logger?: OperationContext['logger'];
/**
* #1061: transport marker for auth-less remote surfaces. The stdio MCP
* server passes 'stdio' so identity ops (whoami) can report the transport
* instead of throwing unknown_transport. Never used for trust decisions.
*/
transport?: OperationContext['transport'];
/**
* v0.28: per-token allow-list for the takes.holder field. Threaded by
* the HTTP/stdio transport from `access_tokens.permissions.takes_holders`.
@@ -209,7 +203,6 @@ export function buildOperationContext(
logger: opts.logger || stderrLogger,
dryRun: !!params.dry_run,
remote: opts.remote ?? true,
transport: opts.transport,
takesHoldersAllowList: opts.takesHoldersAllowList,
// v0.34 D4: sourceId is REQUIRED at the type level. Auto-fill 'default'
// for single-source brains and any caller who didn't resolve a sourceId.
-4
View File
@@ -42,10 +42,6 @@ export async function startMcpServer(engine: BrainEngine) {
// `gbrain call <op>` (sets remote=false in src/cli.ts).
return dispatchToolCall(engine, name, params, {
remote: true,
// #1061: mark the transport so whoami can report {transport: 'stdio'}
// instead of throwing unknown_transport. Trust posture unchanged —
// stdio stays remote/untrusted.
transport: 'stdio',
takesHoldersAllowList: ['world'],
// v0.31: source defaults to 'default' for stdio (no per-token scope).
// Operators who want a different source on stdio MCP should set
+22
View File
@@ -154,6 +154,28 @@ describe('gateway.rerank() — happy path', () => {
describe('gateway.rerank() — error classification', () => {
beforeEach(() => configureZE());
test('missing required reranker API key → RerankError(auth) before HTTP call', async () => {
configureGateway({
reranker_model: 'zeroentropyai:zerank-2',
env: {},
});
let called = false;
__setRerankTransportForTests(async () => {
called = true;
return mockResp({ results: [{ index: 0, relevance_score: 0.5 }] });
});
try {
await rerank({ query: 'q', documents: ['d'] });
throw new Error('should have thrown');
} catch (err) {
expect(err).toBeInstanceOf(RerankError);
expect((err as RerankError).reason).toBe('auth');
expect((err as Error).message).toContain('ZEROENTROPY_API_KEY');
expect(called).toBe(false);
}
});
test('401 → auth', async () => {
__setRerankTransportForTests(async () => new Response('Unauthorized', { status: 401 }));
try {
+33
View File
@@ -2,6 +2,11 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:tes
import { mkdirSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { withEnv } from './helpers/with-env.ts';
import { logRerankFailure } from '../src/core/rerank-audit.ts';
describe('doctor command', () => {
test('doctor module exports runDoctor', async () => {
@@ -47,6 +52,34 @@ describe('doctor command', () => {
expect(check.issues![0].action).toContain('trigger');
});
test('reranker_health warns on repeated unknown rerank failures', async () => {
const { checkRerankerHealth } = await import('../src/commands/doctor.ts');
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-rerank-doctor-'));
try {
await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => {
for (let i = 0; i < 3; i++) {
logRerankFailure({
model: 'zeroentropyai:zerank-2',
reason: 'unknown',
query_hash: `unknown${i}`,
doc_count: 30,
error_summary: 'ZeroEntropy reranker requires ZEROENTROPY_API_KEY.',
});
}
const check = await checkRerankerHealth({
async getConfig(key: string): Promise<string | null> {
return key === 'search.reranker.enabled' ? 'true' : null;
},
} as any);
expect(check.status).toBe('warn');
expect(check.message).toContain('unknown');
expect(check.message).toContain('ZEROENTROPY_API_KEY');
});
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
test('runDoctor accepts null engine for filesystem-only mode', async () => {
const { runDoctor } = await import('../src/commands/doctor.ts');
// runDoctor should accept null engine — it runs filesystem checks only.
+1 -45
View File
@@ -12,7 +12,7 @@
*/
import { describe, test, expect } from 'bun:test';
import { groupReadyByProvider, findEnvKeyTypos, seedAIOptionsFromConfig } from '../src/commands/init.ts';
import { groupReadyByProvider, findEnvKeyTypos } from '../src/commands/init.ts';
describe('groupReadyByProvider — embedding touchpoint', () => {
test('OPENAI_API_KEY alone → openai is ready', async () => {
@@ -149,47 +149,3 @@ describe('findEnvKeyTypos', () => {
expect(got.find(t => t.userSet === 'COMPLETELY_UNRELATED_KEY')).toBeUndefined();
});
});
describe('seedAIOptionsFromConfig — #1058 cold-install env fallback', () => {
test('null config (no config.json, no DATABASE_URL) falls back to GBRAIN_* env vars', () => {
const got = seedAIOptionsFromConfig(null, {
GBRAIN_EMBEDDING_MODEL: 'voyage:voyage-3-large',
GBRAIN_EMBEDDING_DIMENSIONS: '1024',
GBRAIN_EXPANSION_MODEL: 'openai:gpt-5-mini',
GBRAIN_CHAT_MODEL: 'anthropic:claude-sonnet-4-6',
});
expect(got.embedding_model).toBe('voyage:voyage-3-large');
expect(got.embedding_dimensions).toBe(1024);
expect(got.expansion_model).toBe('openai:gpt-5-mini');
expect(got.chat_model).toBe('anthropic:claude-sonnet-4-6');
});
test('null config + no env vars → empty seed (Tier-3 detection takes over)', () => {
const got = seedAIOptionsFromConfig(null, {});
expect(got).toEqual({});
});
test('persisted config wins (loadConfig already merged env when non-null)', () => {
const got = seedAIOptionsFromConfig(
{ engine: 'pglite', embedding_model: 'openai:text-embedding-3-small', embedding_dimensions: 1536 } as any,
{ GBRAIN_EMBEDDING_MODEL: 'voyage:voyage-3-large' },
);
expect(got.embedding_model).toBe('openai:text-embedding-3-small');
expect(got.embedding_dimensions).toBe(1536);
});
test('embedding_disabled sentinel honored on re-init', () => {
const got = seedAIOptionsFromConfig({ engine: 'pglite', embedding_disabled: true } as any, {});
expect(got.noEmbedding).toBe(true);
expect(got.embedding_model).toBeUndefined();
});
test('non-numeric GBRAIN_EMBEDDING_DIMENSIONS ignored, model still seeds', () => {
const got = seedAIOptionsFromConfig(null, {
GBRAIN_EMBEDDING_MODEL: 'voyage:voyage-3-large',
GBRAIN_EMBEDDING_DIMENSIONS: 'not-a-number',
});
expect(got.embedding_model).toBe('voyage:voyage-3-large');
expect(got.embedding_dimensions).toBeUndefined();
});
});
+35
View File
@@ -12,9 +12,14 @@
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { applyReranker, type RerankerOpts } from '../../src/core/search/rerank.ts';
import { RerankError, type RerankResult } from '../../src/core/ai/gateway.ts';
import { readRecentRerankFailures } from '../../src/core/rerank-audit.ts';
import type { SearchResult } from '../../src/core/types.ts';
import { withEnv } from '../helpers/with-env.ts';
function makeResult(slug: string, score: number, chunk: string): SearchResult {
return {
@@ -160,6 +165,36 @@ describe('applyReranker — fail-open on every RerankError reason', () => {
expect(out).toEqual(results);
});
test('missing gateway reranker API key fail-opens and audits auth', async () => {
const { configureGateway } = await import('../../src/core/ai/gateway.ts');
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-rerank-search-'));
try {
await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => {
configureGateway({
reranker_model: 'zeroentropyai:zerank-2',
env: {},
});
const results = [makeResult('a', 1.0, 'doc a')];
const out = await applyReranker('q', results, {
enabled: true,
topNIn: 1,
topNOut: null,
model: 'zeroentropyai:zerank-2',
});
expect(out).toEqual(results);
const failures = readRecentRerankFailures(1);
expect(failures).toHaveLength(1);
expect(failures[0]!.reason).toBe('auth');
expect(failures[0]!.error_summary).toContain('ZEROENTROPY_API_KEY');
});
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
configureGateway({ env: { ZEROENTROPY_API_KEY: 'test-key' } });
}
});
test('fail-open on non-RerankError throw too', async () => {
const results = [makeResult('a', 1.0, 'a')];
const opts: RerankerOpts = {
-29
View File
@@ -94,35 +94,6 @@ describe('whoami op contract', () => {
expect(result.expires_at).toBeNull();
});
// #1061: stdio MCP is remote/untrusted by design but has no per-token auth
// (local pipe). The stdio dispatch marks ctx.transport='stdio'; whoami
// reports it instead of throwing unknown_transport.
test('stdio transport (remote=true, no auth, transport marker) reports stdio', async () => {
const result = (await whoami.handler(
ctxWith({ remote: true, auth: undefined, transport: 'stdio' }),
{},
)) as any;
expect(result.transport).toBe('stdio');
expect(result.scopes).toEqual([]);
});
test('stdio marker does not mask real auth (auth still wins)', async () => {
const result = (await whoami.handler(
ctxWith({
remote: true,
transport: 'stdio',
auth: {
token: 'gbrain_at_xxx',
clientId: 'gbrain_cl_abc',
scopes: ['read'],
expiresAt: 1,
} as AuthInfo,
}),
{},
)) as any;
expect(result.transport).toBe('oauth');
});
// Q3: ambiguous transport — fail-closed. The footgun this guards against
// is a future transport that lands without threading auth, where a buggy
// caller might trust whoami's output to gate sensitive ops.