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
22 changes: 18 additions & 4 deletions client/src/components/ProviderModelSelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@
* @param {function} props.onModelChange - Called with model string
* @param {string} [props.label] - Label text (default: "Provider")
* @param {boolean} [props.disabled] - Disable both selectors
* @param {boolean} [props.loading] - The caller's provider list hasn't settled
* yet. An empty `providers` is ambiguous — "still fetching" and "none
* configured" both render a picker whose only choice is the
* `emptyProviderOption` ("Default (active provider)", "Inherit (…)"), which
* reads as a broken control rather than a slow one. Pass `true` while the
* fetch is in flight to disable the selects and say so instead. This is the
* same settle-gate the `annotateToolUse` scan below uses on its own fetch,
* applied to the list the caller owns.
* @param {boolean} [props.modelDisabled] - Disable only the model selector (e.g.
* when the selected provider has no models). Composes with `disabled`.
* @param {boolean} [props.compact] - Hide labels for inline/toolbar use
Expand Down Expand Up @@ -95,6 +103,7 @@ export default function ProviderModelSelector({
onModelChange,
label = 'Provider',
disabled = false,
loading = false,
modelDisabled = false,
compact = false,
emptyProviderOption,
Expand Down Expand Up @@ -200,12 +209,17 @@ export default function ProviderModelSelector({
id={providerSelectId}
value={selectedProviderId}
onChange={(e) => onProviderChange(e.target.value)}
disabled={disabled}
disabled={disabled || loading}
title={compact ? label : undefined}
aria-label={compact ? label : undefined}
className={SELECT_CLASS}
>
{emptyProviderOption != null && <option value="">{emptyProviderOption}</option>}
{/* Rendered even when the caller forces a selection: mid-fetch there
is nothing else to offer, and a genuinely empty select reads as the
same broken control. */}
{loading
? <option value="">Loading providers…</option>
: emptyProviderOption != null && <option value="">{emptyProviderOption}</option>}
{visibleProviders.map((p) => {
const hardwareUnavailable = !isProviderHardwareCompatible(p);
const policyDisallowed = Boolean(providerAllowed && !providerAllowed(p));
Expand All @@ -227,7 +241,7 @@ export default function ProviderModelSelector({
id={modelSelectId}
value={selectedModel}
onChange={(e) => handleModelChange(e.target.value)}
disabled={disabled || modelDisabled}
disabled={disabled || modelDisabled || loading}
title={compact ? 'Model' : undefined}
aria-label={compact ? 'Model' : undefined}
className={SELECT_CLASS}
Expand Down Expand Up @@ -271,7 +285,7 @@ export default function ProviderModelSelector({
model={effectiveModel}
value={effort || ''}
onChange={onEffortChange}
disabled={disabled}
disabled={disabled || loading}
optionFilter={effortAllowed}
className={SELECT_CLASS}
/>
Expand Down
29 changes: 29 additions & 0 deletions client/src/components/ProviderModelSelector.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,32 @@ describe('ProviderModelSelector', () => {
});
});
});

describe('ProviderModelSelector — provider list still loading', () => {
it('names the in-flight fetch instead of offering the empty sentinel as the only choice', () => {
// Mid-fetch `providers` is [], so "Default (active provider)" would be the
// select's only option — a slow control that reads as a broken one.
renderSelector({ providers: [], selectedProviderId: '', emptyProviderOption: 'Default (active provider)', loading: true });
const provider = screen.getByLabelText('Provider');
expect([...provider.options].map((o) => o.textContent)).toEqual(['Loading providers…']);
expect(provider.disabled).toBe(true);
});

it('says so even when the caller forces a selection, since there is nothing else to show', () => {
renderSelector({ providers: [], selectedProviderId: '', loading: true });
expect(screen.getByRole('option', { name: 'Loading providers…' })).toBeTruthy();
});

it('disables the model select too, so a pin cannot be retargeted against a list that has not arrived', () => {
renderSelector({ loading: true });
expect(screen.getByLabelText('Model').disabled).toBe(true);
});

it('restores the caller\'s own sentinel once the list settles', () => {
renderSelector({ selectedProviderId: '', emptyProviderOption: 'Default (active provider)' });
const provider = screen.getByLabelText('Provider');
expect(screen.getByRole('option', { name: 'Default (active provider)' })).toBeTruthy();
expect(screen.queryByRole('option', { name: 'Loading providers…' })).toBeNull();
expect(provider.disabled).toBe(false);
});
});
3 changes: 3 additions & 0 deletions client/src/components/cos/AppProviderPin.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { providerPinPatch } from './constants';
* @param {string} props.label Provider-select label (also its aria-label when compact).
* @param {string} [props.inheritLabel] What a blank pin resolves to, e.g. `Inherit (claude-code)`.
* @param {boolean} [props.disabled]
* @param {boolean} [props.loading] Provider list not settled yet — see ProviderModelSelector.
* @param {boolean} [props.compact] Hide labels for inline/table use.
* @param {'row'|'stacked'} [props.layout]
*/
Expand All @@ -38,6 +39,7 @@ export default function AppProviderPin({
label,
inheritLabel = 'Use default provider',
disabled = false,
loading = false,
compact = false,
layout = 'row'
}) {
Expand Down Expand Up @@ -67,6 +69,7 @@ export default function AppProviderPin({
emptyModelOption="Default model"
alwaysShowModel
disabled={disabled}
loading={loading}
compact={compact}
layout={layout}
/>
Expand Down
4 changes: 3 additions & 1 deletion client/src/components/cos/tabs/ScheduleTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function mergeOnDemandRequest(schedule, request) {
// passed down — same convention as TasksTab/AgentsTab — so this tab's provider/
// model pickers stay live without standing up a second independent poll of the
// same data.
export default function ScheduleTab({ apps, providers, activeProviderId }) {
export default function ScheduleTab({ apps, providers, providersLoaded, activeProviderId }) {
const [searchParams, setSearchParams] = useSearchParams();
const [schedule, setSchedule] = useState(null);
const [loading, setLoading] = useState(true);
Expand Down Expand Up @@ -185,6 +185,7 @@ export default function ScheduleTab({ apps, providers, activeProviderId }) {
tasks={tasks}
apps={apps}
providers={providers}
providersLoaded={providersLoaded}
activeProviderId={activeProviderId}
onTrigger={handleTriggerAppImprovement}
onUpdate={handleUpdateTask}
Expand All @@ -208,6 +209,7 @@ export default function ScheduleTab({ apps, providers, activeProviderId }) {
onUpdate={handleUpdateTask}
onTrigger={handleTriggerAppImprovement}
providers={providers}
providersLoaded={providersLoaded}
activeProviderId={activeProviderId}
apps={apps}
onUpdateOverride={handleUpdateOverride}
Expand Down
3 changes: 2 additions & 1 deletion client/src/components/cos/tabs/schedule/AppOverrideRow.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import ToggleSwitch from '../../../ToggleSwitch';
import useFieldDraft from '../../../../hooks/useFieldDraft';
import { INTERVAL_LABELS, setMetadataOverride } from './scheduleConstants';

const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalIntervalType, globalTaskMetadata, managedAgentOptions, fileIssuesCapable, defaultFileIssues, doWorkRequiresWorktree, inheritedProviderText, providers, override, onUpdate }) {
const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalIntervalType, globalTaskMetadata, managedAgentOptions, fileIssuesCapable, defaultFileIssues, doWorkRequiresWorktree, inheritedProviderText, providers, providersLoaded = true, override, onUpdate }) {
const [updating, setUpdating] = useState(false);
const [cronEditing, setCronEditing] = useState(false);
const isEnabled = override?.enabled === true;
Expand Down Expand Up @@ -202,6 +202,7 @@ const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalInter
<div className="w-full sm:w-auto sm:min-w-[240px] sm:max-w-[360px] sm:flex-1">
<AppProviderPin
providers={providers}
loading={!providersLoaded}
providerId={override?.providerId}
model={override?.model}
onChange={handlePinChange}
Expand Down
4 changes: 2 additions & 2 deletions client/src/components/cos/tabs/schedule/AppTaskCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import TaskModelQuickControls from './TaskModelQuickControls';
// One scheduled task rendered as a status-rich card. Browsing plus the common
// "retarget the model and run it" loop happen here; the rest of the
// configuration lives in the slide-over drawer (opened via Configure).
export default function AppTaskCard({ taskType, config, apps, onTrigger, onConfigure, onUpdate, providers, activeProviderId, improvementDisabled }) {
export default function AppTaskCard({ taskType, config, apps, onTrigger, onConfigure, onUpdate, providers, providersLoaded = true, activeProviderId, improvementDisabled }) {
// Owned here, not in the controls, so Run can gate on the same `saving` flag —
// it reads the server-side config, so a run fired mid-write uses the old pins.
const pins = useTaskModelPins({ taskType, config, providers, activeProviderId, onUpdate });
Expand Down Expand Up @@ -80,7 +80,7 @@ export default function AppTaskCard({ taskType, config, apps, onTrigger, onConfi
Provider/model is set per stage ({stageCount}) — configure
</button>
) : (
<TaskModelQuickControls pins={pins} providers={providers} />
<TaskModelQuickControls pins={pins} providers={providers} loading={!providersLoaded} />
))}

{/* Footer actions */}
Expand Down
9 changes: 9 additions & 0 deletions client/src/components/cos/tabs/schedule/AppTaskCard.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,15 @@ describe('AppTaskCard', () => {
expect(screen.queryByLabelText('Thinking effort')).toBeNull();
});

it('says the provider list is still loading instead of offering a lone bare Default', () => {
// Proves the card actually threads the flag; the label/disable rule itself
// is ProviderModelSelector's (see its own suite).
renderCardWithPins({}, { providers: [], providersLoaded: false });
const provider = screen.getByLabelText('Provider');
expect(within(provider).getByRole('option', { name: 'Loading providers…' })).toBeTruthy();
expect(provider.disabled).toBe(true);
});

it('hides a disabled provider from the picker unless the task is pinned to it', () => {
const withDisabled = [...providers, { id: 'retired', name: 'Retired CLI', enabled: false }];
renderCardWithPins({}, { providers: withDisabled });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Search, X } from 'lucide-react';
import AppTaskCard from './AppTaskCard';
import { TASK_FILTERS, DEFAULT_FILTER_ID, taskSortKey } from './scheduleConstants';

export default function AppTaskTypeSection({ tasks, apps, providers, activeProviderId, onTrigger, onUpdate, onSelectTask, improvementDisabled, filter, onFilterChange }) {
export default function AppTaskTypeSection({ tasks, apps, providers, providersLoaded, activeProviderId, onTrigger, onUpdate, onSelectTask, improvementDisabled, filter, onFilterChange }) {
const [search, setSearch] = useState('');
const taskEntries = Object.entries(tasks || {});

Expand Down Expand Up @@ -90,6 +90,7 @@ export default function AppTaskTypeSection({ tasks, apps, providers, activeProvi
config={config}
apps={apps}
providers={providers}
providersLoaded={providersLoaded}
activeProviderId={activeProviderId}
onTrigger={onTrigger}
onUpdate={onUpdate}
Expand Down
10 changes: 6 additions & 4 deletions client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const REVIEW_CONFIG_KEYS = [
'reviewerApplies',
];

export default function GlobalConfigControls({ taskType, config, onUpdate, onTrigger, category: _category, providers, activeProviderId, apps, updating, setUpdating, allTaskTypes, improvementDisabled, dataInputCatalog }) {
export default function GlobalConfigControls({ taskType, config, onUpdate, onTrigger, category: _category, providers, providersLoaded = true, activeProviderId, apps, updating, setUpdating, allTaskTypes, improvementDisabled, dataInputCatalog }) {
const reviewDefaults = useCodeReviewDefaults();
// Resolved model lists for the reviewer table's Model column (the picker itself
// never fetches — see its `modelOptions` prop).
Expand Down Expand Up @@ -389,10 +389,12 @@ export default function GlobalConfigControls({ taskType, config, onUpdate, onTri
<select
value={selectedProviderId}
onChange={(e) => handleProviderChange(e.target.value)}
disabled={updating}
disabled={updating || !providersLoaded}
className="w-full bg-port-card border border-port-border rounded px-3 py-2 text-white text-sm"
>
<option value="">{defaultProviderLabel}</option>
{/* Mid-fetch the list is empty, so "Default (active provider)" would
be this select's only option — a slow control that reads broken. */}
<option value="">{providersLoaded ? defaultProviderLabel : 'Loading providers…'}</option>
{providers?.map(provider => (
<option key={provider.id} value={provider.id}>{provider.name}</option>
))}
Expand All @@ -404,7 +406,7 @@ export default function GlobalConfigControls({ taskType, config, onUpdate, onTri
<select
value={selectedModel}
onChange={(e) => handleModelChange(e.target.value)}
disabled={updating}
disabled={updating || !providersLoaded}
className="w-full bg-port-card border border-port-border rounded px-3 py-2 text-white text-sm"
>
{/* `availableModels` already carries a pin the provider no longer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState } from 'react';
import { providerModelLabel } from '../../../../utils/providers';
import AppOverrideRow from './AppOverrideRow';

export default function PerAppOverrideList({ taskType, config, apps, providers, onUpdateOverride, onBulkToggleOverride }) {
export default function PerAppOverrideList({ taskType, config, apps, providers, providersLoaded = true, onUpdateOverride, onBulkToggleOverride }) {
const [bulkUpdating, setBulkUpdating] = useState(false);
const activeApps = apps?.filter(app => !app.archived) || [];
const appOverrides = config.appOverrides || {};
Expand Down Expand Up @@ -58,6 +58,7 @@ export default function PerAppOverrideList({ taskType, config, apps, providers,
doWorkRequiresWorktree={config.doWorkRequiresWorktree}
inheritedProviderText={inheritedProviderText}
providers={providers}
providersLoaded={providersLoaded}
override={appOverrides[app.id]}
onUpdate={onUpdateOverride}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const actionsStageNote = (eligibleProviders) => {
].join(' ');
};

export default function PipelineStageConfig({ taskType, config, providers, onUpdate, updating, setUpdating }) {
export default function PipelineStageConfig({ taskType, config, providers, providersLoaded = true, onUpdate, updating, setUpdating }) {
const stages = pipelineStages(config);
const needsSecurityModelPolicy = taskType === 'pr-reviewer';
const { ollama, lmstudio, capabilitiesByBackend, loading: localModelsLoading } = useLocalModels({
Expand Down Expand Up @@ -203,6 +203,7 @@ export default function PipelineStageConfig({ taskType, config, providers, onUpd
)}
{!isSecurityStage && (
<ProviderModelSelector
loading={!providersLoaded}
providers={providers || []}
selectedProviderId={stageProviderId}
selectedModel={stageModel}
Expand Down
4 changes: 4 additions & 0 deletions client/src/components/cos/tabs/schedule/TaskConfigDrawer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default function TaskConfigDrawer({
onUpdate,
onTrigger,
providers,
providersLoaded,
activeProviderId,
apps,
onUpdateOverride,
Expand Down Expand Up @@ -75,6 +76,7 @@ export default function TaskConfigDrawer({
taskType={taskType}
config={config}
providers={providers}
providersLoaded={providersLoaded}
onUpdate={onUpdate}
updating={updating}
setUpdating={setUpdating}
Expand All @@ -89,6 +91,7 @@ export default function TaskConfigDrawer({
onTrigger={onTrigger}
category="appImprovement"
providers={providers}
providersLoaded={providersLoaded}
activeProviderId={activeProviderId}
apps={apps}
updating={updating}
Expand All @@ -105,6 +108,7 @@ export default function TaskConfigDrawer({
config={config}
apps={apps}
providers={providers}
providersLoaded={providersLoaded}
onUpdateOverride={onUpdateOverride}
onBulkToggleOverride={onBulkToggleOverride}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import ProviderModelSelector from '../../../ProviderModelSelector';
//
// `highlightToolUse` is on because a scheduled task IS an agent run: a task
// pinned to a local model that can't call tools narrates instead of working.
export default function TaskModelQuickControls({ pins, providers, disabled = false }) {
export default function TaskModelQuickControls({ pins, providers, loading = false, disabled = false }) {
const {
providerId, model, effort, effectiveProviderId, defaultProviderLabel,
availableModels, saving, changeProvider, changeModel, changeEffort,
Expand All @@ -30,6 +30,7 @@ export default function TaskModelQuickControls({ pins, providers, disabled = fal
alwaysShowModel
compact
highlightToolUse
loading={loading}
disabled={disabled || saving}
/>
</div>
Expand Down
Loading