From 2b37c4f38c826c13af0bc5a30c2e690fe90c1188 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Sun, 23 Aug 2026 05:13:16 +1000 Subject: [PATCH 1/8] fix(search): wire search_engines parameter through to engine dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search_engines parameter was declared in the MCP schema and CLI help but never consumed — passing it had no effect on which engines ran. - Add engineFilter to OrchestratorInput - Filter engine entries by name (case-insensitive) in runV1Search - Pass SearchInput.search_engines as engineFilter in core-provider - Unknown filter names fall back to full roster (graceful degradation) - 5 new tests: filter, case-insensitive, empty, undefined, no-match fallback Closes #303 --- src/search/core/core-provider.ts | 2 + src/search/core/orchestrator.ts | 17 +++- tests/unit/search/v1/orchestrator.test.ts | 97 +++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/search/core/core-provider.ts b/src/search/core/core-provider.ts index a966047e2..fc60eaf51 100644 --- a/src/search/core/core-provider.ts +++ b/src/search/core/core-provider.ts @@ -395,6 +395,7 @@ export class CoreSearchProvider implements SearchProvider { country: input.country, timeRange: input.time_range, exactMatch: input.exact_match, + engineFilter: input.search_engines, }), ), ); @@ -452,6 +453,7 @@ export class CoreSearchProvider implements SearchProvider { country: input.country, timeRange: input.time_range, exactMatch: input.exact_match, + engineFilter: input.search_engines, }); // RRF-merge the retry results on top of the initial dispatch so // we keep ranking signal from both passes. diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index b31feb15b..7d408319b 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -211,6 +211,10 @@ export interface OrchestratorInput { * result whose title+snippet does not contain the unquoted query as a * case-insensitive substring is dropped post-rerank. */ exactMatch?: boolean; + /** Caller-supplied engine allowlist. When non-empty, only engines whose + * name matches an entry (case-insensitive) are dispatched. Wired from + * SearchInput.search_engines via the MCP schema and CLI --search-engines. */ + engineFilter?: string[]; } export interface OrchestratorOutput { @@ -365,9 +369,20 @@ export async function runV1Search( // Probe-only engines are held back from the primary wave: they are a // per-call latency/failure tax on the happy path but still an independent // signal the degraded-recovery wave can pull in when the pool collapses. - const entries = allEntries.filter((e) => e.probeOnly !== true); + let entries = allEntries.filter((e) => e.probeOnly !== true); const probeEntries = allEntries.filter((e) => e.probeOnly === true); + // Apply caller-supplied engine allowlist (SearchInput.search_engines). + // Case-insensitive match against engine name. Unknown names are silently + // ignored — if no entries match, fall back to the full roster. + if (input.engineFilter && input.engineFilter.length > 0) { + const allowlist = input.engineFilter.map((n) => n.toLowerCase()); + const filtered = entries.filter((e) => allowlist.includes(e.engine.name.toLowerCase())); + if (filtered.length > 0) { + entries = filtered; + } + } + const options: SearchEngineOptions = { maxResults: input.maxResults ?? DEFAULT_MAX_RESULTS, timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, diff --git a/tests/unit/search/v1/orchestrator.test.ts b/tests/unit/search/v1/orchestrator.test.ts index 0596e6d7a..df2f6501a 100644 --- a/tests/unit/search/v1/orchestrator.test.ts +++ b/tests/unit/search/v1/orchestrator.test.ts @@ -1352,3 +1352,100 @@ describe('runV1Search — recency boost', () => { ]); }); }); + +describe('runV1Search — engineFilter (search_engines parameter)', () => { + it('filters engines by name when engineFilter is provided', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + const out = await runV1Search({ + query: 'test query', + engineFilter: ['duckduckgo'], + }); + expect(ddgSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(out.enginesUsed).toEqual(['duckduckgo']); + }); + + it('matches engine names case-insensitively', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + const out = await runV1Search({ + query: 'test query', + engineFilter: ['DuckDuckGo'], + }); + expect(ddgSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(out.enginesUsed).toEqual(['duckduckgo']); + }); + + it('falls back to full roster when filter matches nothing', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + await runV1Search({ + query: 'test query', + engineFilter: ['nonexistent-engine'], + }); + // Both engines dispatched when filter matches nothing + expect(bingSpy).toHaveBeenCalledOnce(); + expect(ddgSpy).toHaveBeenCalledOnce(); + }); + + it('does not filter when engineFilter is empty', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + await runV1Search({ + query: 'test query', + engineFilter: [], + }); + expect(bingSpy).toHaveBeenCalledOnce(); + expect(ddgSpy).toHaveBeenCalledOnce(); + }); + + it('does not filter when engineFilter is undefined', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + await runV1Search({ query: 'test query' }); + expect(bingSpy).toHaveBeenCalledOnce(); + expect(ddgSpy).toHaveBeenCalledOnce(); + }); +}); From 9d4476f21dbdce115cc77512af228a05d774340e Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Mon, 24 Aug 2026 02:11:00 +1000 Subject: [PATCH 2/8] fix(search): apply engineFilter to recovery wave, backfill, and cache key coderabbitai findings from PR#414: - Include search_engines in buildSearchCacheKey fingerprint so cached unfiltered results cannot satisfy filtered requests - Apply engineFilter allowlist to probeEntries (recovery wave) and getGeneralEngines (starvation backfill) so a degraded or thin search does not dispatch unselected engines - Extract applyEngineAllowlist helper to avoid duplicating the case-insensitive allowlist logic across three call sites --- src/cache/store.ts | 4 ++++ src/search/core/core-provider.ts | 1 + src/search/core/orchestrator.ts | 22 +++++++++++++++------- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/cache/store.ts b/src/cache/store.ts index de3706ff6..d8ca29c6b 100644 --- a/src/cache/store.ts +++ b/src/cache/store.ts @@ -353,6 +353,7 @@ export interface SearchCacheFilters { exact_match?: boolean | null; search_depth?: string | null; reranker?: string | null; + search_engines?: string[] | null; } function normaliseDomainList(list?: string[] | null): string[] | null { @@ -400,6 +401,9 @@ export function buildSearchCacheKey( exact_match: filters!.exact_match ?? null, search_depth: filters!.search_depth ?? null, reranker: filters!.reranker ?? null, + search_engines: filters!.search_engines && filters!.search_engines.length > 0 + ? [...filters!.search_engines].sort() + : null, }; return `${query}${JSON.stringify(fingerprint)}`; } diff --git a/src/search/core/core-provider.ts b/src/search/core/core-provider.ts index fc60eaf51..d8355ea61 100644 --- a/src/search/core/core-provider.ts +++ b/src/search/core/core-provider.ts @@ -203,6 +203,7 @@ export class CoreSearchProvider implements SearchProvider { exact_match: input.exact_match, search_depth: depth, reranker: getConfig().reranker, + search_engines: input.search_engines, }); let items: SearchResultItem[] = []; diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index 7d408319b..ac1c76a58 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -307,6 +307,12 @@ interface RunV1SearchOptions { _isFallback?: boolean; } +function applyEngineAllowlist(entries: EngineEntry[], allowlist: string[]): EngineEntry[] { + const lowered = allowlist.map((n) => n.toLowerCase()); + const filtered = entries.filter((e) => lowered.includes(e.engine.name.toLowerCase())); + return filtered.length > 0 ? filtered : entries; +} + export async function runV1Search( input: OrchestratorInput, opts: RunV1SearchOptions = {}, @@ -376,11 +382,7 @@ export async function runV1Search( // Case-insensitive match against engine name. Unknown names are silently // ignored — if no entries match, fall back to the full roster. if (input.engineFilter && input.engineFilter.length > 0) { - const allowlist = input.engineFilter.map((n) => n.toLowerCase()); - const filtered = entries.filter((e) => allowlist.includes(e.engine.name.toLowerCase())); - if (filtered.length > 0) { - entries = filtered; - } + entries = applyEngineAllowlist(entries, input.engineFilter); } const options: SearchEngineOptions = { @@ -638,10 +640,13 @@ export async function runV1Search( const skippedPrimary = outcomes .filter((o) => o.skipped) .map((o) => o.engine); - const recoveryEntries = [ + let recoveryEntries = [ ...probeEntries, ...entries.filter((e) => skippedPrimary.includes(e.engine.name)), ]; + if (input.engineFilter && input.engineFilter.length > 0) { + recoveryEntries = applyEngineAllowlist(recoveryEntries, input.engineFilter); + } if ( outcomes.length > 0 && primaryHealthy < poolHealthFloor(outcomes.length) && @@ -699,7 +704,10 @@ export async function runV1Search( vertical !== 'images' && !opts._isFallback ) { - const generalEntries = getGeneralEngines(); + let generalEntries = getGeneralEngines(); + if (input.engineFilter && input.engineFilter.length > 0) { + generalEntries = applyEngineAllowlist(generalEntries, input.engineFilter); + } if (generalEntries.length > 0) { log.info('vertical starved below floor, backfilling from general', { from: vertical, From ab9e3b546365009de94cd099cf29652cdf272ce1 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:18:13 +1000 Subject: [PATCH 3/8] fix(search): address coderabbitai findings on PR #414 1. hasAnyFilter now includes search_engines - ensures filtered cache requests produce distinct cache keys from unfiltered ones. 2. applyEngineAllowlist returns empty array when no matches - the fallback to full roster is now handled explicitly at the primary wave call site, while recovery/backfill waves correctly respect the filter. --- src/cache/store.ts | 3 ++- src/search/core/orchestrator.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/cache/store.ts b/src/cache/store.ts index d8ca29c6b..93aa180d9 100644 --- a/src/cache/store.ts +++ b/src/cache/store.ts @@ -376,7 +376,8 @@ function hasAnyFilter(filters?: SearchCacheFilters): boolean { filters.time_range != null || filters.exact_match != null || filters.search_depth != null || - filters.reranker != null + filters.reranker != null || + (filters.search_engines?.length ?? 0) > 0 ); } diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index ac1c76a58..38641f6f9 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -310,7 +310,7 @@ interface RunV1SearchOptions { function applyEngineAllowlist(entries: EngineEntry[], allowlist: string[]): EngineEntry[] { const lowered = allowlist.map((n) => n.toLowerCase()); const filtered = entries.filter((e) => lowered.includes(e.engine.name.toLowerCase())); - return filtered.length > 0 ? filtered : entries; + return filtered; } export async function runV1Search( @@ -379,10 +379,15 @@ export async function runV1Search( const probeEntries = allEntries.filter((e) => e.probeOnly === true); // Apply caller-supplied engine allowlist (SearchInput.search_engines). - // Case-insensitive match against engine name. Unknown names are silently - // ignored — if no entries match, fall back to the full roster. + // Case-insensitive match against engine name. For the primary wave, if no + // entries match the allowlist, fall back to the full roster (the caller + // likely made a typo or passed an unknown engine name). For recovery and + // backfill waves, an empty result means those waves run nothing — which is + // correct: if the caller explicitly filtered out all probe/backfill engines, + // we don't secretly re-introduce them. if (input.engineFilter && input.engineFilter.length > 0) { - entries = applyEngineAllowlist(entries, input.engineFilter); + const allowlisted = applyEngineAllowlist(entries, input.engineFilter); + entries = allowlisted.length > 0 ? allowlisted : entries; } const options: SearchEngineOptions = { From fcf468c23d436692a4444d591eb1dff787272717 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:25:54 +1000 Subject: [PATCH 4/8] fix(search): address coderabbitai findings on PR #521 1. Probe-only filter match (Major): selecting a configured probe-only engine (e.g. Mojeek with searchMojeekProbeOnly) no longer triggers the full-primary-roster fallback; the probe-only engine is dispatched instead. Full-roster fallback now only fires when the filter matches no configured engine at all. 2. Cache key normalisation (Minor): search_engines is now normalised (trim, lowercase, dedupe, sort) before fingerprinting, shared with hasAnyFilter, so casing/order/duplicate lists dispatching the same engine set produce identical cache keys. --- src/cache/store.ts | 17 +++-- src/search/core/orchestrator.ts | 15 ++++- tests/unit/cache/store-search-key.test.ts | 20 ++++++ tests/unit/search/v1/orchestrator.test.ts | 79 ++++++++++++++++++++++- 4 files changed, 125 insertions(+), 6 deletions(-) diff --git a/src/cache/store.ts b/src/cache/store.ts index 93aa180d9..0c9a35c5b 100644 --- a/src/cache/store.ts +++ b/src/cache/store.ts @@ -363,6 +363,17 @@ function normaliseDomainList(list?: string[] | null): string[] | null { return [...new Set(lower)].sort(); } +/** Normalise a caller-supplied engine list: trim, lowercase, dedupe, sort. + * Mirrors the orchestrator's case-insensitive engine matching, so + * `['DuckDuckGo']`, `['duckduckgo']`, reordered, or duplicate-name lists — + * all of which dispatch the same engine set — produce identical cache keys. */ +export function normaliseEngineList(list?: string[] | null): string[] | null { + if (!list || list.length === 0) return null; + const lower = list.map((e) => e.toLowerCase().trim()).filter((e) => e.length > 0); + if (lower.length === 0) return null; + return [...new Set(lower)].sort(); +} + function hasAnyFilter(filters?: SearchCacheFilters): boolean { if (!filters) return false; return ( @@ -377,7 +388,7 @@ function hasAnyFilter(filters?: SearchCacheFilters): boolean { filters.exact_match != null || filters.search_depth != null || filters.reranker != null || - (filters.search_engines?.length ?? 0) > 0 + normaliseEngineList(filters.search_engines) != null ); } @@ -402,9 +413,7 @@ export function buildSearchCacheKey( exact_match: filters!.exact_match ?? null, search_depth: filters!.search_depth ?? null, reranker: filters!.reranker ?? null, - search_engines: filters!.search_engines && filters!.search_engines.length > 0 - ? [...filters!.search_engines].sort() - : null, + search_engines: normaliseEngineList(filters!.search_engines), }; return `${query}${JSON.stringify(fingerprint)}`; } diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index 38641f6f9..0f189cbdc 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -387,7 +387,20 @@ export async function runV1Search( // we don't secretly re-introduce them. if (input.engineFilter && input.engineFilter.length > 0) { const allowlisted = applyEngineAllowlist(entries, input.engineFilter); - entries = allowlisted.length > 0 ? allowlisted : entries; + if (allowlisted.length > 0) { + entries = allowlisted; + } else { + // No NON-probe entry matched. Distinguish a caller typo from an explicit + // probe-only selection: if the filter names a configured probe-only + // engine (e.g. Mojeek with searchMojeekProbeOnly enabled), dispatch those + // probe-only engines rather than silently restoring the full primary + // roster and dispatching unselected engines. Fall back to the full roster + // ONLY when the filter matches no configured engine at all. + const probeAllowlisted = applyEngineAllowlist(probeEntries, input.engineFilter); + if (probeAllowlisted.length > 0) { + entries = probeAllowlisted; + } + } } const options: SearchEngineOptions = { diff --git a/tests/unit/cache/store-search-key.test.ts b/tests/unit/cache/store-search-key.test.ts index b518b54d8..f5442bf06 100644 --- a/tests/unit/cache/store-search-key.test.ts +++ b/tests/unit/cache/store-search-key.test.ts @@ -62,6 +62,26 @@ describe('buildSearchCacheKey', () => { expect(balanced).not.toBe(noRerank); expect(balanced).not.toBe('q'); // depth always present -> always fingerprinted }); + + it('normalises search_engines: casing, order, and duplicates share one key', () => { + const a = buildSearchCacheKey('q', { search_engines: ['DuckDuckGo'] }); + const b = buildSearchCacheKey('q', { search_engines: ['duckduckgo'] }); + const c = buildSearchCacheKey('q', { search_engines: ['brave', 'duckduckgo'] }); + const d = buildSearchCacheKey('q', { search_engines: ['DuckDuckGo', 'brave'] }); + const e = buildSearchCacheKey('q', { search_engines: ['duckduckgo', 'duckduckgo'] }); + expect(a).toBe(b); // case-insensitive + expect(c).toBe(d); // order- and case-insensitive + expect(a).toBe(e); // duplicates collapse + expect(a).not.toBe(c); // different engine sets stay distinct + }); + + it('treats whitespace-only or empty search_engines as no filter', () => { + const bare = buildSearchCacheKey('q'); + const empty = buildSearchCacheKey('q', { search_engines: [] }); + const blanks = buildSearchCacheKey('q', { search_engines: [' ', ''] }); + expect(bare).toBe(empty); + expect(bare).toBe(blanks); + }); }); describe('cache miss on filter mismatch', () => { diff --git a/tests/unit/search/v1/orchestrator.test.ts b/tests/unit/search/v1/orchestrator.test.ts index df2f6501a..1f26249f0 100644 --- a/tests/unit/search/v1/orchestrator.test.ts +++ b/tests/unit/search/v1/orchestrator.test.ts @@ -103,7 +103,7 @@ function makeMockEngine(cfg: MockEngineConfig): { } function makeEntry( - cfg: MockEngineConfig & { weight?: number; supportsDateFilter?: boolean }, + cfg: MockEngineConfig & { weight?: number; supportsDateFilter?: boolean; probeOnly?: boolean }, ): { entry: EngineEntry; spy: ReturnType } { const { engine, spy } = makeMockEngine(cfg); return { @@ -111,6 +111,7 @@ function makeEntry( engine, weight: cfg.weight, supportsDateFilter: cfg.supportsDateFilter, + probeOnly: cfg.probeOnly, }, spy, }; @@ -1448,4 +1449,80 @@ describe('runV1Search — engineFilter (search_engines parameter)', () => { expect(bingSpy).toHaveBeenCalledOnce(); expect(ddgSpy).toHaveBeenCalledOnce(); }); + + it('dispatches a configured probe-only engine when it is the only filter match (no full-roster fallback)', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + // Mojeek configured probe-only (searchMojeekProbeOnly enabled): held back + // from the primary wave, but an explicit selection must dispatch it — not + // be treated as "unknown engine" and restore the whole primary roster. + const { entry: mojeek, spy: mojeekSpy } = makeEntry({ + name: 'mojeek', + probeOnly: true, + results: [makeResult('mojeek', 'https://mojeek.test/z')], + }); + verticalState.general = [bing, ddg, mojeek]; + + const out = await runV1Search({ + query: 'test query', + engineFilter: ['mojeek'], + }); + expect(mojeekSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(ddgSpy).not.toHaveBeenCalled(); + expect(out.enginesUsed).toEqual(['mojeek']); + }); + + it('keeps the full-roster fallback when the filter matches no configured engine at all', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: mojeek, spy: mojeekSpy } = makeEntry({ + name: 'mojeek', + probeOnly: true, + results: [makeResult('mojeek', 'https://mojeek.test/z')], + }); + verticalState.general = [bing, mojeek]; + + await runV1Search({ + query: 'test query', + engineFilter: ['nonexistent-engine'], + }); + // Unknown name: full primary roster restored (probe-only still held back). + expect(bingSpy).toHaveBeenCalledOnce(); + expect(mojeekSpy).not.toHaveBeenCalled(); + }); + + it('holds probe-only engines back when the filter also matches a primary engine', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + const { entry: mojeek, spy: mojeekSpy } = makeEntry({ + name: 'mojeek', + probeOnly: true, + results: [makeResult('mojeek', 'https://mojeek.test/z')], + }); + verticalState.general = [bing, ddg, mojeek]; + + const out = await runV1Search({ + query: 'test query', + engineFilter: ['duckduckgo', 'mojeek'], + }); + expect(ddgSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + // Probe-only selection is honoured via its intended wave, not the primary. + expect(out.enginesUsed).not.toContain('bing'); + }); }); From a3cee9cddfa1cb1e8fa9af19acff32d2ada25ac8 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:30:18 +1000 Subject: [PATCH 5/8] fix(search): normalise engineFilter before every gate; no probe re-dispatch in recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two CodeRabbit findings on PR #521 (2026-09-01): 1. Major (orchestrator.ts:391): normalise engineFilter ONCE up front (trim, lowercase, dedupe, sort) via a shared normaliseEngineList (src/util/engine-list.ts) used by BOTH the cache-key fingerprint and every allowlist gate (primary, probe fallback, recovery, starvation backfill). A whitespace-padded valid value like [' duckduckgo '] now dispatches only that engine — before, it missed the raw allowlist and dispatched the full roster, which the cache layer then filed under the trimmed single-engine key. An all-blank list normalises to null, i.e. no filter. 2. Minor (orchestrator.ts:401): engines that received an attempted (non-skipped) primary dispatch are excluded from the probe recovery roster — a selected probe-only engine returning zero results no longer triggers a second external request + recovery wait against the same engine. Skipped (breaker-open) engines stay eligible; a name-dedupe also covers the skipped-probe appearing in both lists. Tests: 3 new orchestrator regressions (padded valid name, all-blank filter, zero-result probe-only). tsc --noEmit clean; orchestrator suite 68/68 green. --- src/cache/store.ts | 14 ++--- src/search/core/orchestrator.ts | 55 ++++++++++++----- src/util/engine-list.ts | 16 +++++ tests/unit/search/v1/orchestrator.test.ts | 75 +++++++++++++++++++++++ 4 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 src/util/engine-list.ts diff --git a/src/cache/store.ts b/src/cache/store.ts index 0c9a35c5b..77b282cc0 100644 --- a/src/cache/store.ts +++ b/src/cache/store.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import { getDatabase } from './db.js'; import { getConfig } from '../config.js'; import { createLogger } from '../logger.js'; +import { normaliseEngineList } from '../util/engine-list.js'; import type { RawFetchResult, ExtractionResult, CachedContent, SearchResultItem, CacheStats, ContentCompleteness } from '../types.js'; const log = createLogger('cache'); @@ -363,16 +364,9 @@ function normaliseDomainList(list?: string[] | null): string[] | null { return [...new Set(lower)].sort(); } -/** Normalise a caller-supplied engine list: trim, lowercase, dedupe, sort. - * Mirrors the orchestrator's case-insensitive engine matching, so - * `['DuckDuckGo']`, `['duckduckgo']`, reordered, or duplicate-name lists — - * all of which dispatch the same engine set — produce identical cache keys. */ -export function normaliseEngineList(list?: string[] | null): string[] | null { - if (!list || list.length === 0) return null; - const lower = list.map((e) => e.toLowerCase().trim()).filter((e) => e.length > 0); - if (lower.length === 0) return null; - return [...new Set(lower)].sort(); -} +// Shared with the orchestrator's allowlist gates (src/util/engine-list.ts) +// so the cache-key fingerprint and dispatch matching can never drift apart. +export { normaliseEngineList } from '../util/engine-list.js'; function hasAnyFilter(filters?: SearchCacheFilters): boolean { if (!filters) return false; diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index 0f189cbdc..c52f4dcfb 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -6,6 +6,7 @@ import type { SearchEngineOptions, } from '../../types.js'; import { createLogger } from '../../logger.js'; +import { normaliseEngineList } from '../../util/engine-list.js'; import { classifyIntentDetailed, extractErrorTokens, @@ -379,14 +380,23 @@ export async function runV1Search( const probeEntries = allEntries.filter((e) => e.probeOnly === true); // Apply caller-supplied engine allowlist (SearchInput.search_engines). - // Case-insensitive match against engine name. For the primary wave, if no - // entries match the allowlist, fall back to the full roster (the caller - // likely made a typo or passed an unknown engine name). For recovery and - // backfill waves, an empty result means those waves run nothing — which is - // correct: if the caller explicitly filtered out all probe/backfill engines, - // we don't secretly re-introduce them. - if (input.engineFilter && input.engineFilter.length > 0) { - const allowlisted = applyEngineAllowlist(entries, input.engineFilter); + // Normalise ONCE up front (trim, lowercase, dedupe, sort) and use that + // normalised list at every gate below — primary, probe fallback, recovery, + // and starvation backfill. Normalising at the gates (rather than raw-matching) + // keeps the orchestrator consistent with the cache-key fingerprint in + // cache/store.ts, which trims the same value: a whitespace-padded valid name + // like [' duckduckgo '] must dispatch ONLY that engine, not miss the + // allowlist and dispatch the full roster (which would then be cached under + // the trimmed single-engine key). An all-blank list normalises to null, + // i.e. treated as "no filter". Case-insensitive match against engine name. + // For the primary wave, if no entries match the allowlist, fall back to the + // full roster (the caller likely made a typo or passed an unknown engine + // name). For recovery and backfill waves, an empty result means those waves + // run nothing — which is correct: if the caller explicitly filtered out all + // probe/backfill engines, we don't secretly re-introduce them. + const engineAllowlist = normaliseEngineList(input.engineFilter); + if (engineAllowlist && engineAllowlist.length > 0) { + const allowlisted = applyEngineAllowlist(entries, engineAllowlist); if (allowlisted.length > 0) { entries = allowlisted; } else { @@ -396,7 +406,7 @@ export async function runV1Search( // probe-only engines rather than silently restoring the full primary // roster and dispatching unselected engines. Fall back to the full roster // ONLY when the filter matches no configured engine at all. - const probeAllowlisted = applyEngineAllowlist(probeEntries, input.engineFilter); + const probeAllowlisted = applyEngineAllowlist(probeEntries, engineAllowlist); if (probeAllowlisted.length > 0) { entries = probeAllowlisted; } @@ -658,12 +668,29 @@ export async function runV1Search( const skippedPrimary = outcomes .filter((o) => o.skipped) .map((o) => o.engine); + // Engines that received an ATTEMPTED (non-skipped) primary dispatch. A + // probe-only engine selected via engineFilter and dispatched as the primary + // wave that returns zero results must NOT re-enter the recovery roster via + // probeEntries — that would fire a second external request and recovery wait + // against the same engine without probing a new one. Engines SKIPPED in the + // primary wave (breaker open) stay eligible: recovery is their retry path. + const attemptedPrimary = new Set( + outcomes.filter((o) => !o.skipped).map((o) => o.engine), + ); let recoveryEntries = [ - ...probeEntries, + ...probeEntries.filter((e) => !attemptedPrimary.has(e.engine.name)), ...entries.filter((e) => skippedPrimary.includes(e.engine.name)), ]; - if (input.engineFilter && input.engineFilter.length > 0) { - recoveryEntries = applyEngineAllowlist(recoveryEntries, input.engineFilter); + // Dedupe by engine name: a probe-only engine selected via engineFilter and + // skipped (breaker open) appears in both lists above. + const seenRecovery = new Set(); + recoveryEntries = recoveryEntries.filter((e) => { + if (seenRecovery.has(e.engine.name)) return false; + seenRecovery.add(e.engine.name); + return true; + }); + if (engineAllowlist && engineAllowlist.length > 0) { + recoveryEntries = applyEngineAllowlist(recoveryEntries, engineAllowlist); } if ( outcomes.length > 0 && @@ -723,8 +750,8 @@ export async function runV1Search( !opts._isFallback ) { let generalEntries = getGeneralEngines(); - if (input.engineFilter && input.engineFilter.length > 0) { - generalEntries = applyEngineAllowlist(generalEntries, input.engineFilter); + if (engineAllowlist && engineAllowlist.length > 0) { + generalEntries = applyEngineAllowlist(generalEntries, engineAllowlist); } if (generalEntries.length > 0) { log.info('vertical starved below floor, backfilling from general', { diff --git a/src/util/engine-list.ts b/src/util/engine-list.ts new file mode 100644 index 000000000..163ceb81b --- /dev/null +++ b/src/util/engine-list.ts @@ -0,0 +1,16 @@ +/** Normalise a caller-supplied engine list: trim, lowercase, dedupe, sort. + * Mirrors the orchestrator's case-insensitive engine matching, so + * `['DuckDuckGo']`, `['duckduckgo']`, whitespace-padded, reordered, or + * duplicate-name lists — all of which dispatch the same engine set — hit + * the same allowlist gates AND produce identical cache keys. Shared by the + * cache-key fingerprint (cache/store.ts) and every engineFilter gate in the + * orchestrator so the two can never drift apart (a padded value that misses + * the dispatch allowlist but trims into the cache key would file a + * full-roster response under a single-engine key). An all-blank list + * normalises to null, i.e. "no filter". */ +export function normaliseEngineList(list?: string[] | null): string[] | null { + if (!list || list.length === 0) return null; + const lower = list.map((e) => e.toLowerCase().trim()).filter((e) => e.length > 0); + if (lower.length === 0) return null; + return [...new Set(lower)].sort(); +} diff --git a/tests/unit/search/v1/orchestrator.test.ts b/tests/unit/search/v1/orchestrator.test.ts index 1f26249f0..3aeee1afb 100644 --- a/tests/unit/search/v1/orchestrator.test.ts +++ b/tests/unit/search/v1/orchestrator.test.ts @@ -1525,4 +1525,79 @@ describe('runV1Search — engineFilter (search_engines parameter)', () => { // Probe-only selection is honoured via its intended wave, not the primary. expect(out.enginesUsed).not.toContain('bing'); }); + + it('treats a whitespace-padded valid engine name as its trimmed value', async () => { + // The cache-key fingerprint trims filter values, so the dispatch gates + // must too — otherwise [' duckduckgo '] misses the allowlist, dispatches + // the FULL roster, and that response gets cached under the trimmed + // single-engine key. + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + const out = await runV1Search({ + query: 'test query', + engineFilter: [' duckduckgo '], + }); + expect(ddgSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(out.enginesUsed).toEqual(['duckduckgo']); + }); + + it('treats an all-blank engineFilter as no filter', async () => { + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + verticalState.general = [bing, ddg]; + + await runV1Search({ + query: 'test query', + engineFilter: [' ', ''], + }); + // Blank-only list normalises to null: both engines dispatched. + expect(bingSpy).toHaveBeenCalledOnce(); + expect(ddgSpy).toHaveBeenCalledOnce(); + }); + + it('does not re-dispatch a zero-result probe-only engine selected as the primary wave', async () => { + // Selecting a probe-only engine dispatches it as the primary wave. If it + // returns zero results, the pool is below the health floor and the + // recovery wave fires — but it must NOT re-dispatch the same engine + // (second external request + recovery wait without probing a new one). + const { entry: bing, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + const { entry: ddg, spy: ddgSpy } = makeEntry({ + name: 'duckduckgo', + results: [makeResult('duckduckgo', 'https://ddg.test/y')], + }); + const { entry: mojeek, spy: mojeekSpy } = makeEntry({ + name: 'mojeek', + probeOnly: true, + results: [], + }); + verticalState.general = [bing, ddg, mojeek]; + + await runV1Search({ + query: 'test query', + engineFilter: ['mojeek'], + }); + // Exactly one dispatch: the primary attempt. The recovery wave (which + // triggers on 0 healthy < floor 1) excludes the already-attempted probe. + expect(mojeekSpy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(ddgSpy).not.toHaveBeenCalled(); + }); }); From acaf8b9716d7513ab33501e0a83e73e9de7c9c38 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:13:41 +1000 Subject: [PATCH 6/8] fix(search): keep a recognized engineFilter restricted across the general fallback When a specialized engine matched the engineFilter but returned no results, the degraded fallback to category 'general' re-entered the allowlist gate with zero matches and silently restored the full general roster, running engines the caller never selected. The fallback now preserves the recognized-filter state: if the filter matched configured engines in this vertical but none exist in the general roster, the fallback is skipped and the degraded result is surfaced. An unrecognized filter (typo) still falls back to the full general roster as before. Adds two regression tests (70/70 green, tsc --noEmit clean). --- src/search/core/orchestrator.ts | 26 ++++++++++++- tests/unit/search/v1/orchestrator.test.ts | 46 +++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index c52f4dcfb..488cb719f 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -897,8 +897,30 @@ export async function runV1Search( // image vertical surfaces empty + engine_warnings rather than silently // morphing into a general search. if (degraded && vertical !== 'general' && vertical !== 'images' && !opts._isFallback) { - log.info('vertical degraded, falling back to general', { from: vertical }); - return runV1Search({ ...input, category: 'general' }, { _isFallback: true }); + // Keep a RECOGNIZED engineFilter restricted across the fallback. If the + // filter matched configured engines in this vertical but none of them + // exist in the general roster, recursing would re-enter the allowlist + // gate with zero matches and silently restore the FULL general roster, + // running engines the caller never selected. Skip the fallback in that + // case and surface the degraded result. An unrecognized filter (typo) + // still falls back to the full general roster as before, and a filter + // that matches general engines falls back normally (the gate in the + // recursive call restricts dispatch to the matched engines). + let skipFallback = false; + if (engineAllowlist && engineAllowlist.length > 0) { + const matchedHere = applyEngineAllowlist(allEntries, engineAllowlist).length > 0; + const matchedGeneral = + applyEngineAllowlist(getEntriesForVertical('general'), engineAllowlist).length > 0; + skipFallback = matchedHere && !matchedGeneral; + } + if (!skipFallback) { + log.info('vertical degraded, falling back to general', { from: vertical }); + return runV1Search({ ...input, category: 'general' }, { _isFallback: true }); + } + log.info( + 'vertical degraded; engineFilter matches no general engine — skipping fallback to keep the filter restricted', + { from: vertical }, + ); } return { diff --git a/tests/unit/search/v1/orchestrator.test.ts b/tests/unit/search/v1/orchestrator.test.ts index 3aeee1afb..1cffc5dbf 100644 --- a/tests/unit/search/v1/orchestrator.test.ts +++ b/tests/unit/search/v1/orchestrator.test.ts @@ -976,6 +976,52 @@ describe('runV1Search — degraded fallback to general', () => { const out = await runV1Search({ query: 'fix python regex' }); expect(out.vertical).toBe('general'); }); + + it('keeps a recognized engineFilter restricted after category fallback (no full general roster)', async () => { + // github-code exists ONLY in the code roster. The filter selects it, it + // returns zero results, so the vertical degrades. The general fallback + // must NOT treat the recognized filter as unmatched and restore the full + // general roster — no unselected general engine may run. + const codeOnly = makeEntry({ name: 'github-code', results: [] }); + verticalState.code = [codeOnly.entry]; + + const { entry: bingEntry, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + verticalState.general = [bingEntry]; + + const out = await runV1Search({ + query: 'fix typescript error', + engineFilter: ['github-code'], + }); + expect(codeOnly.spy).toHaveBeenCalledOnce(); + expect(bingSpy).not.toHaveBeenCalled(); + expect(out.vertical).toBe('code'); + expect(out.degraded).toBe(true); + expect(out.results).toEqual([]); + }); + + it('still falls back to the full general roster when the filter matches no configured engine (typo)', async () => { + // Unrecognized filter = caller typo: the pre-existing full-roster + // fallback semantics must survive the recognized-filter guard. + const codeEmpty = makeEntry({ name: 'github-code', results: [] }); + verticalState.code = [codeEmpty.entry]; + + const { entry: bingEntry, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/y')], + }); + verticalState.general = [bingEntry]; + + const out = await runV1Search({ + query: 'fix typescript error', + engineFilter: ['nonexistent-engine'], + }); + expect(bingSpy).toHaveBeenCalledOnce(); + expect(out.vertical).toBe('general'); + expect(out.degraded).toBe(false); + }); }); describe('runV1Search — per-result starvation re-dispatch', () => { From dbe92192eb79fb853b12f8b8ba14a8875327a786 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Thu, 10 Sep 2026 02:10:59 +1000 Subject: [PATCH 7/8] fix(search): keep a globally-recognized engineFilter from restoring the full roster CodeRabbit re-review finding (2026-09-09T14:21Z) on orchestrator.ts: probeAllowlisted and the primary allowlist check are both local to the current vertical. A filter naming an engine configured only in ANOTHER vertical (e.g. a code-vertical engine on a general request) missed both checks, leaving entries as the FULL roster of the current vertical and dispatching engines the caller never selected. Fix: add isEngineFilterRecognisedAnywhere() over all six vertical rosters. Recognised-but-unavailable-here now dispatches nothing (entries = [] + warn log); the full-roster fallback survives only for filters matching no configured engine anywhere (caller typo). Also per the Minor test finding: assert the probe-only mojeekSpy was not called in the holds-probe-back test (previously only bing was excluded from enginesUsed). Tests: new regression (cross-vertical recognized filter on general dispatches nothing, degraded + empty); 71/71 orchestrator tests green; tsc --noEmit clean. --- src/search/core/orchestrator.ts | 29 +++++++++++++++++++++-- tests/unit/search/v1/orchestrator.test.ts | 29 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index 488cb719f..23ca58af1 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -314,6 +314,20 @@ function applyEngineAllowlist(entries: EngineEntry[], allowlist: string[]): Engi return filtered; } +// Every vertical in the registry, used to decide whether an engineFilter names +// a CONFIGURED engine somewhere in the system even when it is unavailable in +// the vertical being dispatched. A filter that is recognised anywhere must +// never fall back to the full roster of the current vertical — that would +// dispatch engines the caller did not select. Only a filter that matches no +// configured engine at all (caller typo) keeps the full-roster fallback. +const ALL_VERTICALS: Vertical[] = ['general', 'news', 'code', 'docs', 'papers', 'images']; + +function isEngineFilterRecognisedAnywhere(allowlist: string[]): boolean { + return ALL_VERTICALS.some( + (v) => applyEngineAllowlist(getEntriesForVertical(v), allowlist).length > 0, + ); +} + export async function runV1Search( input: OrchestratorInput, opts: RunV1SearchOptions = {}, @@ -404,11 +418,22 @@ export async function runV1Search( // probe-only selection: if the filter names a configured probe-only // engine (e.g. Mojeek with searchMojeekProbeOnly enabled), dispatch those // probe-only engines rather than silently restoring the full primary - // roster and dispatching unselected engines. Fall back to the full roster - // ONLY when the filter matches no configured engine at all. + // roster and dispatching unselected engines. Then check GLOBAL + // recognition: if the filter names an engine configured only in ANOTHER + // vertical (e.g. a code-vertical engine requested on a general search), + // dispatch nothing rather than the full roster — the caller selected + // engines that this vertical cannot provide, not "every engine". + // Fall back to the full roster ONLY when the filter matches no + // configured engine anywhere (caller typo or unknown engine name). const probeAllowlisted = applyEngineAllowlist(probeEntries, engineAllowlist); if (probeAllowlisted.length > 0) { entries = probeAllowlisted; + } else if (isEngineFilterRecognisedAnywhere(engineAllowlist)) { + log.warn( + 'engineFilter matches configured engines in other verticals but none here — dispatching nothing to keep the selection restricted', + { vertical, engineAllowlist }, + ); + entries = []; } } } diff --git a/tests/unit/search/v1/orchestrator.test.ts b/tests/unit/search/v1/orchestrator.test.ts index 1cffc5dbf..ce567bc0c 100644 --- a/tests/unit/search/v1/orchestrator.test.ts +++ b/tests/unit/search/v1/orchestrator.test.ts @@ -1022,6 +1022,34 @@ describe('runV1Search — degraded fallback to general', () => { expect(out.vertical).toBe('general'); expect(out.degraded).toBe(false); }); + + it('dispatches nothing when the filter is recognized only in ANOTHER vertical (no full-roster restore)', async () => { + // github-code exists ONLY in the code roster; the caller asks for a + // general search filtered to it. Both local checks miss, but the filter + // is recognized in the registry — restoring the full general roster + // would dispatch engines the caller never selected. The selection stays + // restricted: nothing runs, the result surfaces degraded + empty. + const codeOnly = makeEntry({ name: 'github-code', results: [] }); + verticalState.code = [codeOnly.entry]; + + const { entry: bingEntry, spy: bingSpy } = makeEntry({ + name: 'bing', + results: [makeResult('bing', 'https://bing.test/x')], + }); + verticalState.general = [bingEntry]; + + const out = await runV1Search({ + query: 'fix typescript error', + engineFilter: ['github-code'], + category: 'general', + }); + expect(bingSpy).not.toHaveBeenCalled(); + expect(codeOnly.spy).not.toHaveBeenCalled(); + expect(out.vertical).toBe('general'); + expect(out.degraded).toBe(true); + expect(out.results).toEqual([]); + expect(out.enginesUsed).toEqual([]); + }); }); describe('runV1Search — per-result starvation re-dispatch', () => { @@ -1568,6 +1596,7 @@ describe('runV1Search — engineFilter (search_engines parameter)', () => { }); expect(ddgSpy).toHaveBeenCalledOnce(); expect(bingSpy).not.toHaveBeenCalled(); + expect(mojeekSpy).not.toHaveBeenCalled(); // Probe-only selection is honoured via its intended wave, not the primary. expect(out.enginesUsed).not.toContain('bing'); }); From cec5eeb21f307726d72b98513f189ed00da4d8e4 Mon Sep 17 00:00:00 2001 From: fuleinist <1163738+fuleinist@users.noreply.github.com> Date: Thu, 10 Sep 2026 04:06:04 +1000 Subject: [PATCH 8/8] fix(search): make ALL_VERTICALS exhaustive via satisfies Record --- src/search/core/orchestrator.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/search/core/orchestrator.ts b/src/search/core/orchestrator.ts index 23ca58af1..c6414ad36 100644 --- a/src/search/core/orchestrator.ts +++ b/src/search/core/orchestrator.ts @@ -320,7 +320,14 @@ function applyEngineAllowlist(entries: EngineEntry[], allowlist: string[]): Engi // never fall back to the full roster of the current vertical — that would // dispatch engines the caller did not select. Only a filter that matches no // configured engine at all (caller typo) keeps the full-roster fallback. -const ALL_VERTICALS: Vertical[] = ['general', 'news', 'code', 'docs', 'papers', 'images']; +const ALL_VERTICALS = Object.keys({ + general: true, + news: true, + code: true, + docs: true, + papers: true, + images: true, +} satisfies Record) as Vertical[]; function isEngineFilterRecognisedAnywhere(allowlist: string[]): boolean { return ALL_VERTICALS.some(