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
9 changed files with 129 additions and 84 deletions
-7
View File
@@ -438,13 +438,6 @@ export async function runApplyMigrations(args: string[]): Promise<void> {
const result = await m.orchestrator(orchestratorOptsFrom(cli));
if (result.status === 'failed') {
console.error(`Migration v${m.version} reported status=failed.`);
// Surface each failed phase's detail — the ledger records it, but
// the operator needs it on stderr to act (#921).
for (const p of result.phases) {
if (p.status === 'failed') {
console.error(` phase ${p.name}: ${p.detail ?? '(no detail)'}`);
}
}
// Record the attempt as 'partial' (not 'complete') so the cap counts
// it. Don't let a failed orchestrator look like it never ran.
try {
+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',
+11 -15
View File
@@ -186,6 +186,17 @@ async function phaseBFenceFacts(
const localPathById = new Map<string, string | null>();
for (const s of sources) localPathById.set(s.id, s.local_path);
// Dirty-tree refusal: check every source's local_path before writing.
for (const [id, localPath] of localPathById) {
if (localPath && isLocalPathDirty(localPath)) {
return {
name: 'fence_facts',
status: 'failed',
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
};
}
}
// Walk legacy rows in (source_id, entity_slug) groups for per-page
// atomic writes.
const legacy = await engine.executeRaw<LegacyFactRow>(
@@ -224,21 +235,6 @@ async function phaseBFenceFacts(
groups.set(key, list);
}
// Dirty-tree refusal: check ONLY the sources we are about to write
// into. A dirty tree in an unrelated source (or zero fenceable rows
// at all) must not block a no-op or a targeted backfill (#927).
const targetSourceIds = new Set([...groups.keys()].map(k => k.split('\0')[0]));
for (const id of targetSourceIds) {
const localPath = localPathById.get(id);
if (localPath && isLocalPathDirty(localPath)) {
return {
name: 'fence_facts',
status: 'failed',
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
};
}
}
for (const [key, group] of groups) {
const [sourceId, entitySlug] = key.split('\0');
const localPath = localPathById.get(sourceId)!;
+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
+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 {
-13
View File
@@ -180,16 +180,3 @@ describe('runApplyMigrations exit codes (v0.36.1.x #1062)', () => {
expect(src).toMatch(/All migrations up to date[\s\S]{0,80}process\.exit\(0\)/);
});
});
// #921: a failed orchestrator must print each failed phase's detail to
// stderr — not just "reported status=failed" — so the operator can act
// without digging through the ledger.
describe('failed migration prints phase detail (#921)', () => {
test('runner loops result.phases and console.errors failed phase details', async () => {
const { readFileSync } = await import('fs');
const src = readFileSync('src/commands/apply-migrations.ts', 'utf8');
expect(src).toMatch(
/reported status=failed[\s\S]{0,400}for \(const p of result\.phases\)[\s\S]{0,200}p\.status === 'failed'[\s\S]{0,200}console\.error\([\s\S]{0,80}p\.name[\s\S]{0,80}p\.detail/,
);
});
});
+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 -48
View File
@@ -10,11 +10,10 @@
* __setTestEngineOverride so we don't need a configured brain.
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { execFileSync } from 'node:child_process';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { v0_32_2, __setTestEngineOverride, __testing } from '../src/commands/migrations/v0_32_2.ts';
@@ -239,52 +238,6 @@ describe('phaseBFenceFacts — happy path backfill', () => {
});
});
describe('phaseBFenceFacts — dirty-tree refusal scoping (#927)', () => {
let dirtyDir: string;
beforeEach(async () => {
// A second source whose local_path is a git repo with uncommitted changes.
dirtyDir = mkdtempSync(join(tmpdir(), 'mig-v0_32_2-dirty-'));
execFileSync('git', ['-C', dirtyDir, 'init', '-q']);
writeFileSync(join(dirtyDir, 'uncommitted.md'), 'dirty', 'utf-8');
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await (engine as any).db.query(
`INSERT INTO sources (id, name, local_path) VALUES ('other', 'other', $1)`,
[dirtyDir],
);
});
afterEach(async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await (engine as any).db.query(`DELETE FROM sources WHERE id = 'other'`);
rmSync(dirtyDir, { recursive: true, force: true });
});
test('no legacy facts at all → complete, dirty unrelated source ignored', async () => {
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('complete');
expect(r.detail).toContain('scanned=0');
});
test('facts scoped to a clean source fence despite dirty unrelated source', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'Founded Acme' });
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('complete');
expect(r.detail).toContain('fenced=1');
expect(existsSync(join(brainDir, 'people/alice.md'))).toBe(true);
});
test('still refuses when the TARGETED source is dirty', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1', source_id: 'other' });
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('failed');
expect(r.detail).toContain('"other"');
expect(r.detail).toContain('uncommitted changes');
});
});
describe('phaseCVerify', () => {
test('returns complete when fence + DB row counts match', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1' });
+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 = {