mirror of
https://github.com/proffesor-for-testing/agentic-qe.git
synced 2026-09-19 08:45:47 +08:00
3bc09cda6d
A post-Phase-4 devil's-advocate audit identified 10 issues. All addressed
with code fixes + tests. Some were real bugs (silent billing regression,
2 unwired MCP tools, 2 HybridRouter routing bugs); others were robustness
improvements (split singletons, silent errors, decorative type fields).
Findings → fixes (severity tagged in audit report):
1. CRITICAL — silent billing regression on upgrade
Users with ANTHROPIC_API_KEY in env would silently start paying for
Claude API calls on every Fleet operation. The CHANGELOG's "opt out
via code changes" wasn't sufficient — needed an env-only escape.
Fix: AQE_LLM_ROUTER_DISABLED=1 env kill-switch honored in both
QEKernelImpl._initializeLLMRouter and src/mcp/tools/base.ts
getSharedLLMRouter. Truthy values disable; 'false'/'0'/'no'/'off'/''
pass. Unified parse rule between kernel and MCP base.
2. HIGH — 2 unwired MCP standalone tools (audit said 3; QualityCriteria
isn't LLM-aware so excluded)
src/mcp/tools/test-generation/generate.ts:82 was calling
createTestGeneratorService(memory) without router. Same for
visual-accessibility/index.ts:177. Both now use the
createXxxServiceWithDependencies factories with { memory, llmRouter }.
3. CRITICAL — E2E only tested manual routing mode, not production
Production default is rule-based. Adding two new E2E tests for
rule-based mode SURFACED two real bugs in HybridRouter that I fixed:
3a. executeWithFallback hardcoded ['claude','openai','ollama'] as the
only fallback providers; gemini/openrouter/azure-openai/bedrock
users had no fallback even though ProviderManager can construct
them. Plus the user's defaultProvider wasn't auto-included in the
chain. Both fixed at src/shared/llm/router/hybrid-router.ts:612.
3b. A non-retryable error from one provider (e.g. Claude "invalid
x-api-key") aborted the entire fallback loop via `throw error`,
defeating the whole point of a fallback chain. Fixed: skip the
inter-attempt delay for non-retryable but continue to the next
fallback entry.
4. HIGH — MCPToolContext.llmRouter field was decorative
Added the field but no code populated it. Every getLLMRouter(context)
call fell through to getSharedLLMRouter().
Fix: added llmRouter and memory to BaseMCPTool.invoke() options;
qe-tool-bridge.ts and registry.ts now explicitly forward the shared
singleton via { llmRouter } at invoke time.
5. HIGH — 14 of 15 wired services had only STRUCTURAL verification, not
behavioral (only TestExecutor had a chat() round-trip test)
Fix: new tests/integration/llm-router-behavioral.test.ts asserts
isLLMAnalysisAvailable()===true (the exact gate the audit named) for
all 14 services + 2 full behavioral round-trips. 16 tests total.
6. HIGH — MCP and kernel built SEPARATE HybridRouter instances
Different cost trackers, caches, circuit breakers per side.
routing_economics/routing_metrics would only see half the traffic.
Fix: kernel.ts._initializeLLMRouter calls setSharedLLMRouter(router)
on init so both paths share one instance. kernel.dispose() calls
resetSharedLLMRouter() so the next kernel doesn't inherit the
previous instance.
7. HIGH — Silent error swallowing in _initializeLLMRouter
console.warn'd then continued without LLM, with no signal for
monitoring. A misconfigured provider would silently degrade with no
alert.
Fix: now publishes kernel.llm-router.init-failed (caught error) and
kernel.llm-router.init-no-provider (enabled=true but nothing
available) events on the event bus. Test confirms event delivery.
8. MEDIUM — Service count mismatch in docs (ADR/CHANGELOG said "14
services / 12 domains"; test enumerates 15 services / 11
LLM-enhanced domains)
Fix: ADR-043 addendum + CHANGELOG corrected to 15/11. Added
explicit service list and noted the 2 non-LLM domains
(enterprise-integration, coordination).
9. MEDIUM — aqe llm config --set apiKey=... silently dropped the key
stripApiKeys() returned a successfully-saved file without the key,
user thought it was saved.
Fix: CLI updateConfig() now throws a clear error pointing to env
vars BEFORE calling save. stripApiKeys also now logs a console.warn
when it strips anything, so programmatic callers see the strip.
10. LOW — Env vs disk precedence was counterintuitive and undocumented
Force-enable-on-env overrides explicit `enabled: false` on disk.
Fix: doc comment in config-store.applyEnvProviderDetection +
ADR-043 "Precedence & env-var conventions" section explains.
Verification
- npx tsc --noEmit: clean
- 191 unit + integration tests pass (16 new behavioral tests for #5,
17 wiring tests, gemini provider, config-store, llm-router-service,
kernel-llm-router with 17 tests including all kill-switch cases,
CLI persistence with 28 tests)
- 5 real-provider E2E tests pass (Gemini 2.5 Flash, OpenAI gpt-4o-mini,
OpenRouter, plus 2 rule-based-mode tests that previously failed and
now pass with the HybridRouter fallback fixes)
Refs ADR-043 audit findings (2026-05-21)
Builds on 94033e3 (Phase 1), be1e60d (Phase 2), c206c81 (Phase 3+4)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
313 lines
11 KiB
TypeScript
313 lines
11 KiB
TypeScript
/**
|
|
* Tests for ADR-043 kernel ↔ LLM router wiring.
|
|
*
|
|
* Covers:
|
|
* - kernel.llmRouter is undefined when llmRouter.enabled = false
|
|
* - kernel.llmRouter is undefined when no provider is available (auto)
|
|
* - kernel.llmRouter is built when an injected ProviderManager is provided
|
|
* - the router is forwarded to plugin factories as the 4th arg
|
|
* - a chat() call round-trips through the mock provider
|
|
*/
|
|
|
|
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
|
import * as fs from 'fs';
|
|
import * as os from 'os';
|
|
import * as path from 'path';
|
|
|
|
import { QEKernelImpl } from '../../../src/kernel/kernel';
|
|
import { resetUnifiedMemory } from '../../../src/kernel/unified-memory';
|
|
import { ProviderManager } from '../../../src/shared/llm/provider-manager';
|
|
import { createMockLLMProvider } from '../../mocks';
|
|
import {
|
|
getSharedLLMRouter,
|
|
resetSharedLLMRouter,
|
|
} from '../../../src/mcp/tools/base';
|
|
|
|
let tmpRoot: string;
|
|
|
|
beforeEach(() => {
|
|
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'aqe-kernel-llm-'));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
resetUnifiedMemory();
|
|
// ADR-043: clear shared singleton between tests so each test gets a
|
|
// fresh router. Kernel.dispose() does this when a router was built,
|
|
// but tests that fail mid-setup might not reach dispose.
|
|
resetSharedLLMRouter();
|
|
// Defensive: also clear the env kill-switch tests may have set.
|
|
delete process.env.AQE_LLM_ROUTER_DISABLED;
|
|
if (fs.existsSync(tmpRoot)) {
|
|
fs.rmSync(tmpRoot, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
/** Build a ProviderManager pre-loaded with a mock provider — bypasses
|
|
* network on initialize(). Same trick as in llm-router-service.test.ts. */
|
|
function makeMockProviderManager(): { pm: ProviderManager; stats: ReturnType<typeof createMockLLMProvider>['stats'] } {
|
|
const { provider, stats } = createMockLLMProvider({ type: 'claude' });
|
|
const pm = new ProviderManager({
|
|
primary: 'claude',
|
|
fallbacks: [],
|
|
providers: { claude: { model: 'mock-model' } as any },
|
|
});
|
|
(pm as any).providers.set('claude', provider);
|
|
(pm as any).initialized = true;
|
|
(pm as any).initializeMetrics('claude');
|
|
return { pm, stats };
|
|
}
|
|
|
|
describe('Kernel ↔ LLM router wiring (ADR-043)', () => {
|
|
it('does not build a router when llmRouter.enabled is false', async () => {
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: false },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
expect(kernel.llmRouter).toBeUndefined();
|
|
|
|
await kernel.dispose();
|
|
});
|
|
|
|
it('does not build a router in auto mode when no provider is available', async () => {
|
|
// Save and clear any provider keys from the host env so this test
|
|
// is hermetic (the dev shell may have keys exported from .env).
|
|
const saved = {
|
|
ANTHROPIC_API_KEY: process.env.ANTHROPIC_API_KEY,
|
|
OPENAI_API_KEY: process.env.OPENAI_API_KEY,
|
|
OPENROUTER_API_KEY: process.env.OPENROUTER_API_KEY,
|
|
GEMINI_API_KEY: process.env.GEMINI_API_KEY,
|
|
GOOGLE_AI_API_KEY: process.env.GOOGLE_AI_API_KEY,
|
|
GOOGLE_API_KEY: process.env.GOOGLE_API_KEY,
|
|
};
|
|
for (const k of Object.keys(saved)) delete process.env[k];
|
|
|
|
try {
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: 'auto' },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
expect(kernel.llmRouter).toBeUndefined();
|
|
|
|
await kernel.dispose();
|
|
} finally {
|
|
for (const [k, v] of Object.entries(saved)) {
|
|
if (v !== undefined) process.env[k] = v;
|
|
}
|
|
}
|
|
});
|
|
|
|
it('builds a router when an injected ProviderManager is supplied', async () => {
|
|
const { pm } = makeMockProviderManager();
|
|
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
expect(kernel.llmRouter).toBeDefined();
|
|
|
|
await kernel.dispose();
|
|
});
|
|
|
|
it('round-trips a chat() call through the injected mock provider', async () => {
|
|
const { pm, stats } = makeMockProviderManager();
|
|
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
const router = kernel.llmRouter as any;
|
|
const response = await router.chat({
|
|
messages: [{ role: 'user', content: 'hello from a domain plugin' }],
|
|
model: 'mock-model',
|
|
});
|
|
|
|
expect(response.content).toBe('mock response');
|
|
expect(stats.generateCalls).toBe(1);
|
|
|
|
await kernel.dispose();
|
|
});
|
|
|
|
describe('AQE_LLM_ROUTER_DISABLED env kill-switch', () => {
|
|
it('AQE_LLM_ROUTER_DISABLED=1 prevents the router from being built even with enabled=true', async () => {
|
|
process.env.AQE_LLM_ROUTER_DISABLED = '1';
|
|
const { pm } = makeMockProviderManager();
|
|
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
expect(kernel.llmRouter).toBeUndefined();
|
|
await kernel.dispose();
|
|
});
|
|
|
|
it.each(['true', 'yes', 'on'])(
|
|
'AQE_LLM_ROUTER_DISABLED=%s also disables the router',
|
|
async (value) => {
|
|
process.env.AQE_LLM_ROUTER_DISABLED = value;
|
|
const { pm } = makeMockProviderManager();
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
expect(kernel.llmRouter).toBeUndefined();
|
|
await kernel.dispose();
|
|
}
|
|
);
|
|
|
|
it.each(['', '0', 'false', 'no', 'off'])(
|
|
'AQE_LLM_ROUTER_DISABLED=%s does NOT disable the router (router still builds)',
|
|
async (value) => {
|
|
process.env.AQE_LLM_ROUTER_DISABLED = value;
|
|
const { pm } = makeMockProviderManager();
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
expect(kernel.llmRouter).toBeDefined();
|
|
await kernel.dispose();
|
|
}
|
|
);
|
|
|
|
it('kill-switch also short-circuits the MCP getSharedLLMRouter() path', async () => {
|
|
process.env.AQE_LLM_ROUTER_DISABLED = '1';
|
|
const shared = await getSharedLLMRouter();
|
|
expect(shared).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe('Shared singleton unification (Fix #6)', () => {
|
|
it('kernel registers its router as the shared singleton so MCP sees the same instance', async () => {
|
|
const { pm } = makeMockProviderManager();
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm },
|
|
});
|
|
await kernel.initialize();
|
|
|
|
const shared = await getSharedLLMRouter();
|
|
expect(shared).toBe(kernel.llmRouter);
|
|
|
|
await kernel.dispose();
|
|
});
|
|
|
|
it('kernel.dispose() clears the shared singleton so the next kernel does not inherit it', async () => {
|
|
const { pm: pm1 } = makeMockProviderManager();
|
|
const kernel1 = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm1 },
|
|
});
|
|
await kernel1.initialize();
|
|
const router1 = kernel1.llmRouter;
|
|
await kernel1.dispose();
|
|
|
|
// After dispose, the shared singleton is cleared; building a fresh
|
|
// kernel with a fresh provider produces a DIFFERENT router instance.
|
|
const { pm: pm2 } = makeMockProviderManager();
|
|
const kernel2 = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true, providerManager: pm2 },
|
|
});
|
|
await kernel2.initialize();
|
|
expect(kernel2.llmRouter).not.toBe(router1);
|
|
await kernel2.dispose();
|
|
});
|
|
});
|
|
|
|
describe('Init failure observability (Fix #7)', () => {
|
|
it('publishes kernel.llm-router.init-no-provider event when enabled=true but no provider available', async () => {
|
|
// Hermetic: clear all provider keys
|
|
const saved: Record<string, string | undefined> = {
|
|
ANTHROPIC_API_KEY: process.env.ANTHROPIC_API_KEY,
|
|
OPENAI_API_KEY: process.env.OPENAI_API_KEY,
|
|
OPENROUTER_API_KEY: process.env.OPENROUTER_API_KEY,
|
|
GEMINI_API_KEY: process.env.GEMINI_API_KEY,
|
|
GOOGLE_AI_API_KEY: process.env.GOOGLE_AI_API_KEY,
|
|
GOOGLE_API_KEY: process.env.GOOGLE_API_KEY,
|
|
};
|
|
for (const k of Object.keys(saved)) delete process.env[k];
|
|
|
|
try {
|
|
const kernel = new QEKernelImpl({
|
|
memoryBackend: 'memory',
|
|
enabledDomains: [],
|
|
enableExperienceBridge: false,
|
|
enableDreamScheduler: false,
|
|
dataDir: tmpRoot,
|
|
llmRouter: { enabled: true },
|
|
});
|
|
|
|
const captured: Array<{ type: string; payload: unknown }> = [];
|
|
kernel.eventBus.subscribe('kernel.llm-router.init-no-provider', async (e: any) => {
|
|
captured.push({ type: e.type, payload: e.payload });
|
|
});
|
|
|
|
await kernel.initialize();
|
|
// Wait a tick for the event to propagate.
|
|
await new Promise((r) => setImmediate(r));
|
|
|
|
expect(captured.length).toBe(1);
|
|
expect(captured[0].type).toBe('kernel.llm-router.init-no-provider');
|
|
expect((captured[0].payload as any).reason).toContain('no provider available');
|
|
|
|
await kernel.dispose();
|
|
} finally {
|
|
for (const [k, v] of Object.entries(saved)) {
|
|
if (v !== undefined) process.env[k] = v;
|
|
}
|
|
}
|
|
});
|
|
});
|
|
});
|