fix(search): distinguish all-engines-failed from genuinely empty results

[shauneccles]

Fixes #262.

Problem

unresponsive_engines is declared on SearXNGWeb (src/types.ts:31) but never read anywhere in src/ — it is parsed off the SearXNG response and discarded.

So a search where every engine that reported back failed is returned to the MCP client as a successful tool call carrying the same string as a search that ran cleanly and matched nothing:

🔍 No results found for "<query>". Try different search terms or check if SearXNG search engines are working.

Verified against a live SearXNG instance with v2.1.0, pinning a single engine so the all-engines-failed case is deterministic:

engines=	results	unresponsive_engines
startpage	[]	[["startpage", "Suspended: CAPTCHA"]]
brave	[]	[["brave", "too many requests"]]
yahoo	[]	[["yahoo", "HTTP protocol error"]]
wikidata	[]	[] — the only genuinely empty one

Through searxng_web_search, engines=startpage and engines=wikidata return byte-identical success strings. An autonomous caller reads the CAPTCHA as an authoritative "nothing exists" and stops looking, rather than retrying or switching engines. The existing message's own hint — "check if SearXNG search engines are working" — is precisely the check the server can already perform from the response body and does not.

Change

createEngineFailureError in src/error-handler.ts, raised from performWebSearch when the raw result set is empty and no zero-row response came back clean.

The classification runs on the aggregate, not on one payload. Both all-empty multi-instance paths retain a single replica's data chosen by ordering — performFailoverSearch keeps emptyResults[0].data, the all-empty fan-out branch keeps successes[0].data — so reading data.unresponsive_engines alone would make the verdict depend on replica order. Instead:

• Both helpers now carry every zero-row response out as emptyResponses on MultiInstanceSearchResult. servedBy semantics and the with-results return paths are unchanged.
• One shared classifyEmptyResponses() decides the verdict, so the two helpers do not duplicate the rule. It returns null as soon as any zero-row response reports no failing engines — a clean empty replica is authoritative evidence that nothing matched, whichever replica produced it — and otherwise returns the union of failures across replicas, keyed by engine name in first-seen order so an engine failing on several replicas is named once with a deterministic reason.
• performWebSearch classifies once, via classifyEmptyResponses(emptyResponses ?? [data]). The single-instance path leaves emptyResponses undefined and classifies [data], so its behaviour is exactly as it was.

Two further details:

• The check reads the raw data.results, not the filtered/sliced set. If engines did return rows and min_score/num_results removed them, that is a filtering outcome and the existing no-results message stays correct.
• It sits above the response_format === "json" branch, so both response formats are covered. (compact JSON drops everything but results, so unresponsive_engines did not survive there either.)

The error names each failed engine and its reason so the caller can act. unresponsive_engines is external data that is only cast to SearXNGWeb and never runtime-validated, so a single exported normalizeEngineFailure() coerces each entry to [engine, reason]: a non-array entry (null, a bare string, a number) and a short tuple are all handled, values are bounded to a single line of at most 120 characters, and an unidentifiable engine falls back to unknown engine. The classifier and the message renderer both go through it, so there is one definition of "malformed" in this path and the dedup key is always a string. A replica whose failures are all unidentifiable still counts as failed and collapses to one unknown engine entry — hasItems has already established that it reported failures, and calling it a clean empty would resurrect the very silent failure this PR removes.

