Skip to content

fix(web): convert provider-editor credential reads to TanStack Query#33173

Merged
vex-assistant-bot[bot] merged 1 commit into
mainfrom
devin/1780448993-provider-editor-tq-credential
Jun 3, 2026
Merged

fix(web): convert provider-editor credential reads to TanStack Query#33173
vex-assistant-bot[bot] merged 1 commit into
mainfrom
devin/1780448993-provider-editor-tq-credential

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Prompt / plan

Same pattern as WEB-2H fix in PR #33165 — the provider editor modal has imperative useCallback + useEffect secret reads with an empty catch that permanently sets hasStoredCredential = false on any transient daemon error. No retry, no Sentry reporting.

What changed

Converted both imperative daemon calls to TQ useQuery hooks:

1. Credential presence check (secretsReadPost)

  • credentialPresenceQuery: TQ query keyed on [assistantId, service, field] parsed from the credential reference
  • enabled gate: assistantId && authType === "api_key" && parsedCredRef && isOrgReady
  • retry: shouldRetryDaemonError — retries 503/502/401/400-org-header with exponential backoff
  • bestEffort: true Sentry reporting when retries exhaust
  • hasStoredCredential derived from credentialPresenceQuery.data ?? false instead of manual useState

2. Available credentials list (secretsGet)

  • credentialsListQuery: TQ query keyed on [assistantId]
  • Same isOrgReady gate, retry predicate, and Sentry reporting
  • availableCredentials derived from query data instead of manual useState

3. Eliminated manual plumbing

  • Removed loadCredentialPresence and loadAvailableCredentials imperative callbacks
  • Removed hasStoredCredential, isLoadingCredential, availableCredentials useState — all derived from TQ query state
  • Removed imperative void loadCredentialPresence(v) from dropdown onChange handlers — TQ auto-refetches when credential ref (query key) changes
  • Optimistic cache update after saving a key (queryClient.setQueryData)

Root cause analysis

  1. How did the code get into this state? The provider editor predates the bestEffort and shouldRetryDaemonError conventions. Imperative fetches with empty catches were the only pattern available.
  2. What mistakes led to it? No retry on daemon calls + empty catch = transient errors treated as permanent failures. The hasStoredCredential(false) fallback is silently wrong.
  3. Warning signs missed? The same pattern was caught and fixed in web-search-card.tsx (WEB-2H / PR fix(web): root-cause fix for daemon query retries + TQ conversion (WEB-7, WEB-2H) #33165). This modal wasn't covered because it's lower traffic (user-initiated modal vs always-mounted card).
  4. Prevention? The shouldRetryDaemonError convention is now documented. New daemon queries should use TQ with the retry predicate and isOrgReady gate.

Closes LUM-2205
Related: PR #33165 (WEB-2H fix, same pattern in web-search-card.tsx)

Test plan

  • bunx tsc --noEmit — clean
  • bun run lint — clean
  • Config/logic-only changes — behavioral verification is Sentry noise clearing post-deploy
  • The modal is user-initiated (provider settings → edit connection), so daemon is typically ready. The fix prevents the edge case where it isn't.

Link to Devin session: https://app.devin.ai/sessions/c2e17ff1867f4ebd90aac007ea0f5453
Requested by: @ashleeradka

Replace imperative useCallback + useEffect secret reads with TQ useQuery
hooks for both credential presence check and credentials list.

Fixes:
- Empty catch on secretsReadPost permanently sets hasStoredCredential=false
  on transient daemon errors (same class as WEB-2H)
- No retry on either daemon call — transient 503/502 errors are fatal
- No Sentry reporting on credential read failures

Changes:
- credentialPresenceQuery: TQ useQuery with shouldRetryDaemonError retry,
  isOrgReady gate, and bestEffort Sentry reporting
- credentialsListQuery: TQ useQuery for available credentials list
- Optimistic cache update after saving a key (queryClient.setQueryData)
- Remove loadCredentialPresence/loadAvailableCredentials callbacks
- Remove hasStoredCredential/isLoadingCredential/availableCredentials
  useState — derived from TQ query data instead
- Remove imperative load calls from useEffect and onChange handlers —
  TQ auto-refetches when query keys change (credential ref, provider)

Closes LUM-2205

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@linear

linear Bot commented Jun 3, 2026

Copy link
Copy Markdown

LUM-2205

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@vex-assistant-bot vex-assistant-bot 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.

APPROVE

Value: Closes the last instance of the imperative-daemon-read pattern in apps/web/src/domains/settings/ai/. Provider editor modal now matches the canonical TanStack Query shape established in PR #33165 (WEB-2H / LUM-2199) — same shouldRetryDaemonError predicate, same useIsOrgReady() gate, same bestEffort: true Sentry reporting. Transient daemon errors (503 boot, 502 proxy, 401 auth-race, 400-org-header hydration) no longer get latched into permanent hasStoredCredential = false / empty availableCredentials state.

What this does: Two useQuery conversions in provider-editor-modal.tsx. Imperative loadCredentialPresence + loadAvailableCredentials useCallbacks deleted; the matching useStates removed; void load*(v) calls in dropdown handlers removed (TQ auto-refetches when parsedCredRef query-key segments change). Save handler keeps its explicit try/catch with user-facing error string + optimistic setQueryData(credentialPresenceKey, true) + invalidateQueries(credentialsListKey). Single file, single commit, +112/-58.

Pattern verification vs LUM-2199 canonical
  • credentialPresenceQuery (L211–230): key [PROVIDER_CREDENTIAL_PRESENCE_QK, assistantId, parsedCredRef.service, parsedCredRef.field], enabled: !!assistantId && needsCredentialCheck && isOrgReady, retry: shouldRetryDaemonError, staleTime: 30_000. Throws ApiError(response.status, extractErrorMessage(...)) on non-OK so the retry predicate sees a typed status. ✓
  • credentialsListQuery (L253–272): key [PROVIDER_CREDENTIALS_LIST_QK, assistantId], enabled: !!assistantId && needsCredentialsList && isOrgReady, retry: shouldRetryDaemonError, staleTime: 30_000. Parses through parseCredentialEntries(data!.secrets ?? data!.accounts ?? []). ✓
  • Sentry reporting (L232–239, L274–281): both wrap captureError(query.error, { context: "settings-provider-editor-…", bestEffort: true }) in a useEffect keyed on query.error. ✓ Matches the #33165 pattern — transient errors silently drop after retry-exhaust, non-transient still ship.
  • Derived state (L241–242, L282): hasStoredCredential = credentialPresenceQuery.data ?? false; isLoadingCredential = credentialPresenceQuery.isLoading && needsCredentialCheck; availableCredentials = credentialsListQuery.data ?? []. No useState shadows. ✓
  • Optimistic save (L407–409): queryClient.setQueryData(credentialPresenceKey, true) + void queryClient.invalidateQueries({ queryKey: credentialsListKey }) after a successful secretsPost. ✓
Anti-pattern grep on diff
  • Empty .catch(() => {}): 0 hits in added lines. The save handler's catch { setError("Failed to save API key. Please try again."); return; } is a user-facing error string, not silent.
  • Runtime-boundary as casts: 0 new. Pre-existing as ServiceMode on localStorage read is untouched (parity with sibling cards).
  • @ts-ignore / @ts-nocheck: 0.
  • eslint-disable react-hooks/*: 0.
  • || 0 fallback: 0.
  • Raw Sentry.captureException: 0 — uses captureError wrapper everywhere.
  • Dead-state cleanup: loadCredentialPresence and loadAvailableCredentials callbacks have 0 hits at HEAD; matching useStates for hasStoredCredential / availableCredentials / isLoadingCredential are gone (now derived). void load*(v) dropdown handlers removed. Clean migration.
  • data!.found / data!.secrets: both guarded by if (!response.ok) throw immediately prior. Same pattern as #33165 and the rest of the daemon SDK call sites.

Merge gate:

  • Vex APPROVED at HEAD b35860c5
  • Codex 👍 on PR description at HEAD b35860c5 (01:16:52Z, ~2.5 min after PR open) ✓
  • CI 7/7 green at HEAD ✓
  • No outstanding REQUEST_CHANGES ✓

LUM-2199 follow-up confirmed closed — the "(separate PR pending)" note on #33165 lands here. Merging.

@vex-assistant-bot vex-assistant-bot Bot merged commit ccdd46f into main Jun 3, 2026
7 checks passed
@vex-assistant-bot vex-assistant-bot Bot deleted the devin/1780448993-provider-editor-tq-credential branch June 3, 2026 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant