Repository navigation
feat(desktop): add Tavily web search settings - #439
Merged
Sun-sunshine06 merged 3 commits intoSep 28, 2026
Merged
Conversation
Signed-off-by: HUANG <15866338256@163.com>
Signed-off-by: HUANG <15866338256@163.com>
Signed-off-by: HUANG <15866338256@163.com>
Contributor
There was a problem hiding this comment.
Findings
- [Nit] The enable/disable toggle wipes an unsaved API key.
save()unconditionally runssetApiKey('')before it inspects the patch, so the switch handleronChange={(event) => void save({ enabled: event.target.checked })}clears any pasted-but-unsaved key. Users who paste a key and then flip the switch lose the input and must re-paste. Evidence:apps/desktop/src/renderer/src/components/settings/WebSearchTab.tsx:50(setApiKey('')at the top ofsave) and the switch atapps/desktop/src/renderer/src/components/settings/WebSearchTab.tsx:141.
Suggested fix / next action: only clear the transient field when the patch actually touches the credential, e.g.The existing key-clearing test still passes because it submitsif (input.apiKey !== undefined || input.clearKey === true) setApiKey('');
{ apiKey }/{ clearKey: true }.
Questions
apps/desktop/src/main/provider-settings.ts:117now skips every secret whose id is neither incfg.providersnor a supported onboarding provider (if (entry === null) continue;). That is exactly the intended Tavily case, but it also changes the previously documented "union of provider entries and stored secrets" behavior whose stated goal was to avoid silently dropping rows (apps/desktop/src/main/provider-settings.ts:104-113). Is there any supported config path that persists a model credential whose id is neither a stored provider entry nor a built-in onboarding provider (for example an older/custom provider whoseprovidersentry was lost)? If not, this is fine and can be ignored. If yes, consider an explicit search-only secret key (tavily) rather than allow-listing by entry, so legacy model credentials still surface in Models → API Services even if only readable.
Summary
- Review mode: initial
- Scope looks correct and self-consistent: versioned
settings:v1:get/save/test-web-searchchannels, strict zod contracts inpackages/shared/src/web-search-settings.ts, a preload bridge that never accepts credentials/authorization on test, and a renderer tab that keeps the key as transient UI state and reports sanitized statuses only. - Runtime wiring is sound.
createWebResearchRunsnapshotsenabled/limits and the ciphertext at run start, defers decryption to firstweb_searchuse viagetApiKey, and keepsweb_fetchworking when the Tavily credential is missing/unreadable (apps/desktop/src/main/web-research-run.ts:14-41). Both generation entry points now take the per-run snapshot (apps/desktop/src/main/ipc/generate.ts:1098,apps/desktop/src/main/ipc/generate.ts:1610), preserving the prior "no key decrypt when disabled" behavior. - The probe is bounded and safe: one
POST https://api.tavily.com/search, no redirects with the credential, 1 MiB cap, local timeout distinct from network error, and no budget consumption (apps/desktop/src/main/web-research-network.ts:326-379). HTTP→status mapping matches the linked public Tavily reference (401/429/432/433). - Settings persistence is serialized (
pendingSavechain) so overlapping saves cannot lose the key or switch, errors are surfaced asCodesignErrorrather than silently swallowed, andhasKey/key is never returned over IPC (apps/desktop/src/main/web-search-settings.ts:25-85). - Constraints: no new dependencies (lockfile unchanged), no direct provider-SDK imports, no release/distribution files touched, and a changeset is present covering the user-visible
desktop/shared/i18nchanges. This branch has no linked issue, so there is no completion claim to validate. - Residual risk: the PR body states native Settings-window visual smoke testing was not performed; automated coverage is strong but does not exercise real Electron
safeStorageor an actual network round trip.
Testing
- Not run (automation). Suggested additions if the maintainer wants more coverage: an assertion that the enable/disable toggle does not clear an unsaved key after the fix above, and (if the question above is resolved as a real path) a
toProviderRowscase showing a legacy non-builtin model credential still renders when it has noprovidersentry.
Open-CoDesign Bot
Sun-sunshine06
pushed a commit
that referenced
this pull request
Sep 28, 2026
Unify deterministic preview source editing and extend text coverage while retaining source hashes, validation-before-save, and stale-source/busy protections. Maintainer validation of bf20818 against main 143cf96: - Integrated current main without rewriting the original PR history; preserved source-edit and Tavily locale changes. The Tavily changes were already merged separately in #439 (143cf96); they appear only in the branch-update comparison, not as new scope in this PR's diff against main. - Frozen-lockfile installation, lint, all workspace typechecks, and the Electron production build passed on Windows with Node 24.11.0 and pnpm 10.33.4. - The initial source-edit repair (6c762ed) passed a fresh full suite: 3997 Vitest tests and 61 script tests, all 10 Turbo tasks with no cache hits. After the final test-budget-only change (bf20818), pnpm test passed again: desktop's 2578 tests and all 61 script tests newly executed; the other nine packages reused valid passing caches (1419 tests). Three existing opt-in exporter browser tests remained skipped. - SourceEditPanel ran 22/22 tests in system Chrome without skips. Focused desktop 483, runtime 85, and shared 18 tests passed. - A pre-push failure exposed a browser-test timing race: pointerdown mutation could correctly disable Save before submit. A test-only fix injects the mutation during submit capture and retains a held-pointer regression, original timeouts, actual validation replies, and all no-write assertions. Controlled serial red/green runs verified the cause with unchanged source hashes during each run. - Linux CI on 6c762ed exposed the first Tavily registration test's default five-second cold-import timeout. The final one-line correction gives it the same 15-second initialization budget as its neighboring registration tests; no assertions, production code, browser timeouts, or global configuration changed. All 70 onboarding IPC tests passed, with lint, workspace typechecks, and the desktop build rerun successfully. - Actual Electron 39.8.10/preload/main IPC smoke passed on 6c762ed with isolated synthetic data: exact save, cancel, restored interaction, runtime-mutation rejection, and stale-source rejection without overwriting external edits. This smoke evidence is reused for bf20818 because all tracked files except the registration-test budget are identical; it was not rerun. No live model calls or user credentials were used. - Final head bf20818 passed GitHub CI 36441801400 (lint, typecheck, test, and Electron build smoke), CodeQL 36441801533, Dependency Review 36441801597, and PR review. All reported checks succeeded; tested main remained 143cf96 immediately before merge. Signed-off-by: Sun-sunshine06 <Sun-sunshine06@users.noreply.github.com> Co-Authored-By: HUANG <15866338256@163.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a bilingual Settings → Web Search entry so users can enable/disable Tavily search and save, replace, clear, or explicitly test their API key without editing configuration files or restarting the app. Reuse the existing config cache, credential store, versioned IPC, and bounded public-network transport.
secretsmap now read when taking the initial run snapshot; the suite's 26 tests pass with no skipped assertions.Request fields, basic-search credit cost, and HTTP mappings were checked against the official Tavily Search reference: 401 authentication, 429 rate limit, 432/433 usage limits; other responses remain service/unknown failures.
Type of change
Linked issue
No linked issue. This is a focused settings follow-up to the existing web-research implementation.
Checklist
pnpm -r typecheck,pnpm lint, andpnpm test(no hooks bypassed)Validation
Design principles
Screenshots / recordings
Not captured in this environment. UI interaction, navigation, locale coverage, and safe bridge behavior are covered by automated tests; native visual review remains pending.