This follows the same fail-closed shape the repo already uses for engines + time_range (#244) and for the HTML-fallback path, which likewise avoids "silently returning empty results".

Files changed

• src/error-handler.ts — createEngineFailureError, the exported normalizeEngineFailure shared with the classifier, and their formatting helpers
• src/search.ts — classifyEmptyResponses, the emptyResponses field and its two producers, and the guard in performWebSearch above the json branch
• __tests__/unit/error-handler.test.ts — 5 tests: message content, empty/absent tuple halves, the length cap, entries that are not tuples at all (null, ['yahoo'], [], 42), and a bare string entry reading as the engine name
• __tests__/unit/search.test.ts — 10 tests. Single instance: all-engines-failed raises (text), the same for json, genuinely-empty keeps the existing message, failing engines with rows returned still produce results, rows filtered to empty by min_score keep the existing message. Multi-instance: failover and fan-out each get "earlier replica failing + later replica clean → does not throw" and "every empty replica failing → throws naming the union", plus a dedup test asserting an engine failing on two replicas is named exactly once. Untrusted input: a response mixing malformed and well-formed entries must raise MCPSearXNGError rather than a raw TypeError and still name the well-formed engine, and an all-malformed response must still raise with exactly one unknown engine.
• README.md — one bullet in the searxng_web_search section, alongside the existing time_range fail-closed note

The pre-existing test "text output prepends infoboxes but omits unresponsive engines" already covers the failing-engines-with-rows case and is unchanged.

Verification

Baseline on main before any edit: build 0, lint 0, 773/773 tests, 96.1% lines / 92.11% branches. After the change:

$ npm run build      # exit 0
$ npm run lint       # exit 0  (eslint src __tests__ scripts --max-warnings=0)
$ npm test           # exit 0  — 790/790 passed (773 baseline + 17 new)
$ npm run test:coverage
                     # exit 0  — 96.2% lines / 92.17% branches overall
                     #           (gate: 90 / 85); search.ts 98.8 / 95.54,
                     #           error-handler.ts 98.43 / 93.4

I also confirmed the tests are not vacuous, by mutation rather than by assertion alone:

• Reverting the classifier to the single retained payload (classifyEmptyResponses([data])) fails exactly the four multi-instance tests, and leaves the single-instance ones green — which also demonstrates that path is untouched.
• Removing the guard from src/search.ts entirely fails exactly the two single-instance "raises" tests with Missing expected rejection.
• Removing the union's dedup keying fails only the dedup test, with brave (timeout), startpage (Suspended: CAPTCHA), brave (too many requests).
• Reverting either the classifier or the renderer to raw entry indexing fails only that layer's untrusted-input tests, with Cannot read properties of null (reading '0') and, for the renderer, b (r) in place of brave.

Trade-off worth your call

SearXNG's response lists engines that failed but not engines that answered successfully, so within a single instance a mixed outcome — some engines returned zero rows while one failed — is indistinguishable from a total failure and raises under this rule. (Across replicas this is now resolved: one clean replica settles it.) The cost of that false positive is a retry; the cost of today's behaviour is an agent silently accepting a failed search as fact, so I have erred toward surfacing. Happy to change it if you would rather be more conservative — either gate it behind an env flag, or narrow it to the case where the caller explicitly pinned engines=, where the denominator is known. Say which you prefer and I will push the change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NBoX4ym1XSuM4cHHdZ4ufd

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric	Results
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6803270143

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Aggregate empty replica responses before classifying failure

When multiple interchangeable instances all return zero rows, performFailoverSearch and the all-empty fan-out path retain only successes[0].data, so this condition depends on replica ordering. If the first replica reports failed engines but a later replica completes cleanly with no matches, the request incorrectly throws despite the clean result; if every replica reports different failures, only the first replica's engines are named. Classify the aggregate empty responses instead, treating the search as failed only when none completed cleanly and combining the applicable engine failures.

Useful? React with 👍 / 👎.

[shauneccles]

Valid finding — this was a real ordering dependency, and I've reworked the PR. Confirmed both paths retain one replica's payload: performFailoverSearch returns emptyResults[0].data and the all-empty fan-out branch returns successes[0].data, while servedBy correctly names them all. So the guard saw only the first replica's unresponsive_engines, which produced both problems you describe — a false throw when a later replica completed cleanly, and a partial engine list when replicas failed differently.

What changed in a785a7c:

• MultiInstanceSearchResult gains an optional emptyResponses: SearXNGWeb[]. Both helpers now populate it in their all-empty branches with every zero-row response, not just the retained one. servedBy semantics and the with-results return paths are untouched.
• A single classifyEmptyResponses(responses) does the classification, so the rule lives in one place rather than being duplicated in each helper. It returns null as soon as any zero-row response has no unresponsive engines — one clean empty replica is authoritative evidence that nothing matched — and otherwise returns the union of failures.
• The union is keyed by engine name in a Map and kept in first-seen order, so an engine failing on several replicas is named once with a deterministic reason. Nothing depends on incidental Set/Map iteration beyond documented insertion order.
• performWebSearch classifies once: classifyEmptyResponses(emptyResponses ?? [data]). The single-instance path leaves emptyResponses undefined and so classifies [data], exactly as before.

Five tests added for the multi-instance cases, which nothing previously exercised: failover and fan-out each get "earlier replica failing + later replica clean → does not throw" and "every empty replica failing → throws naming the union", plus a dedup test asserting an engine failing on two replicas appears exactly once with its first-seen reason.

I checked the new tests actually catch the bug rather than merely passing. Reverting the classifier to the single retained payload (classifyEmptyResponses([data])) fails exactly the four multi-instance tests and leaves the single-instance ones green, which also confirms that path is unchanged. Removing the dedup keying fails only the dedup test, with brave (timeout), startpage (Suspended: CAPTCHA), brave (too many requests).

Build, lint, npm test (786/786) and npm run test:coverage all pass; search.ts coverage went from 98.26/95.37 to 98.79/95.54.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a785a7ca63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Validate engine entries before indexing them

When a zero-result JSON response contains a malformed entry such as unresponsive_engines: [null], hasItems accepts the array and this indexing throws a raw TypeError before the bounded engine-failure error can be created. Parsed responses are only cast to SearXNGWeb, so this external data is not runtime-validated; skip or normalize non-array entries before reading their fields.

Useful? React with 👍 / 👎.

[shauneccles]

Valid, and worse than described — fixed in 82f2b1f. I reproduced it before changing anything:

null entry             -> THROW TypeError: Cannot read properties of null (reading '0')
bare string entry      -> OK "b (r)"
tuple missing reason   -> OK "yahoo"
empty tuple            -> OK "unknown engine"
number entry           -> OK "unknown engine"
well-formed            -> OK "brave (timeout)"

Two corrections to the report, both making it more serious rather than less:

1. describeEngineFailure in error-handler.ts was not already hardened against this. It read asEngineFailureText(entry[0]), which throws on null just as the classifier did — so both layers were vulnerable, not just the new one. My existing malformed-entry test passed [undefined, undefined], a well-formed two-element tuple, so it never exercised a malformed entry.
2. A bare string entry did not throw but rendered wrongly: indexing "brave" positionally yields entry[0] === "b", entry[1] === "r", so the message said b (r).

Fix, following your point about not letting two layers hold different notions of "malformed": one exported normalizeEngineFailure(entry: unknown): [string, string] in error-handler.ts, used by both the classifier and the renderer. It coerces a non-array entry to [entry, undefined] so a bare string reads as the engine name, runs both halves through the existing single-line/120-char bounding, and falls back to unknown engine. The dedup key is therefore always a string.

On the semantics you raised, I chose normalize, not skip. Skipping would mean a replica whose entries are all malformed yields size === 0 and returns null, i.e. "clean empty → No results found" — but hasItems has already established that this replica did report failures. Reporting "no results" for a replica that explicitly reported failing engines is precisely the silent failure this PR exists to remove, so it would be self-defeating. Normalizing keeps the error firing and degrades the message to unknown engine; dedup then collapses several unidentifiable failures under that one key, which reads correctly. Both the docstring on classifyEmptyResponses and the commit message record the reasoning.

Tests: four added. In error-handler, a mixed array of null, ['yahoo'], [], 42 and a well-formed tuple, plus one pinning the bare-string case to brave rather than b (r). In search, an end-to-end performWebSearch with [null, 'brave', ['yahoo'], ['startpage', 'Suspended: CAPTCHA']] asserting error.name === 'MCPSearXNGError' and that the well-formed entry is still named, plus the all-malformed case asserting it still raises with exactly one unknown engine.

Mutation-verified again. Reverting the classifier to raw indexing fails both new search tests with raw TypeError escaped: Cannot read properties of null (reading '0'); reverting the renderer fails both new error-handler tests, the second reproducing b (r) exactly.

Build, lint, npm test (790/790) and npm run test:coverage all pass. error-handler.ts coverage 98.34/93.18 → 98.43/93.40.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82f2b1f8b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve direct answers before raising engine failure

When SearXNG returns a usable answers or infoboxes entry with no result rows while an unrelated engine is unresponsive, this guard throws before formatSearchMetadata() can return that content. For example, an answerer plugin can supply a calculator answer even if a web engine times out; the search demonstrably ran and produced useful output, but this change turns it into a failed tool call. Restrict the engine-failure classification to responses that contain neither rows nor usable answer/infobox metadata.

Useful? React with 👍 / 👎.