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
10 changed files with 137 additions and 49 deletions
@@ -233,14 +233,13 @@ keep it or `git checkout` to throw it away. Nothing is committed for you.
**For a skill that ships with gbrain** (anything under the gbrain repo's own
`skills/`): SkillOpt refuses to overwrite it by default and writes the winner to
`skills/<name>/skillopt/proposed.md` instead (while keeping `best.md` as the
optimizer's current-best pointer), so an optimization pass can never silently
mutate a skill other people depend on. Two ways to handle that:
`skills/<name>/skillopt/best.md` instead, so an optimization pass can never
silently mutate a skill other people depend on. Two ways to handle that:
```bash
# See the proposed improvement without touching SKILL.md (works for ANY skill):
gbrain skillopt meeting-prep --split 1:1:1 --no-mutate
# → writes skills/meeting-prep/skillopt/proposed.md, updates best.md, and prints the proposal path.
# → writes skills/meeting-prep/skillopt/best.md (the proposed rewrite), prints its path. Copy what you want.
# Actually rewrite a bundled skill (explicit opt-in + an independent held-out set):
gbrain skillopt brain-ops --split 1:1:1 --allow-mutate-bundled \
+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',
+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
+4 -10
View File
@@ -93,13 +93,7 @@ import { resolveLrSchedule } from './lr-schedule.ts';
import { preflight, formatPreflightReport } from './preflight.ts';
import { isRejected, loadRejectedBuffer, makeRejectedEntry, saveRejectedBuffer } from './rejected-buffer.ts';
import { runReflect, runOneShotRewrite, describeJudges } from './reflect.ts';
import {
acceptCandidate,
proposedPath as proposedFilePath,
revertAllPending,
skillPath,
writeProposed,
} from './version-store.ts';
import { acceptCandidate, bestPath, revertAllPending, skillPath, writeProposed } from './version-store.ts';
import { runValidationGate, scoreSkillOnTasks } from './validate-gate.ts';
import { ROLLOUT_SUCCESS_THRESHOLD } from './types.ts';
import type { SkillOptOpts, EditOp, RunReceipt, BenchmarkTask } from './types.ts';
@@ -708,9 +702,9 @@ async function runOptimizationLoop(
// to the catch's assignment values only (it can't prove the async callback ran).
const finalOutcome = outcome as 'accepted' | 'no_improvement' | 'aborted' | 'errored';
if (!mutateDecision.mutate && finalOutcome === 'accepted') {
// writeProposed() emitted both the best pointer and the stable review
// artifact in the accept branch. SKILL.md remains untouched.
proposedPath = proposedFilePath(skillsDir, skillName);
// best.md was written by writeProposed() in the accept branch (no-mutate
// path); it doubles as proposed.md for human review. SKILL.md untouched.
proposedPath = bestPath(skillsDir, skillName);
} else if (mutateDecision.mutate) {
mutatedSkillFile = finalOutcome === 'accepted';
}
+9 -15
View File
@@ -23,7 +23,6 @@
*
* history.json
* best.md
* proposed.md
* versions/
* v0001_e1_s1.md
* v0002_e1_s2.md
@@ -53,10 +52,6 @@ export function bestPath(skillsDir: string, skillName: string): string {
return path.join(skilloptDir(skillsDir, skillName), 'best.md');
}
export function proposedPath(skillsDir: string, skillName: string): string {
return path.join(skilloptDir(skillsDir, skillName), 'proposed.md');
}
export function skillPath(skillsDir: string, skillName: string): string {
return path.join(skillsDir, skillName, 'SKILL.md');
}
@@ -176,18 +171,17 @@ export function acceptCandidate(input: AcceptInput): AcceptResult {
}
/**
* Write the candidate to both `best.md` and `proposed.md` WITHOUT touching
* SKILL.md or the history ledger. `best.md` remains the optimizer's current
* best pointer; `proposed.md` is the stable human-review artifact promised by
* `--no-mutate`. Returns the proposal path. Each write is atomic (.tmp + rename).
* Write the candidate to `best.md` (which doubles as `proposed.md`) WITHOUT
* touching SKILL.md or the history ledger. Used by the `--no-mutate` /
* bundled-without-allow paths: the optimizer found a better candidate but the
* caller opted out of in-place mutation, so we surface it for human review.
* Returns the path written. Atomic (.tmp + rename).
*/
export function writeProposed(skillsDir: string, skillName: string, candidateText: string): string {
const best = bestPath(skillsDir, skillName);
const proposed = proposedPath(skillsDir, skillName);
fs.mkdirSync(path.dirname(best), { recursive: true });
atomicWrite(best, candidateText);
atomicWrite(proposed, candidateText);
return proposed;
const p = bestPath(skillsDir, skillName);
fs.mkdirSync(path.dirname(p), { recursive: true });
atomicWrite(p, candidateText);
return p;
}
/**
+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.
+4 -4
View File
@@ -39,7 +39,6 @@ import { runSkillOpt } from '../../src/core/skillopt/orchestrator.ts';
import {
bestPath,
loadHistory,
proposedPath,
skillPath,
} from '../../src/core/skillopt/version-store.ts';
import { loadRejectedBuffer } from '../../src/core/skillopt/rejected-buffer.ts';
@@ -742,7 +741,7 @@ describe('skillopt T3 — F11 held-out gate, ablation opts, no-DB-pollution', ()
} finally { fixture.cleanup(); }
});
test('--no-mutate writes proposed.md and best.md, leaves SKILL.md untouched', async () => {
test('--no-mutate writes proposed.md (best.md), leaves SKILL.md untouched', async () => {
const fixture = setupFixture(SKILL_PEOPLE_ONLY, CITATIONS_BENCHMARK);
try {
installStub({
@@ -754,9 +753,10 @@ describe('skillopt T3 — F11 held-out gate, ablation opts, no-DB-pollution', ()
const result = await runOnce(fixture, { noMutate: true });
expect(result.outcome).toBe('accepted');
expect(result.mutatedSkillFile).toBe(false);
expect(result.proposedPath).toBe(proposedPath(fixture.skillsDir, SKILL));
expect(result.proposedPath).toBeDefined();
// proposed.md (best.md) exists and carries the improvement.
expect(fs.existsSync(result.proposedPath!)).toBe(true);
expect(fs.readFileSync(result.proposedPath!, 'utf8')).toContain('## Citations');
expect(fs.readFileSync(bestPath(fixture.skillsDir, SKILL), 'utf8')).toContain('## Citations');
// SKILL.md on disk is UNCHANGED (still People-only).
const skill = fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8');
expect(skill).not.toContain('## Citations');
+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 = {
-15
View File
@@ -12,11 +12,9 @@ import {
bestPath,
historyPath,
loadHistory,
proposedPath,
revertAllPending,
skillPath,
versionsDir,
writeProposed,
} from '../../src/core/skillopt/version-store.ts';
let tmpDir: string;
@@ -81,19 +79,6 @@ describe('acceptCandidate (D8 two-phase commit)', () => {
});
});
describe('writeProposed', () => {
test('writes distinct best and proposed artifacts without mutating SKILL.md (#2635)', () => {
const candidate = '---\nname: test\n---\nproposed body\n';
const written = writeProposed(tmpDir, SKILL, candidate);
expect(written).toBe(proposedPath(tmpDir, SKILL));
expect(fs.readFileSync(bestPath(tmpDir, SKILL), 'utf8')).toBe(candidate);
expect(fs.readFileSync(proposedPath(tmpDir, SKILL), 'utf8')).toBe(candidate);
expect(fs.readFileSync(skillPath(tmpDir, SKILL), 'utf8')).toContain('baseline body');
});
});
describe('revertAllPending (D8 crash recovery)', () => {
test('no-op when no pending rows', () => {
const reverted = revertAllPending(tmpDir, SKILL);