Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions apps/web/src/domains/settings/ai/image-generation-card.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Loader2 } from "lucide-react";
import { useCallback, useEffect, useMemo, useState } from "react";
import { useCallback, useMemo, useState } from "react";

import { useQueryClient } from "@tanstack/react-query";

Expand All @@ -25,6 +25,7 @@ import {
import { reconcileFromDaemonConfig } from "@/domains/settings/ai/ai-utils";
import { ServiceCard, SaveButton, ResetButton } from "@/domains/settings/ai/ai-shared-ui";
import { useAssistantId, useDaemonConfigQuery, useDaemonConfigMutation, useProvisionProviderKey } from "@/domains/settings/ai/use-daemon-config";
import { useDraftOverride } from "@/domains/settings/ai/use-draft-override";
import { modelImagegenPut } from "@/generated/daemon/sdk.gen";

export function ImageGenerationCard() {
Expand All @@ -41,17 +42,7 @@ export function ImageGenerationCard() {
return reconciled.imageGenMode ?? (getLocalSetting(LS_IMAGE_GEN_MODE, "your-own") as ServiceMode);
}, [daemonConfig]);

// Draft override — null means the user hasn't changed the value yet.
const [draftImageGenMode, setDraftImageGenMode] = useState<ServiceMode | null>(null);

// Auto-clear draft once the server value catches up after save.
useEffect(() => {
if (draftImageGenMode !== null && serverImageGenMode === draftImageGenMode) {
setDraftImageGenMode(null);
}
}, [serverImageGenMode, draftImageGenMode]);

const imageGenMode = draftImageGenMode ?? serverImageGenMode;
const [imageGenMode, setDraftImageGenMode] = useDraftOverride(serverImageGenMode);

const [imageGenModel, setImageGenModel] = useState(() =>
getLocalSetting(LS_IMAGE_GEN_MODEL, "gemini-3.1-flash-image-preview"),
Expand Down
21 changes: 2 additions & 19 deletions apps/web/src/domains/settings/ai/language-model-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useAssistantFeatureFlagStore } from "@/stores/assistant-feature-flag-st

import { ByoServiceCard, SaveButton } from "@/domains/settings/ai/ai-shared-ui";
import { useDaemonConfigQuery, useDaemonConfigMutation } from "@/domains/settings/ai/use-daemon-config";
import { useDraftOverride } from "@/domains/settings/ai/use-draft-override";
import { CallSiteOverridesModal } from "@/domains/settings/ai/call-site-overrides-modal";
import { ManageProfilesModal } from "@/domains/settings/ai/manage-profiles-modal";
import { ManageProvidersModal } from "@/domains/settings/ai/manage-providers-modal";
Expand All @@ -29,22 +30,7 @@ export function LanguageModelCard() {
} = useDaemonConfigQuery();
const configMutation = useDaemonConfigMutation();

// Draft active profile — ephemeral UI state for the unsaved dropdown
// selection. Resets to the server value when the server value changes
// (i.e. after a successful save + cache invalidation).
const [draftActiveProfile, setDraftActiveProfile] = useState<string | null>(null);
const [draftInitialized, setDraftInitialized] = useState(false);

// Derive the active profile to display: when the user hasn't touched
// the dropdown yet, use the server value; otherwise use the draft.
// After a successful save, `draftInitialized` resets so the dropdown
// re-syncs to the persisted value.
const effectiveActiveProfile = useMemo(() => {
if (!draftInitialized && activeProfile !== null) {
return activeProfile;
}
return draftActiveProfile ?? activeProfile;
}, [draftInitialized, draftActiveProfile, activeProfile]);
const [effectiveActiveProfile, setDraftActiveProfile] = useDraftOverride(activeProfile);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve null as a valid profile draft

When the saved activeProfile is non-null, this hook treats null as “no draft”, but the dropdown also uses null to mean “clear the default profile” (val === "" ? null : val). As a result, selecting the blank/default option immediately falls back to the saved profile, isProfileDirty stays false, and the user cannot save activeProfile: null; the previous draftInitialized flag kept those two states distinct.

Useful? React with 👍 / 👎.

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.

False positive — the old code had the same behavior for null.

Old code path (when val === "" triggers setDraftActiveProfile(null) + setDraftInitialized(true)):

draftInitialized = true → return draftActiveProfile ?? activeProfile → null ?? activeProfile → activeProfile

New code path (same trigger):

draft = null → effective = draft ?? serverValue → null ?? activeProfile → activeProfile

Both fall back to activeProfile identically. The draftInitialized flag only controlled the initial render path (!draftInitialized && activeProfile !== null), not the null-draft case — once initialized, draftActiveProfile ?? activeProfile already collapses null into activeProfile, exactly like useDraftOverride does.

Also, the dropdown options are all non-empty profile name strings — val === "" can't be triggered from the options list. The val === "" ? null : val guard is defensive.

Resolved — no behavioral regression.


// Modal toggles — ephemeral UI state, correct as useState
const [manageProfilesOpen, setManageProfilesOpen] = useState(false);
Expand Down Expand Up @@ -73,8 +59,6 @@ export function LanguageModelCard() {
const handleManagedProfileSave = useCallback(async () => {
try {
await configMutation.mutateAsync({ llm: { activeProfile: effectiveActiveProfile } });
setDraftInitialized(false);
setDraftActiveProfile(null);
toast.success("Profile saved.");
} catch (error) {
toast.error("Failed to switch profile. Please try again.");
Expand All @@ -96,7 +80,6 @@ export function LanguageModelCard() {
<Dropdown
value={effectiveActiveProfile ?? ""}
onChange={(val) => {
setDraftInitialized(true);
setDraftActiveProfile(val === "" ? null : val);
}}
placeholder="Select a default profile…"
Expand Down
49 changes: 11 additions & 38 deletions apps/web/src/domains/settings/ai/provider-editor-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
inferenceProviderconnectionsPost,
secretsGet,
secretsPost,
secretsReadPost,
} from "@/generated/daemon/sdk.gen";
import {
ApiError,
Expand All @@ -23,6 +22,7 @@ import {
import { shouldRetryDaemonError } from "@/utils/daemon-errors";
import { captureError } from "@/lib/sentry/capture-error";
import { useIsOrgReady } from "@/hooks/use-is-org-ready";
import { useStoredCredentialPresence } from "@/domains/settings/ai/use-stored-credential-presence";

import { ChatgptOAuthSection } from "@/domains/settings/ai/chatgpt-oauth-section";
import {
Expand All @@ -42,7 +42,6 @@ import { providerSupportsPlatformAuth } from "@/assistant/llm-model-catalog";
// Query keys
// ---------------------------------------------------------------------------

const PROVIDER_CREDENTIAL_PRESENCE_QK = "provider-credential-presence" as const;
const PROVIDER_CREDENTIALS_LIST_QK = "provider-credentials-list" as const;

function parseCredentialRef(credRef: string): { service: string; field: string } | null {
Expand Down Expand Up @@ -199,48 +198,22 @@ export function ProviderEditorContent({
const queryClient = useQueryClient();
const isOrgReady = useIsOrgReady();

// --- Credential presence query (TanStack Query) ---
// --- Credential presence (shared hook) ---
const parsedCredRef = useMemo(() => parseCredentialRef(credential), [credential]);
const needsCredentialCheck = authType === "api_key" && parsedCredRef !== null;

const credentialPresenceKey = useMemo(
() => [PROVIDER_CREDENTIAL_PRESENCE_QK, assistantId, parsedCredRef?.service ?? "", parsedCredRef?.field ?? ""],
[assistantId, parsedCredRef],
);

const credentialPresenceQuery = useQuery({
const {
hasStoredCredential,
isLoading: isLoadingCredential,
queryKey: credentialPresenceKey,
queryFn: async () => {
const { data, error, response } = await secretsReadPost({
path: { assistant_id: assistantId },
body: { type: "credential", name: `${parsedCredRef!.service}:${parsedCredRef!.field}` },
throwOnError: false,
});
assertHasResponse(response, error, "Failed to check stored credential");
if (!response.ok) {
throw new ApiError(
response.status,
extractErrorMessage(error, response, `Failed to check stored credential (HTTP ${response.status})`),
);
}
return data!.found;
},
enabled: !!assistantId && needsCredentialCheck && isOrgReady,
retry: shouldRetryDaemonError,
staleTime: 30_000,
} = useStoredCredentialPresence({
assistantId,
credentialKind: "credential",
credentialName: parsedCredRef ? `${parsedCredRef.service}:${parsedCredRef.field}` : "",
enabled: needsCredentialCheck,
errorContext: "settings-provider-editor-credential-presence",
});

useEffect(() => {
if (!credentialPresenceQuery.error) return;
captureError(credentialPresenceQuery.error, {
context: "settings-provider-editor-credential-presence",
bestEffort: true,
});
}, [credentialPresenceQuery.error]);

const hasStoredCredential = credentialPresenceQuery.data ?? false;
const isLoadingCredential = credentialPresenceQuery.isLoading && needsCredentialCheck;

// --- Available credentials list query (TanStack Query) ---
const needsCredentialsList =
authType === "api_key" || effectiveMode === "create";
Expand Down
30 changes: 30 additions & 0 deletions apps/web/src/domains/settings/ai/use-draft-override.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { useCallback, useEffect, useState } from "react";

/**
* Manages a local draft that overrides a server-derived value.
*
* Returns `[effectiveValue, setDraft]` where `effectiveValue` is the
* draft when set, otherwise the server value. The draft auto-clears
* when the server value converges (e.g. after a save + cache refetch),
* preventing the UI from briefly reverting to stale server state during
* the refetch window.
*
* Pass `undefined` to clear the draft (revert to server value).
* Any `T` value — including `null` — is stored as a valid draft.
*/
export function useDraftOverride<T>(serverValue: T): [T, (draft: T | undefined) => void] {
const [draft, setDraft] = useState<{ value: T } | undefined>(undefined);

useEffect(() => {
if (draft !== undefined && serverValue === draft.value) {
setDraft(undefined);
}
}, [serverValue, draft]);

const effective = draft !== undefined ? draft.value : serverValue;
const updateDraft = useCallback(
(d: T | undefined) => setDraft(d === undefined ? undefined : { value: d }),
[],
);
return [effective, updateDraft];
}
90 changes: 90 additions & 0 deletions apps/web/src/domains/settings/ai/use-stored-credential-presence.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { useEffect, useMemo } from "react";

import { useQuery } from "@tanstack/react-query";

import { secretsReadPost } from "@/generated/daemon/sdk.gen";
import { ApiError, assertHasResponse, extractErrorMessage } from "@/utils/api-errors";
import { shouldRetryDaemonError } from "@/utils/daemon-errors";
import { captureError } from "@/lib/sentry/capture-error";
import { useIsOrgReady } from "@/hooks/use-is-org-ready";

// ---------------------------------------------------------------------------
// Query key
// ---------------------------------------------------------------------------

const STORED_CREDENTIAL_PRESENCE_QK = "stored-credential-presence" as const;

// ---------------------------------------------------------------------------
// Hook
// ---------------------------------------------------------------------------

interface UseStoredCredentialPresenceOptions {
assistantId: string | undefined;
/** Credential kind sent to the daemon (e.g. "api_key", "credential"). */
credentialKind: string;
/** Credential identifier sent to the daemon (e.g. "tavily", "anthropic:api_key"). */
credentialName: string;
/** Extra guard — the query only fires when all conditions are true. */
enabled?: boolean;
/** Sentry context tag for error reporting. */
errorContext: string;
}

/**
* Checks whether a stored credential exists on the daemon.
*
* Wraps `secretsReadPost` in a TanStack Query hook with org-readiness
* gating, retry logic for transient daemon errors, and Sentry reporting
* for persistent failures.
*/
export function useStoredCredentialPresence({
assistantId,
credentialKind,
credentialName,
enabled = true,
errorContext,
}: UseStoredCredentialPresenceOptions) {
const isOrgReady = useIsOrgReady();

const queryKey = useMemo(
() => [STORED_CREDENTIAL_PRESENCE_QK, assistantId ?? "", credentialKind, credentialName] as const,
[assistantId, credentialKind, credentialName],
);

const query = useQuery({
queryKey,
queryFn: async () => {
const { data, error, response } = await secretsReadPost({
path: { assistant_id: assistantId! },
body: { type: credentialKind, name: credentialName },
throwOnError: false,
});
assertHasResponse(response, error, "Failed to check stored credential");
if (!response.ok) {
throw new ApiError(
response.status,
extractErrorMessage(
error,
response,
`Failed to check stored credential (HTTP ${response.status})`,
),
);
}
return data!.found;
},
enabled: !!assistantId && enabled && isOrgReady,
retry: shouldRetryDaemonError,
staleTime: 30_000,
});

useEffect(() => {
if (!query.error) return;
captureError(query.error, { context: errorContext, bestEffort: true });
}, [query.error, errorContext]);

return {
hasStoredCredential: query.data ?? false,
isLoading: query.isLoading,
queryKey,
};
}
Loading