mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
* fix(providers): reuse buildGatewayConfig for --model test override (#2863)
`gbrain providers test --model <id>` overrode the gateway with only
embedding_model/chat_model + env, dropping config.provider_base_urls
entirely. A brain configured with a custom endpoint (e.g. a China-region
DashScope base URL) would pass the bare `providers test` (which goes
through configureFromEnv() and does forward base_urls) but fail the
`--model`-scoped probe with a misleading "Incorrect API key" error, even
though the key was valid for the configured endpoint — the probe silently
fell back to the recipe's hardcoded default endpoint instead.
Root cause: two independent, drifted resolvers. The production path
(src/cli.ts#connectEngine, src/core/init-embed-check.ts) builds its
AIGatewayConfig via buildGatewayConfig(), which folds provider_base_urls,
env-sourced local-server base URLs, provider_chat_options, and file-plane
API keys. The --model override branch in runTest() hand-rolled a second,
narrower config object that only carried the overridden model + raw env.
Fix: lift `cfg` out of the existing try/catch (it was already loaded there
for the isolation-warning message) and spread `buildGatewayConfig(cfg)`
into both configureGateway() calls before overriding embedding_model/
chat_model. The isolated --model probe now resolves its endpoint exactly
the way the brain's real import/query path would; only the requested
model is overridden, so the probe still targets exactly the model the
user asked for. Falls back to bare env when no brain is configured yet
(cfg is null), matching prior first-time-install behavior.
Confirmed chat_fallback_chain (also threaded through by buildGatewayConfig)
has no runtime retry effect — it's only consumed to pre-register extended
model ids — so spreading the full production config does not mask an
isolated model's own failures behind a silent fallback.
Other diagnostic surfaces (providers list/env/explain) were checked and
are unaffected: `runProviders()` already calls configureFromEnv() (which
forwards base_urls correctly) before dispatch, and none of them accept
--model, so they never hit the broken override branch.
Adds test/providers-test-model-base-url.test.ts: drives runProviders('test',
...) end-to-end against a mocked fetch + temp GBRAIN_HOME/config.json with
provider_base_urls set for the dashscope recipe (the exact recipe named in
the bug report), asserting the outbound request hits the configured base
URL rather than the recipe default. Verified red on pre-fix code via
git stash, green after.
Closes #2863
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: drop duplicate buildGatewayConfig import after master merge
Master's f3e78fd2 added the same import the PR carried; the textual
merge was clean but the result failed typecheck (TS2300 duplicate
identifier).
Co-authored-by: masashiono0611 <masashi.ono.0611@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: masashiono0611 <masashi.ono.0611@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Garry Tan <garrytan@gmail.com>
This commit is contained in:
co-authored by
masashiono0611
Claude Fable 5
Garry Tan
parent
b6dd3e1121
commit
e320ad71b3
@@ -7,9 +7,9 @@
|
||||
|
||||
import { listRecipes, getRecipe } from '../core/ai/recipes/index.ts';
|
||||
import { configureGateway, embedOne, isAvailable as gwIsAvailable, chat as gwChat } from '../core/ai/gateway.ts';
|
||||
import { buildGatewayConfig } from '../core/ai/build-gateway-config.ts';
|
||||
import { probeOllama, probeLMStudio } from '../core/ai/probes.ts';
|
||||
import { loadConfig } from '../core/config.ts';
|
||||
import { buildGatewayConfig } from '../core/ai/build-gateway-config.ts';
|
||||
import { AIConfigError, AITransientError } from '../core/ai/errors.ts';
|
||||
import type { Recipe } from '../core/ai/types.ts';
|
||||
|
||||
@@ -173,8 +173,18 @@ async function runTest(args: string[]): Promise<void> {
|
||||
// the divergence at the top of the test so the recovery experience
|
||||
// doesn't repeat the bug-reporter's "providers test ✓ but import still
|
||||
// broken" trap.
|
||||
//
|
||||
// #2863: `cfg` is lifted out of the try block (not just used for the
|
||||
// warning) so the configureGateway calls below can reuse it. Before this
|
||||
// fix, the --model override only forwarded embedding_model/chat_model +
|
||||
// env, dropping config.provider_base_urls entirely — a probe against a
|
||||
// custom endpoint (e.g. a regional DashScope base URL) would silently
|
||||
// fall back to the recipe's hardcoded default endpoint and fail with a
|
||||
// misleading "Incorrect API key" error even though the key was valid for
|
||||
// the configured endpoint.
|
||||
let cfg: ReturnType<typeof loadConfig> | null = null;
|
||||
try {
|
||||
const cfg = loadConfig();
|
||||
cfg = loadConfig();
|
||||
const configuredModel = tpArg === 'embedding' ? cfg?.embedding_model : cfg?.chat_model;
|
||||
if (!configuredModel) {
|
||||
console.error(
|
||||
@@ -190,17 +200,27 @@ async function runTest(args: string[]): Promise<void> {
|
||||
}
|
||||
} catch { /* loadConfig throws when no brain configured — first-time install path; the no-config branch above handles it. */ }
|
||||
|
||||
// Reuse the SAME resolver the production path uses (buildGatewayConfig —
|
||||
// also used by cli.ts#connectEngine and init-embed-check.ts) so the probe
|
||||
// sees the identical base_urls / provider_chat_options / folded API keys
|
||||
// that a real `gbrain import`/`gbrain query` call would. Only the
|
||||
// touchpoint's model (+ embedding dims) is overridden on top, so an
|
||||
// isolated `--model` probe still targets exactly the requested model —
|
||||
// it just resolves that model's endpoint the way the brain actually
|
||||
// would. Falls back to bare env when no brain is configured yet (cfg is
|
||||
// null on first-time install, matching the old behavior for that case).
|
||||
const baseGatewayConfig = cfg ? buildGatewayConfig(cfg) : { env: { ...process.env } };
|
||||
if (tpArg === 'embedding') {
|
||||
const dims = recipe?.touchpoints.embedding?.default_dims ?? 1536;
|
||||
configureGateway({
|
||||
...baseGatewayConfig,
|
||||
embedding_model: modelArg,
|
||||
embedding_dimensions: dims,
|
||||
env: { ...process.env },
|
||||
});
|
||||
} else {
|
||||
configureGateway({
|
||||
...baseGatewayConfig,
|
||||
chat_model: modelArg,
|
||||
env: { ...process.env },
|
||||
});
|
||||
}
|
||||
void modelId; // intentionally unused but preserved for readability
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
/**
|
||||
* #2863 regression — `gbrain providers test --model` must resolve
|
||||
* `provider_base_urls` the same way the production embed/chat path does.
|
||||
*
|
||||
* Before the fix, the `--model` override branch in `runTest`
|
||||
* (src/commands/providers.ts) forwarded only `embedding_model`/`chat_model`
|
||||
* + `env` into `configureGateway`, dropping `config.provider_base_urls`
|
||||
* entirely. A brain configured with a custom (e.g. China-region DashScope)
|
||||
* endpoint would pass `gbrain providers test --touchpoint embedding` (no
|
||||
* `--model`, uses configureFromEnv() which DOES forward base_urls) but fail
|
||||
* `gbrain providers test --touchpoint embedding --model
|
||||
* dashscope:text-embedding-v3` with a misleading "Incorrect API key" error
|
||||
* — the probe silently fell back to the recipe's hardcoded default endpoint
|
||||
* (dashscope-intl.aliyuncs.com) instead of the configured one.
|
||||
*
|
||||
* This test drives the real `runProviders('test', ...)` CLI path end to end
|
||||
* (loadConfig -> configureGateway -> gateway -> AI SDK -> fetch) and asserts
|
||||
* on the actual HTTP request URL, so it fails on the pre-fix code and only
|
||||
* passes once the --model override reuses buildGatewayConfig (the same
|
||||
* resolver src/cli.ts#connectEngine and init-embed-check.ts use for the
|
||||
* production path).
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { runProviders } from '../src/commands/providers.ts';
|
||||
import { resetGateway } from '../src/core/ai/gateway.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
const CUSTOM_BASE_URL = 'https://llm-custom.cn-beijing.maas.example.test/compatible-mode/v1';
|
||||
|
||||
type FetchHandler = (url: string, init: RequestInit) => Promise<Response>;
|
||||
let fetchHandler: FetchHandler | null = null;
|
||||
const origFetch = globalThis.fetch;
|
||||
let tmpHome: string;
|
||||
|
||||
function okEmbeddingResponse(dims: number): Response {
|
||||
const vec = Array(dims).fill(0).map((_, i) => 0.001 * i);
|
||||
return new Response(
|
||||
JSON.stringify({ data: [{ embedding: vec }] }),
|
||||
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
fetchHandler = null;
|
||||
globalThis.fetch = (async (url: string | URL | Request, init?: RequestInit) => {
|
||||
if (!fetchHandler) throw new Error('fetch called but no handler installed');
|
||||
return fetchHandler(typeof url === 'string' ? url : url.toString(), init ?? {});
|
||||
}) as typeof fetch;
|
||||
|
||||
tmpHome = mkdtempSync(join(tmpdir(), 'gbrain-providers-test-base-url-'));
|
||||
mkdirSync(join(tmpHome, '.gbrain'), { recursive: true });
|
||||
writeFileSync(
|
||||
join(tmpHome, '.gbrain', 'config.json'),
|
||||
JSON.stringify({
|
||||
embedding_model: 'dashscope:text-embedding-v3',
|
||||
embedding_dimensions: 1024,
|
||||
provider_base_urls: { dashscope: CUSTOM_BASE_URL },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = origFetch;
|
||||
resetGateway();
|
||||
rmSync(tmpHome, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('providers test --model — provider_base_urls (#2863)', () => {
|
||||
test('embedding touchpoint probe hits the configured custom base URL, not the recipe default', async () => {
|
||||
let capturedUrl = '';
|
||||
fetchHandler = async (url) => {
|
||||
capturedUrl = url;
|
||||
return okEmbeddingResponse(1024);
|
||||
};
|
||||
|
||||
await withEnv(
|
||||
{ GBRAIN_HOME: tmpHome, DASHSCOPE_API_KEY: 'test-dashscope-key' },
|
||||
async () => {
|
||||
await runProviders('test', ['--touchpoint', 'embedding', '--model', 'dashscope:text-embedding-v3']);
|
||||
},
|
||||
);
|
||||
|
||||
expect(capturedUrl.startsWith(CUSTOM_BASE_URL)).toBe(true);
|
||||
expect(capturedUrl).not.toContain('dashscope-intl.aliyuncs.com');
|
||||
});
|
||||
|
||||
test('bare `providers test` (no --model) already used the custom base URL (control)', async () => {
|
||||
let capturedUrl = '';
|
||||
fetchHandler = async (url) => {
|
||||
capturedUrl = url;
|
||||
return okEmbeddingResponse(1024);
|
||||
};
|
||||
|
||||
await withEnv(
|
||||
{ GBRAIN_HOME: tmpHome, DASHSCOPE_API_KEY: 'test-dashscope-key' },
|
||||
async () => {
|
||||
await runProviders('test', ['--touchpoint', 'embedding']);
|
||||
},
|
||||
);
|
||||
|
||||
expect(capturedUrl.startsWith(CUSTOM_BASE_URL)).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user