fix(web-plugin-config): address review — one options snapshot per search, no public value exports
Three findings from review survived against the staged-save head: The search provider read its options thunk per property, so a settings write landing inside credential resolution sent the key resolved from the old section to the endpoint named by the new one. Each operation now snapshots once at its entry and threads that snapshot into credential resolution; a regression test drives a commit into the middle of a search and pins that the endpoint, model, and key all come from the section the search started on. The /client entry exported components, controllers, and namespace constants with no consumer, which the client export discipline allows only with sign-off. Only types remain. The duplicate per-card Injected/Face interface pairs are one declaration each now, so a member added to one side cannot silently miss the other. The credential state carries the reference it describes and its writability: a reference change no longer projects the old answer onto the new name, an out-of-order response for a stale reference is dropped, and a key that a deployment sources from the process environment disables the control instead of inviting a write the Host must refuse. Also corrected three prose claims against the code they describe: the card's fields do not differ by platform (the served schema does), the section's empty line counts registered rather than visible cards and is read once, and the search README overstated what a configuration surface learns about a key.
This commit is contained in:
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/web/web-search-deepseek/README.md
|
||||
README.md: 3e8b631b5775793e6e6e542c810edce9d92a0d59
|
||||
README.zh.md: 9bd59000c2a0054265444aa90fc46ff5f34fd607
|
||||
README.md: 1e90f947b4a58288722307aaed50c4889cfe7cb4
|
||||
README.zh.md: 21962bcd390f253f899000be7273a79485821018
|
||||
@@ -34,7 +34,7 @@ It reuses the `DEEPSEEK_API_KEY` credential reference (no new secret) but **not*
|
||||
baseURL: https://gateway.internal/anthropic/v1
|
||||
```
|
||||
|
||||
The entry above is the base layer of the `web-search-deepseek` Settings section: a user layer over it reaches the NEXT search, because the provider projects the section per call rather than capturing it at registration. The seam's provider selection therefore never flickers when an endpoint or model changes. `apiKey` carries `role('secret')`, so it never rides a `describe()` response in any layer — a configuration surface learns only that a key is set.
|
||||
The entry above is the base layer of the `web-search-deepseek` Settings section: a user layer over it reaches the NEXT search, because the provider projects the section per call rather than capturing it at registration. The seam's provider selection therefore never flickers when an endpoint or model changes. `apiKey` carries `role('secret')`, so it never rides a `describe()` response in any layer — a configuration surface learns only whether the credentials domain holds a value for the reference `apiKeyEnv` names, never whether a layer carries a literal key.
|
||||
|
||||
## Mapping
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ Exa 和 Perplexity 提供专用搜索端点,DeepSeek 则没有。该提供方
|
||||
baseURL: https://gateway.internal/anthropic/v1
|
||||
```
|
||||
|
||||
上面的条目是 `web-search-deepseek` Settings 段的 base 层:叠加其上的用户层会作用于**下一次**搜索,因为提供方是按次投影该段,而不是在注册时固化它。因此端点或模型变化时,seam 的提供方选择不会闪断。`apiKey` 带有 `role('secret')`,所以它在任何一层都不会出现在 `describe()` 响应中——配置表层只能知道密钥是否已设置。
|
||||
上面的条目是 `web-search-deepseek` Settings 段的 base 层:叠加其上的用户层会作用于**下一次**搜索,因为提供方是按次投影该段,而不是在注册时固化它。因此端点或模型变化时,seam 的提供方选择不会闪断。`apiKey` 带有 `role('secret')`,所以它在任何一层都不会出现在 `describe()` 响应中——配置表层只能知道 credentials 领域是否为 `apiKeyEnv` 所命名的引用持有值,而无从知道某一层是否带着字面密钥。
|
||||
|
||||
## 映射
|
||||
|
||||
|
||||
@@ -178,41 +178,42 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
|
||||
readonly id = DEEPSEEK_PROVIDER_ID
|
||||
|
||||
/**
|
||||
* @param resolveOptions - the options for the NEXT operation. A thunk rather
|
||||
* than a value because the plugin's settings section can change between
|
||||
* searches, and re-registering the provider to carry a new endpoint would
|
||||
* make the seam's selection observable to the user as a flicker.
|
||||
* @param resolveOptions - the options for the NEXT operation, snapshotted
|
||||
* once at each operation's entry so one search never mixes two sections. A
|
||||
* thunk rather than a value because the plugin's settings section can change
|
||||
* between searches, and re-registering the provider to carry a new endpoint
|
||||
* would make the seam's selection observable to the user as a flicker.
|
||||
*/
|
||||
constructor(private readonly resolveOptions: () => DeepSeekSearchProviderOptions) {}
|
||||
|
||||
/** Options resolved per read, so a committed settings change reaches the next search. */
|
||||
private get options(): DeepSeekSearchProviderOptions {
|
||||
return this.resolveOptions()
|
||||
}
|
||||
|
||||
available(): boolean {
|
||||
return ((this.options.apiKey?.length ?? 0) > 0 || this.options.resolveApiKey !== undefined)
|
||||
&& URL.canParse(this.options.baseURL)
|
||||
&& isPositiveInteger(this.options.maxTokens)
|
||||
&& isPositiveInteger(this.options.maxUses)
|
||||
const options = this.resolveOptions()
|
||||
return ((options.apiKey?.length ?? 0) > 0 || options.resolveApiKey !== undefined)
|
||||
&& URL.canParse(options.baseURL)
|
||||
&& isPositiveInteger(options.maxTokens)
|
||||
&& isPositiveInteger(options.maxUses)
|
||||
}
|
||||
|
||||
async search(request: WebSearchRequest, signal?: AbortSignal): Promise<WebSearchResult> {
|
||||
const apiKey = await this.apiKey(signal)
|
||||
// One snapshot for the whole operation: credential resolution awaits, and a
|
||||
// settings write landing inside that await must not send the key resolved
|
||||
// from the old section to the endpoint named by the new one.
|
||||
const options = this.resolveOptions()
|
||||
const apiKey = await this.apiKey(options, signal)
|
||||
throwIfSearchAborted(signal)
|
||||
const endpoint = `${this.options.baseURL}/messages`
|
||||
const endpoint = `${options.baseURL}/messages`
|
||||
const body: DeepSeekSearchLlmRequest['body'] = {
|
||||
model: this.options.model,
|
||||
max_tokens: this.options.maxTokens,
|
||||
model: options.model,
|
||||
max_tokens: options.maxTokens,
|
||||
messages: [{
|
||||
role: 'user',
|
||||
content: [{ type: 'text', text: `Perform a web search for the query: ${request.query}` }],
|
||||
}],
|
||||
tools: [{ type: 'web_search_20250305', name: 'web_search', max_uses: this.options.maxUses }],
|
||||
tools: [{ type: 'web_search_20250305', name: 'web_search', max_uses: options.maxUses }],
|
||||
}
|
||||
this.options.recordRequest?.({
|
||||
options.recordRequest?.({
|
||||
endpoint,
|
||||
apiVersion: this.options.apiVersion,
|
||||
apiVersion: options.apiVersion,
|
||||
body,
|
||||
})
|
||||
throwIfSearchAborted(signal)
|
||||
@@ -226,7 +227,7 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
|
||||
// may expect `Authorization: Bearer` — send both so either resolves.
|
||||
'x-api-key': apiKey,
|
||||
'authorization': `Bearer ${apiKey}`,
|
||||
'anthropic-version': this.options.apiVersion,
|
||||
'anthropic-version': options.apiVersion,
|
||||
'content-type': 'application/json',
|
||||
'accept': 'application/json',
|
||||
'user-agent': USER_AGENT,
|
||||
@@ -268,13 +269,18 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
|
||||
}
|
||||
}
|
||||
|
||||
/** Resolve one operation's credential without retaining it on the provider. */
|
||||
private async apiKey(signal?: AbortSignal): Promise<string> {
|
||||
/**
|
||||
* Resolve one operation's credential without retaining it on the provider.
|
||||
* @param options - the caller's snapshot, so the key and the endpoint it is sent to come from one section.
|
||||
* @param signal - abort signal for the surrounding search.
|
||||
* @returns the resolved key.
|
||||
*/
|
||||
private async apiKey(options: DeepSeekSearchProviderOptions, signal?: AbortSignal): Promise<string> {
|
||||
throwIfSearchAborted(signal)
|
||||
if (this.options.apiKey !== undefined && this.options.apiKey.length > 0) return this.options.apiKey
|
||||
if (options.apiKey !== undefined && options.apiKey.length > 0) return options.apiKey
|
||||
let resolved: string | undefined
|
||||
try {
|
||||
resolved = await abortable(this.options.resolveApiKey?.() ?? Promise.resolve(undefined), signal)
|
||||
resolved = await abortable(options.resolveApiKey?.() ?? Promise.resolve(undefined), signal)
|
||||
} catch (error: unknown) {
|
||||
if (signal?.aborted === true || isAbortError(error)) throw searchAborted(signal, error)
|
||||
throw new WebError(
|
||||
@@ -284,7 +290,7 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
|
||||
)
|
||||
}
|
||||
if (resolved !== undefined && resolved.length > 0) return resolved
|
||||
const ref = this.options.apiKeyEnv ?? 'DEEPSEEK_API_KEY'
|
||||
const ref = options.apiKeyEnv ?? 'DEEPSEEK_API_KEY'
|
||||
throw new WebError(
|
||||
`DeepSeek search has no API key for "${ref}"; store it through the credentials service`
|
||||
+ ' (the web Models page writes it), export it in the launching environment, or set a literal'
|
||||
|
||||
@@ -205,6 +205,34 @@ describe('DeepSeekSearchProvider request mapping', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('DeepSeekSearchProvider settings changes mid-search', () => {
|
||||
it('serves one search from one section even when settings land during credential resolution', async () => {
|
||||
// The section the search starts on, and the one a user commits while the
|
||||
// credential is still resolving.
|
||||
const before = { ...options, apiKey: '', baseURL: 'https://before.test/v1', model: 'model-before', maxUses: 2 }
|
||||
const after = { ...options, apiKey: '', baseURL: 'https://after.test/v1', model: 'model-after', maxUses: 9 }
|
||||
let current = before
|
||||
let commitSettings = () => {}
|
||||
const resolveApiKey = () => new Promise<string>((resolve) => {
|
||||
commitSettings = () => { current = after; resolve('key-from-before') }
|
||||
})
|
||||
const fetchMock = vi.fn(async () => jsonResponse(searchResponse()))
|
||||
vi.stubGlobal('fetch', fetchMock)
|
||||
|
||||
const provider = new DeepSeekSearchProvider(() => ({ ...current, resolveApiKey }))
|
||||
const search = provider.search({ query: 'q' })
|
||||
await vi.waitFor(() => { expect(typeof commitSettings).toBe('function') })
|
||||
commitSettings()
|
||||
await search
|
||||
|
||||
const [endpoint, init] = fetchMock.mock.calls[0] as unknown as [string, RequestInit]
|
||||
// The key resolved from `before` must never reach `after`'s origin.
|
||||
expect(endpoint).toBe('https://before.test/v1/messages')
|
||||
expect((init.headers as Record<string, string>)['x-api-key']).toBe('key-from-before')
|
||||
expect(JSON.parse(String(init.body))).toMatchObject({ model: 'model-before' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('DeepSeekSearchProvider error handling', () => {
|
||||
it('does not start credential resolution or dispatch for a pre-aborted call', async () => {
|
||||
const resolveApiKey = vi.fn(async () => 'late-key')
|
||||
|
||||
Reference in New Issue
Block a user