Skip to content

feat(desktop): add Tavily web search settings - #439

Open
L4b0R wants to merge 3 commits into
OpenCoworkAI:mainfrom
L4b0R:feat/tavily-search-settings
Open

L4b0R wants to merge 3 commits into
OpenCoworkAI:mainfrom
L4b0R:feat/tavily-search-settings

Conversation

@L4b0R

@L4b0R L4b0R commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

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.

  • Return only configured/enabled state, never a saved key; preserve keys and custom search limits on ordinary saves.
  • Test the saved key only when clicked, with one basic Tavily search (1 credit), no automatic retry, and sanitized/localized status reporting. Unknown failures are not mislabeled as invalid keys.
  • Snapshot search settings and credentials at the start of each new generation or apply-comment run. Active runs retain their original snapshot.
  • Preserve per-run web authorization: enabling search/saving a key/testing a connection never grants task consent or consumes its call budget.
  • Keep direct page reading independent of missing, cleared, or unreadable Tavily credentials.
  • Exclude search-only credentials from Models → API Services, without deleting the Tavily key or breaking legacy model entries.
  • Update WEB_SEARCH.md and add focused coverage for persistence, credential formats/redaction, probe errors, snapshots, IPC, and UI actions.
  • Update the generation integration test fixture to include the canonical empty secrets map 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

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Build / CI / tooling
  • Breaking change

Linked issue

No linked issue. This is a focused settings follow-up to the existing web-research implementation.

Checklist

  • I checked the relevant implementation and repository guidance before starting
  • Full pre-push checks passed locally: pnpm -r typecheck, pnpm lint, and pnpm test (no hooks bypassed)
  • Added/updated tests for the change
  • Added a changeset for user-visible behavior
  • Updated docs for changed behavior

Validation

  • Full pre-push validation passed: all workspace package typechecks, Biome (663 files), automation-script tests, and all 10 Turborepo test tasks. Desktop: 171 test files, 2330 tests passed and 1 skipped; other package suites also passed (exporters retains 3 skipped tests).
  • Focused shared/i18n/desktop suites passed, including the follow-up's 124 provider/settings regression tests.
  • Affected package typechecks and complete Biome checks passed.
  • Electron main/preload/renderer production build passed.
  • Existing mocked research integration with real browser-rendered slides passed.
  • No live Tavily key/request or live-model run was used. Native Settings-window visual smoke testing has not been performed.

Design principles

  • Compatible: retain existing config/credential formats, custom limits, manual configuration, and per-run consent.
  • Upgradeable: existing versioned IPC/config mechanisms; no migration or new storage system required.
  • Lean: no new dependencies, providers, or research panel.
  • Elegant: one focused settings tab, explicit key mutations, and per-run configuration snapshots.

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.

Signed-off-by: HUANG <15866338256@163.com>
Signed-off-by: HUANG <15866338256@163.com>
Signed-off-by: HUANG <15866338256@163.com>
@github-actions github-actions Bot added docs Documentation area:desktop apps/desktop (Electron shell, renderer) labels Sep 25, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Nit] The enable/disable toggle wipes an unsaved API key. save() unconditionally runs setApiKey('') before it inspects the patch, so the switch handler onChange={(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 of save) and the switch at apps/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.
    if (input.apiKey !== undefined || input.clearKey === true) setApiKey('');
    The existing key-clearing test still passes because it submits { apiKey } / { clearKey: true }.

Questions

  • apps/desktop/src/main/provider-settings.ts:117 now skips every secret whose id is neither in cfg.providers nor 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 whose providers entry 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-search channels, strict zod contracts in packages/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. createWebResearchRun snapshots enabled/limits and the ciphertext at run start, defers decryption to first web_search use via getApiKey, and keeps web_fetch working 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 (pendingSave chain) so overlapping saves cannot lose the key or switch, errors are surfaced as CodesignError rather than silently swallowed, and hasKey/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/i18n changes. 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 safeStorage or 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 toProviderRows case showing a legacy non-builtin model credential still renders when it has no providers entry.

Open-CoDesign Bot

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:desktop apps/desktop (Electron shell, renderer) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant