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
26 changes: 20 additions & 6 deletions desktop/src/features/agents/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,34 @@ with a TypeScript lookup table or an id comparison in a component.
sole onboarding surface that chooses and persists `preferred_runtime`.
`onboarding-agent-defaults.spec.ts` is the acceptance gate for anything
touching this flow or the shared renderer.
8. **Omit the Model control only after a confirmed successful empty
discovery on an optional-model harness.** When the field model marks model
as `acpNative` (Claude Code / Codex), `shouldRenderModelControl` hides the
picker while discovery is in flight and after IPC resolves with no usable
options (`modelDiscoverySuccessfulEmpty` / `isSuccessfulEmptyDiscovery`).
A thrown or unavailable discovery keeps the control so #2246 failure UI can
render, and must not heal/clear persisted model or effort. Full disclosure
still shows the control when Custom model is available. Required-model
harnesses always keep the field. Gate: `defaults hides model when optional
harness has empty discovery` (and the failed-discovery counterpart) in
`onboarding-agent-defaults.spec.ts`.

## The tests that enforce this

- `lib/agentConfigCore.test.mjs` — field model per harness × scope, clearing
policy. Update when the capability model changes.
- `ui/agentConfigFieldsContract.test.mjs` — canonical behaviors + disclosure
presets + `shouldShowModelStatusMessage` status-bypass rule. If this fails,
you probably reintroduced a per-surface flag or broke the status-bypass.
presets + `shouldShowModelStatusMessage` status-bypass +
`shouldRenderModelControl` (successful-empty omit vs failure keep). If this
fails, you probably reintroduced a per-surface flag or conflated empty with
failed discovery.
- `ui/usePersonaModelDiscovery.test.mjs` — `synthesizeEmptyDiscoveryStatus`,
`isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`. If the
"reopen to retry" copy becomes inert again, these tests will catch it.
`isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`,
`isSuccessfulEmptyDiscovery`. If the "reopen to retry" copy becomes inert
again, these tests will catch it.
- `desktop/tests/e2e/onboarding-agent-defaults.spec.ts` — onboarding behavior
acceptance coverage for readiness, failure states, defaults, navigation, and
persistence races.
acceptance coverage for readiness, failure states, defaults, navigation,
successful-empty vs failed optional-model discovery, and persistence races.
- Rust: `runtime_metadata_env_vars` tests pin spawn-time key application.

## Keep this file true
Expand Down
166 changes: 113 additions & 53 deletions desktop/src/features/agents/ui/AgentConfigFields.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,41 @@ export function shouldShowModelStatusMessage(
return showDescriptions || status !== null;
}

/**
* Whether the Model control should render given discovery state.
*
* Optional-model harnesses (Claude Code / Codex, `acpNative`) omit the control
* while discovery is in flight and after a **confirmed successful empty**
* catalog (IPC resolved, no usable options) — there is nothing useful to pick.
* Discovery failures / unavailable runtimes keep the control so #2246 failure
* UI can render. Full disclosure still shows the control when Custom model is
* available. Required-model harnesses always render the control.
*/
export function shouldRenderModelControl({
discoveredModelOptions,
modelDiscoveryLoading,
modelDiscoverySuccessfulEmpty,
modelIsOptional,
showCustomModelOption,
}: {
discoveredModelOptions: readonly { id: string }[] | null;
modelDiscoveryLoading: boolean;
/** True only when discovery IPC resolved with a response that yielded no options. */
modelDiscoverySuccessfulEmpty: boolean;
modelIsOptional: boolean;
showCustomModelOption: boolean;
}): boolean {
if (!modelIsOptional) return true;
if (modelDiscoveryLoading) return false;
const hasExplicitModel = (discoveredModelOptions ?? []).some(
(option) => option.id.trim().length > 0,
);
if (hasExplicitModel) return true;
if (showCustomModelOption) return true;
// Omit only on confirmed successful empty — not on failure/unavailable.
return !modelDiscoverySuccessfulEmpty;
}

export type AgentConfigFieldsProps = {
bakedEnv: BakedEnvEntry[];
selectedRuntime: AcpRuntimeCatalogEntry | undefined;
Expand Down Expand Up @@ -277,6 +312,7 @@ export function AgentConfigFields({
discoveredModelOptions,
modelDiscoveryLoading,
modelDiscoveryStatus,
modelDiscoverySuccessfulEmpty,
} = usePersonaModelDiscovery({
envVars: config.env_vars,
isCustomProviderEditing: isCustomProvider,
Expand All @@ -285,6 +321,18 @@ export function AgentConfigFields({
provider: providerForDiscovery,
selectedRuntime,
});
const modelControlVisible = shouldRenderModelControl({
discoveredModelOptions: dependentFieldsDisabled
? null
: discoveredModelOptions,
modelDiscoveryLoading: dependentFieldsDisabled
? false
: modelDiscoveryLoading,
modelDiscoverySuccessfulEmpty:
!dependentFieldsDisabled && modelDiscoverySuccessfulEmpty,
modelIsOptional,
showCustomModelOption,
});

// Mount-time healing policy: onboarding page 4 edits the root config during
// first-run (no higher layers to inherit from), so acting on open is safe
Expand Down Expand Up @@ -353,16 +401,23 @@ export function AgentConfigFields({
// the old harness. In onboarding, heal that stale value as soon as the new
// harness catalog proves it is unsupported; otherwise a Codex id like
// `gpt-5.5[low]` appears as a Claude Code custom model.
// Also clear when the Model control is omitted after a confirmed successful
// empty catalog — never while discovery failed/unavailable (transient
// failures must not erase saved model/effort).
React.useEffect(() => {
if (!healOnMount) return;
const currentModel = (config.model ?? "").trim();
if (currentModel.length === 0) return;
if (modelDiscoveryLoading || discoveredModelOptions === null) return;
if (
discoveredModelOptions.some((option) => option.id.trim() === currentModel)
) {
return;
}
if (modelDiscoveryLoading) return;

const catalogMiss =
discoveredModelOptions !== null &&
!discoveredModelOptions.some(
(option) => option.id.trim() === currentModel,
);
const omittedAfterSuccessfulEmpty =
modelIsOptional && !modelControlVisible && modelDiscoverySuccessfulEmpty;
if (!catalogMiss && !omittedAfterSuccessfulEmpty) return;

const nextEnvVars = { ...config.env_vars };
if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey];
Expand All @@ -371,7 +426,10 @@ export function AgentConfigFields({
}, [
config,
discoveredModelOptions,
modelControlVisible,
modelDiscoveryLoading,
modelDiscoverySuccessfulEmpty,
modelIsOptional,
onConfigChange,
onCustomModelEditingChange,
healOnMount,
Expand Down Expand Up @@ -659,53 +717,55 @@ export function AgentConfigFields({
</div>
) : null}

{/* Model field */}
<div className={showDescriptions ? fieldClassName : undefined}>
<AgentModelField
allowDefaultModel={fallbackModel !== null}
defaultModelLabel={
fallbackModel ? `Default model (${fallbackModel})` : undefined
}
disableSelectDuringDiscovery={disableModelSelectDuringDiscovery}
disabled={dependentFieldsDisabled}
discoveredModelOptions={
dependentFieldsDisabled ? null : discoveredModelOptions
}
globalModel={fallbackModel ?? undefined}
id="global-agent-model"
isCustomModelEditing={isCustomModelEditing}
isRequired={
showRequiredIndicators &&
!modelIsOptional &&
fallbackModel === null &&
!dependentFieldsDisabled
}
keepSelectedModelValueLabel
model={dependentFieldsDisabled ? "" : (config.model ?? "")}
modelDiscoveryLoading={
dependentFieldsDisabled ? false : modelDiscoveryLoading
}
modelDiscoveryStatus={
dependentFieldsDisabled ? null : modelDiscoveryStatus
}
onIsCustomModelEditingChange={onCustomModelEditingChange}
onModelChange={handleModelChange}
placeholderClassName={placeholderClassName}
placeholder="Select a model"
provider={providerForDiscovery}
fieldClassName={unstyled ? fieldClassName : undefined}
labelClassName={fieldLabelClassName}
selectClassName={selectClassName}
showCustomModelOption={showCustomModelOption}
showStatusMessage={shouldShowModelStatusMessage(
showDescriptions,
modelDiscoveryStatus,
)}
testId="global-agent-model"
useCustomSelect={useCustomSelect}
useChevronIcon={useChevronSelectIcon}
/>
</div>
{/* Model field — omitted only after confirmed successful empty discovery */}
{modelControlVisible ? (
<div className={showDescriptions ? fieldClassName : undefined}>
<AgentModelField
allowDefaultModel={fallbackModel !== null}
defaultModelLabel={
fallbackModel ? `Default model (${fallbackModel})` : undefined
}
disableSelectDuringDiscovery={disableModelSelectDuringDiscovery}
disabled={dependentFieldsDisabled}
discoveredModelOptions={
dependentFieldsDisabled ? null : discoveredModelOptions
}
globalModel={fallbackModel ?? undefined}
id="global-agent-model"
isCustomModelEditing={isCustomModelEditing}
isRequired={
showRequiredIndicators &&
!modelIsOptional &&
fallbackModel === null &&
!dependentFieldsDisabled
}
keepSelectedModelValueLabel
model={dependentFieldsDisabled ? "" : (config.model ?? "")}
modelDiscoveryLoading={
dependentFieldsDisabled ? false : modelDiscoveryLoading
}
modelDiscoveryStatus={
dependentFieldsDisabled ? null : modelDiscoveryStatus
}
onIsCustomModelEditingChange={onCustomModelEditingChange}
onModelChange={handleModelChange}
placeholderClassName={placeholderClassName}
placeholder="Select a model"
provider={providerForDiscovery}
fieldClassName={unstyled ? fieldClassName : undefined}
labelClassName={fieldLabelClassName}
selectClassName={selectClassName}
showCustomModelOption={showCustomModelOption}
showStatusMessage={shouldShowModelStatusMessage(
showDescriptions,
dependentFieldsDisabled ? null : modelDiscoveryStatus,
)}
testId="global-agent-model"
useCustomSelect={useCustomSelect}
useChevronIcon={useChevronSelectIcon}
/>
</div>
) : null}

{/* Thinking / Effort */}
{effortFieldVisible ? (
Expand Down
15 changes: 13 additions & 2 deletions desktop/src/features/agents/ui/agentConfigControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export function AgentDropdownSelect({
ariaRequired,
className,
disabled = false,
emptyOptionsLabel = "No options available",
id,
onValueChange,
options,
Expand All @@ -77,6 +78,8 @@ export function AgentDropdownSelect({
ariaRequired?: boolean;
className?: string;
disabled?: boolean;
/** Shown when the option list is empty (not a search filter miss). */
emptyOptionsLabel?: string;
id: string;
onValueChange: (value: string) => void;
options: readonly AgentDropdownOption[];
Expand Down Expand Up @@ -184,8 +187,15 @@ export function AgentDropdownSelect({
/>
</div>
) : null}
{showSearch && filteredOptions.length === 0 ? (
<p className="px-3 py-2 text-sm text-foreground/55">No matches</p>
{filteredOptions.length === 0 ? (
<p
className="px-3 py-2 text-sm text-foreground/55"
data-testid={testId ? `${testId}-empty` : undefined}
>
{showSearch && query.trim().length > 0
? "No matches"
: emptyOptionsLabel}
</p>
) : null}
{filteredOptions.map((option) => {
const selected = option.value === value;
Expand Down Expand Up @@ -470,6 +480,7 @@ export function AgentModelField({
ariaRequired={isRequired}
className={selectClassName}
disabled={selectDisabled}
emptyOptionsLabel="Couldn't load models"
id={id}
onValueChange={handleModelSelectChange}
options={modelOptions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import test from "node:test";
import {
CANONICAL_CONFIG_BEHAVIORS,
resolveDisclosure,
shouldRenderModelControl,
shouldShowModelStatusMessage,
} from "./AgentConfigFields.tsx";

Expand Down Expand Up @@ -83,3 +84,94 @@ test("shouldShowModelStatusMessage_onboardingPreset_warningStatus_showsMessage",
};
assert.equal(shouldShowModelStatusMessage(showDescriptions, warning), true);
});

// ── shouldRenderModelControl ──────────────────────────────────────────────────
// Optional-model harnesses omit the control only after a confirmed successful
// empty catalog. Failures keep the control so #2246 status UI can render.

test("optional model control hides while loading", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: null,
modelDiscoveryLoading: true,
modelDiscoverySuccessfulEmpty: false,
modelIsOptional: true,
showCustomModelOption: false,
}),
false,
"optional + loading must hide the control",
);
});

test("optional model control hides after successful empty discovery", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: null,
modelDiscoveryLoading: false,
modelDiscoverySuccessfulEmpty: true,
modelIsOptional: true,
showCustomModelOption: false,
}),
false,
"optional + successful empty + no custom must hide the control",
);
});

test("optional model control stays visible on discovery failure", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: null,
modelDiscoveryLoading: false,
modelDiscoverySuccessfulEmpty: false,
modelIsOptional: true,
showCustomModelOption: false,
}),
true,
"optional + failed/unavailable discovery must keep the control",
);
});

test("optional model control shows when explicit models are available", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: [
{ id: "", label: "Default model" },
{ id: "claude-sonnet-4", label: "Claude Sonnet 4" },
],
modelDiscoveryLoading: false,
modelDiscoverySuccessfulEmpty: false,
modelIsOptional: true,
showCustomModelOption: false,
}),
true,
"explicit models must show the control",
);
});

test("full disclosure keeps Custom escape hatch when discovery is empty", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: null,
modelDiscoveryLoading: false,
modelDiscoverySuccessfulEmpty: true,
modelIsOptional: true,
showCustomModelOption: true,
}),
true,
"full disclosure keeps Custom escape hatch when discovery is empty",
);
});

test("required-model harnesses always keep the control", () => {
assert.equal(
shouldRenderModelControl({
discoveredModelOptions: null,
modelDiscoveryLoading: false,
modelDiscoverySuccessfulEmpty: true,
modelIsOptional: false,
showCustomModelOption: false,
}),
true,
"required-model harnesses always keep the control",
);
});
Loading
Loading