diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index da0093fa3..2a4f4dff9 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -39,6 +39,7 @@ ### Fixed +- Fixed `web_search` SearXNG fallback when HTTP 200 responses contain no usable results plus `unresponsive_engines`; SearXNG now raises a transient provider error, and the fallback loop rejects any provider response with no renderable content before formatting an invisible success ([#2571](https://github.com/can1357/oh-my-pi/issues/2571)). - Fixed `read` on a GitHub commit URL (`github.com///commit/`) returning the raw commit HTML page instead of structured content. `parseGitHubUrl` had no `commit` case, so commit URLs fell through to generic HTML rendering; they now resolve via the commits API and render as markdown (subject, author, stats, parents, full commit message, and a per-file unified diff), matching the existing blob/tree/issue/PR handling. - Fixed release runs being silently cancelled by a later `main` push, which left tagged versions (`v15.12.6` in the wild) without a GitHub Release or npm publish. The CI workflow's `concurrency` group was `${{ github.workflow }}-${{ github.ref }}`, and since the release-script commit + `v*` tag are pushed atomically to `refs/heads/main`, the release run shared the `CI-refs/heads/main` group with every subsequent push; `cancel-in-progress: true` then killed it before `release_binary` / `release_github` / `release_npm` could run, and no future run carried the release tag at HEAD. The group now resolves to a per-sha `release-` slot with `cancel-in-progress: false` whenever the push subject matches `chore: bump version to ` (the release-script convention) or `github.ref` is a `v*` tag (`workflow_dispatch` recovery), so release runs are isolated from PR/main churn ([#2564](https://github.com/can1357/oh-my-pi/issues/2564)). - Allowed `compat.streamIdleTimeoutMs: 0` in `models.yml`. The schema was `.positive()`, so the documented "set to 0 to disable" escape hatch was only reachable via the global env var ([#2422](https://github.com/can1357/oh-my-pi/issues/2422)) diff --git a/packages/coding-agent/src/web/search/index.ts b/packages/coding-agent/src/web/search/index.ts index 12b110158..f671734a8 100644 --- a/packages/coding-agent/src/web/search/index.ts +++ b/packages/coding-agent/src/web/search/index.ts @@ -115,6 +115,15 @@ function formatForLLM(response: SearchResponse): string { return parts.join("\n"); } +function hasRenderableSearchContent(response: SearchResponse): boolean { + if (response.answer?.trim()) return true; + if (response.sources.length > 0) return true; + if (response.citations?.length) return true; + if (response.relatedQuestions?.some(question => question.trim())) return true; + if (response.searchQueries?.some(query => query.trim())) return true; + return false; +} + interface ExecuteSearchOptions { authStorage: AuthStorage; sessionId?: string; @@ -162,6 +171,10 @@ async function executeSearch( sessionId, }); + if (!hasRenderableSearchContent(response)) { + throw new SearchProviderError(provider.id, `${provider.label} returned no renderable search content.`, 204); + } + const text = formatForLLM(response); return { diff --git a/packages/coding-agent/src/web/search/providers/searxng.ts b/packages/coding-agent/src/web/search/providers/searxng.ts index 6d75907e4..9decfe9c7 100644 --- a/packages/coding-agent/src/web/search/providers/searxng.ts +++ b/packages/coding-agent/src/web/search/providers/searxng.ts @@ -281,9 +281,21 @@ export async function searchSearXNG(params: { }); } + const limitedSources = sources.slice(0, numResults); + if (limitedSources.length === 0 && response.unresponsive_engines?.length) { + const upstreamFailures = response.unresponsive_engines + .map(([engine, reason]) => `${engine}: ${reason}`) + .join("; "); + throw new SearchProviderError( + "searxng", + `SearXNG returned no usable results; upstream engines failed: ${upstreamFailures}`, + 503, + ); + } + return { provider: "searxng", - sources: sources.slice(0, numResults), + sources: limitedSources, relatedQuestions: response.suggestions?.length ? response.suggestions : undefined, }; } diff --git a/packages/coding-agent/test/tools/web-search-searxng.test.ts b/packages/coding-agent/test/tools/web-search-searxng.test.ts index d49386210..abbaf84ce 100644 --- a/packages/coding-agent/test/tools/web-search-searxng.test.ts +++ b/packages/coding-agent/test/tools/web-search-searxng.test.ts @@ -5,6 +5,7 @@ import * as path from "node:path"; import type { FetchImpl } from "@oh-my-pi/pi-ai/types"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { searchSearXNG } from "@oh-my-pi/pi-coding-agent/web/search/providers/searxng"; +import { SearchProviderError } from "@oh-my-pi/pi-coding-agent/web/search/types"; describe("SearXNG web search provider", () => { afterEach(() => { @@ -228,4 +229,27 @@ describe("SearXNG web search provider", () => { expect(captured.headers?.get("Authorization")).toBe("Bearer bearer-token"); }); + + it("treats empty SearXNG results with upstream failures as a provider error", async () => { + process.env.SEARXNG_ENDPOINT = "https://searx.example.org"; + + const fetchMock: FetchImpl = () => + Promise.resolve( + new Response( + JSON.stringify({ + results: [], + unresponsive_engines: [ + ["brave", "Suspended: too many requests"], + ["duckduckgo", "CAPTCHA"], + ], + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + + await expect(searchSearXNG({ query: "throttled search", fetch: fetchMock })).rejects.toThrow(SearchProviderError); + await expect(searchSearXNG({ query: "throttled search", fetch: fetchMock })).rejects.toThrow( + "SearXNG returned no usable results; upstream engines failed: brave: Suspended: too many requests; duckduckgo: CAPTCHA", + ); + }); }); diff --git a/packages/coding-agent/test/web/search/abort-and-timeout.test.ts b/packages/coding-agent/test/web/search/abort-and-timeout.test.ts index f89e84920..3ca38a00c 100644 --- a/packages/coding-agent/test/web/search/abort-and-timeout.test.ts +++ b/packages/coding-agent/test/web/search/abort-and-timeout.test.ts @@ -160,11 +160,13 @@ describe("Brave provider hard-timeout wiring", () => { describe("executeSearch abort propagation", () => { afterEach(() => vi.restoreAllMocks()); - function fakeProvider(behaviour: (params: SearchParams) => Promise): provider.SearchProvider { - const id: SearchProviderId = "anthropic"; + function fakeProvider( + id: SearchProviderId, + behaviour: (params: SearchParams) => Promise, + ): provider.SearchProvider { return { id, - label: "Anthropic", + label: id, isAvailable: () => true, isExplicitlyAvailable: () => true, search: behaviour, @@ -178,10 +180,10 @@ describe("executeSearch abort propagation", () => { // re-throw stops the loop immediately. const secondProviderSearch = vi.fn(); vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([ - fakeProvider(async () => { + fakeProvider("anthropic", async () => { throw new DOMException("aborted", "AbortError"); }), - fakeProvider(secondProviderSearch), + fakeProvider("brave", secondProviderSearch), ]); const tool = new WebSearchTool(FAKE_SESSION); @@ -197,7 +199,7 @@ describe("executeSearch abort propagation", () => { // flow. A genuine provider error should still produce an error result // rather than throwing. vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([ - fakeProvider(async () => { + fakeProvider("anthropic", async () => { throw new Error("upstream 500"); }), ]); @@ -209,4 +211,33 @@ describe("executeSearch abort propagation", () => { expect(block && "text" in block ? block.text : "").toContain("upstream 500"); expect(result.details?.error).toContain("upstream 500"); }); + + it("falls through when a provider returns no renderable search content", async () => { + const emptyProviderSearch = vi.fn( + async (): Promise => ({ + provider: "searxng", + sources: [], + }), + ); + const sourceProviderSearch = vi.fn( + async (): Promise => ({ + provider: "brave", + sources: [{ title: "Fallback result", url: "https://example.com/fallback", snippet: "fallback body" }], + }), + ); + vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([ + fakeProvider("searxng", emptyProviderSearch), + fakeProvider("brave", sourceProviderSearch), + ]); + + const tool = new WebSearchTool(FAKE_SESSION); + const result = await tool.execute("test-id", { query: "anything" }); + + expect(emptyProviderSearch).toHaveBeenCalledTimes(1); + expect(sourceProviderSearch).toHaveBeenCalledTimes(1); + const block = result.content[0]; + expect(block?.type).toBe("text"); + expect(block && "text" in block ? block.text : "").toContain("Fallback result"); + expect(result.details?.response.provider).toBe("brave"); + }); });