Skip to content

Commit 9cc17ee

Browse files
committed
fix(pi): make the search provider drift guards actually fire
The "you cannot add a provider without mirroring it" story rested on two mechanisms that did not hold. Verified by adding a fifth provider to `PI_SEARCH_PROVIDERS` and running the build: it produced only two errors, and every test still passed. - `normalizePiSearchRecords` assigns to `let built` inside its switch rather than returning, so unlike its two siblings a missing case was not a type error — it silently normalized the new provider to zero results. Added an explicit `never` check. - The sandbox copy's `normalizeRecords` used a trailing `else` for Firecrawl, so an unmirrored provider was silently normalized with Firecrawl's field names; `extractRecords` did the same with its `payload.data` tail. Both now test for `firecrawl` explicitly and throw otherwise. - `Record<PiSearchProvider, ...>` on the `TOOLS` and `payloads` fixtures looked like exhaustiveness guards but are inert: `apps/sim/tsconfig.json` excludes `**/*.test.ts`, and vitest transpiles without typechecking. Both suites drive their providers off `Object.keys(fixture)`, so a missing provider was skipped rather than failed. Each suite now asserts its fixture covers the registry. Re-running the same experiment now yields three compile errors plus two test failures naming the missing fixtures.
1 parent 41d2917 commit 9cc17ee

4 files changed

Lines changed: 33 additions & 3 deletions

File tree

apps/sim/executor/handlers/pi/search/extension-source.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { tmpdir } from 'node:os'
1111
import { join } from 'node:path'
1212
import { pathToFileURL } from 'node:url'
1313
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
14-
import type { PiSearchProvider } from '@/executor/handlers/pi/keys'
14+
import { PI_SEARCH_PROVIDERS, type PiSearchProvider } from '@/executor/handlers/pi/keys'
1515
import {
1616
PI_SEARCH_API_KEY_ENV_VAR,
1717
PI_SEARCH_EXTENSION_PATH,
@@ -258,6 +258,13 @@ describe('normalization parity with the host adapter', () => {
258258
},
259259
}
260260

261+
// `Record<PiSearchProvider, unknown>` on `payloads` is not enforced anywhere — test files are
262+
// excluded from tsconfig and vitest only transpiles — so a provider missing here would be quietly
263+
// skipped by `it.each` instead of failing. This assertion is what actually holds the copies together.
264+
it('covers every registered search provider', () => {
265+
expect(Object.keys(payloads).sort()).toEqual(Object.keys(PI_SEARCH_PROVIDERS).sort())
266+
})
267+
261268
it.each(Object.keys(payloads) as PiSearchProvider[])(
262269
'produces the same envelope as normalize.ts for %s',
263270
async (provider) => {

apps/sim/executor/handlers/pi/search/extension-source.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,9 @@ function extractRecords(provider, payload) {
177177
if (provider === "serper") {
178178
return Array.isArray(payload.organic) ? payload.organic : []
179179
}
180+
if (provider !== "firecrawl") {
181+
throw new Error("Unsupported search provider: " + provider)
182+
}
180183
const data = payload.data
181184
if (Array.isArray(data)) return data
182185
if (!isRecord(data)) return []
@@ -210,12 +213,16 @@ function normalizeRecords(provider, records, limit) {
210213
snippet: firstText(record.excerpts),
211214
publishedDate: firstText(record.publish_date, record.publishedDate),
212215
})
213-
} else {
216+
} else if (provider === "firecrawl") {
214217
built = buildResult({
215218
title: record.title,
216219
url: record.url,
217220
snippet: firstText(record.description, record.snippet),
218221
})
222+
} else {
223+
// Never the trailing else: an unmirrored provider would otherwise be normalized with
224+
// Firecrawl's field names and quietly return nothing.
225+
throw new Error("Unsupported search provider: " + provider)
219226
}
220227
if (built) results.push(built)
221228
}

apps/sim/executor/handlers/pi/search/normalize.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,13 @@ export function normalizePiSearchRecords(
311311
snippet: firstText(record.description, record.snippet),
312312
})
313313
break
314+
default: {
315+
// Unlike the sibling switches this one assigns rather than returns, so without an explicit
316+
// exhaustiveness check a new provider would compile clean and silently normalize to zero
317+
// results. `satisfies never` fails the build instead.
318+
const unhandled: never = provider
319+
throw new Error(`Unhandled Pi search provider: ${String(unhandled)}`)
320+
}
314321
}
315322

316323
if (built) results.push(built)

apps/sim/executor/handlers/pi/search/parity.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { tmpdir } from 'node:os'
1515
import { join } from 'node:path'
1616
import { pathToFileURL } from 'node:url'
1717
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
18-
import type { PiSearchProvider } from '@/executor/handlers/pi/keys'
18+
import { PI_SEARCH_PROVIDERS, type PiSearchProvider } from '@/executor/handlers/pi/keys'
1919
import {
2020
PI_SEARCH_API_KEY_ENV_VAR,
2121
PI_SEARCH_EXTENSION_SOURCE,
@@ -110,6 +110,15 @@ function buildHostRequest(provider: PiSearchProvider): CapturedRequest {
110110
}
111111
}
112112

113+
// `Record<PiSearchProvider, ...>` on TOOLS looks like it enforces this, but `**/*.test.ts` is
114+
// excluded from tsconfig and vitest only transpiles — so a provider missing from the fixture would
115+
// silently be skipped by `describe.each` rather than failing. This assertion is the real gate.
116+
describe('fixture coverage', () => {
117+
it('exercises every registered search provider', () => {
118+
expect(Object.keys(TOOLS).sort()).toEqual(Object.keys(PI_SEARCH_PROVIDERS).sort())
119+
})
120+
})
121+
113122
describe.each(Object.keys(TOOLS) as PiSearchProvider[])('%s request parity', (provider) => {
114123
it('sends the same url, headers, and body from the sandbox as from the host', async () => {
115124
const sandbox = await captureExtensionRequest(provider)

0 commit comments

Comments
 (0)