diff --git a/client/src/components/cos/ReviewerPicker.jsx b/client/src/components/cos/ReviewerPicker.jsx
index b3672f02b7..118f10a6d1 100644
--- a/client/src/components/cos/ReviewerPicker.jsx
+++ b/client/src/components/cos/ReviewerPicker.jsx
@@ -65,10 +65,10 @@ const CUSTOM_MODEL_OPTION = '[custom]';
*
* `modelOptions` is the resolved model-picker data, shaped like
* `useReviewerModelOptions()`'s return: `{ optionsByReviewer, defaultModels,
- * freeText, unavailable, loaded }`. Callers keep owning their own
- * `api.getLocalLlmStatus` / `api.getProviders` fetches (that's what the hook is
- * for) — passing nothing degrades every Model cell to a free-text input, which is
- * still fully usable, rather than hiding the column.
+ * freeText, unavailable, providerDisabled, loaded }`. Callers keep owning their
+ * own `api.getLocalLlmStatus` / `api.getProviders` fetches (that's what the hook
+ * is for) — passing nothing degrades every Model cell to a free-text input, which
+ * is still fully usable, rather than hiding the column.
*
* `showRunFlags={false}` hides the stop-mode select and the "reviewer applies
* fixes" checkbox for surfaces that can't honor them — the `/do:next` claim
@@ -79,11 +79,16 @@ const CUSTOM_MODEL_OPTION = '[custom]';
* `installed` is a per-reviewer-slug install probe from the Code Review
* Defaults endpoint (`GET /api/code-review/defaults`'s `installed` field,
* #3606) — `{ claude: true, antigravity: false, ... }`. Only an explicit
- * `false` renders a "not installed" badge; `undefined` (not a CLI reviewer,
- * or the caller didn't fetch it) renders nothing. Warn-only: a reviewer stays
- * selectable and selected even when flagged not-installed, since the CLI
- * check is local-machine-only and a federated peer (or a later install) may
- * satisfy it.
+ * `false` counts as missing; `undefined` (not a CLI reviewer, or the caller
+ * didn't fetch it) says nothing.
+ *
+ * Together with `modelOptions.providerDisabled`, that decides which reviewers
+ * the **Add** row offers up front: one whose CLI is missing here, or whose
+ * provider records are all switched off, is folded behind a `+N unavailable`
+ * toggle. Warn-only either way — the toggle reveals them with a badge and they
+ * stay selectable, and an ALREADY-SELECTED reviewer always renders its row
+ * (badged), since both checks are local-machine-only and the reviewer list is
+ * federation-wide config a peer may satisfy.
*/
export default function ReviewerPicker({
reviewers = [],
@@ -108,6 +113,9 @@ export default function ReviewerPicker({
// pin maps use. Purely presentational — nothing is stored until an id is typed,
// so this never has to round-trip through `onChange`.
const [customModelTokens, setCustomModelTokens] = useState(() => new Set());
+ // Whether the Add row also lists the reviewers this machine can't run (see
+ // `hiddenAddable`). Presentational only — nothing about it is stored.
+ const [showUnavailable, setShowUnavailable] = useState(false);
const isCustomModel = (token) => customModelTokens.has(token.toLowerCase());
const setCustomModel = (token, on) => setCustomModelTokens((prev) => {
const next = new Set(prev);
@@ -122,7 +130,7 @@ export default function ReviewerPicker({
// the active provider's own reviewer (falling back to copilot when that
// provider maps to none) — see `codeReviewDefaultsFromProvider`.
const selected = Array.isArray(reviewers) ? [...new Set(reviewers.map(normalizeReviewerValue))] : [];
- const available = REVIEWER_OPTIONS.filter(o => !selected.includes(o.value));
+ const addable = REVIEWER_OPTIONS.filter(o => !selected.includes(o.value));
const hasNonCopilot = selected.some(r => r !== 'copilot');
const selectedUsernames = normalizeReviewUsernames(usernames);
const atMaxUsernames = selectedUsernames.length >= MAX_REVIEW_USERNAMES;
@@ -165,20 +173,53 @@ export default function ReviewerPicker({
// normally does", so clearing the select DELETES the key rather than writing `''`.
const effortsMap = asMap(reviewerEfforts);
const efforts = keyedLookup(effortsMap);
- // Only an explicit `false` counts — `undefined` covers both "not a CLI
- // reviewer" (copilot/lmstudio/ollama/@username) and "caller didn't fetch
- // `installed`", neither of which should render a warning badge.
- const notInstalled = (token) => installed?.[token] === false;
- const renderInstalledBadge = (token) => notInstalled(token) && (
-
- not installed
-
- );
+ // Why this reviewer can't run here, or null when nothing says it can't.
+ //
+ // Two independent signals, both warn-only and both reported only when the
+ // caller actually fetched them — a reviewer stays selectable and selected
+ // either way, since the checks are local-machine-only and a federated peer
+ // (or a later install / a flip in Settings) may satisfy them:
+ //
+ // - `installed[token] === false` — the CLI binary isn't on PATH. Only an
+ // explicit `false` counts; `undefined` covers both "not a CLI reviewer"
+ // (copilot/@username) and "caller didn't fetch `installed`".
+ // - `providerDisabled[token]` — every provider record fronting that binary is
+ // switched off on this install, so the user has said they don't use it. A
+ // `/api/providers` that failed or hasn't landed reports nothing (see the
+ // hook), so this never fires on a slow page.
+ const unavailability = (token) => {
+ if (installed?.[token] === false) {
+ return {
+ label: 'not installed',
+ title: `${reviewerLabel(token)}'s CLI binary wasn't found on this machine. It still runs (federation-wide config), but the review loop here will report it unsatisfied until it's installed.`
+ };
+ }
+ if (modelOptions?.providerDisabled?.[token]) {
+ return {
+ label: 'disabled',
+ title: `${reviewerLabel(token)}'s provider records are all switched off in Settings → AI Providers, so this machine isn't set up to use it. Adding it still works — the review loop spawns its CLI directly, and a federated peer may have it enabled.`
+ };
+ }
+ return null;
+ };
+ const renderUnavailableBadge = (token) => {
+ const reason = unavailability(token);
+ return reason && (
+
+ {reason.label}
+
+ );
+ };
+ // The Add row lists what this machine can actually run, so a reviewer whose
+ // CLI is missing or whose providers are all switched off is folded behind a
+ // count instead of padding the row with things the review loop would report
+ // unsatisfied. HIDDEN, not dropped: the checks are local-machine-only and the
+ // reviewer list is federation-wide config, so the toggle reveals them (badged)
+ // rather than making a peer's reviewer unconfigurable from here.
+ const hiddenAddable = addable.filter(opt => unavailability(opt.value));
+ const addOptions = showUnavailable
+ ? addable
+ : addable.filter(opt => !hiddenAddable.includes(opt));
const emit = (next) => onChange?.({
reviewers: selected,
@@ -586,7 +627,7 @@ export default function ReviewerPicker({
{reviewerLabel(value)}
- {renderInstalledBadge(value)}
+ {renderUnavailableBadge(value)}
Model
)}
diff --git a/client/src/components/cos/ReviewerPicker.test.jsx b/client/src/components/cos/ReviewerPicker.test.jsx
index c40ab85c2b..427144c5be 100644
--- a/client/src/components/cos/ReviewerPicker.test.jsx
+++ b/client/src/components/cos/ReviewerPicker.test.jsx
@@ -32,10 +32,66 @@ describe('ReviewerPicker', () => {
expect(screen.queryByText('not installed')).not.toBeInTheDocument();
});
- it('flags an unselected reviewer in the Add row too', () => {
+ it('flags an unselected reviewer once the Add row reveals it', async () => {
+ const user = userEvent.setup();
render( {}} />);
- const addButton = screen.getByRole('button', { name: /Antigravity/ });
- expect(addButton).toHaveTextContent('not installed');
+ await user.click(screen.getByRole('button', { name: /1 unavailable/ }));
+ expect(screen.getByRole('button', { name: /Antigravity/ })).toHaveTextContent('not installed');
+ });
+ });
+
+ // The Add row lists what this machine can actually run. Hidden, not dropped:
+ // both signals are local-machine-only and the reviewer list is
+ // federation-wide config, so a peer's reviewer stays configurable from here.
+ describe('unavailable reviewers in the Add row', () => {
+ const modelOptions = { providerDisabled: { kimi: true, cursor: true } };
+
+ it('hides a missing CLI and an all-off provider behind one count', () => {
+ render(
+ {}}
+ />
+ );
+ expect(screen.getByRole('button', { name: /3 unavailable/ })).toBeInTheDocument();
+ for (const hidden of [/Antigravity/, /Kimi/, /Cursor Agent/]) {
+ expect(screen.queryByRole('button', { name: hidden })).not.toBeInTheDocument();
+ }
+ // An available reviewer is still offered up front.
+ expect(screen.getByRole('button', { name: /Codex/ })).toBeInTheDocument();
+ });
+
+ it('reveals them, badged with which signal fired, and adds them normally', async () => {
+ const onChange = vi.fn();
+ const user = userEvent.setup();
+ render(
+
+ );
+ await user.click(screen.getByRole('button', { name: /3 unavailable/ }));
+ expect(screen.getByRole('button', { name: /Kimi/ })).toHaveTextContent('disabled');
+ expect(screen.getByRole('button', { name: /Antigravity/ })).toHaveTextContent('not installed');
+ await user.click(screen.getByRole('button', { name: /Kimi/ }));
+ expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ reviewers: ['copilot', 'kimi'] }));
+ });
+
+ it('keeps an already-selected unavailable reviewer visible, badged', () => {
+ render(
+ {}} />
+ );
+ expect(screen.getByText('Kimi').parentElement).toHaveTextContent('disabled');
+ });
+
+ it('offers the whole roster when neither signal was fetched', () => {
+ render( {}} />);
+ expect(screen.queryByRole('button', { name: /unavailable/ })).not.toBeInTheDocument();
+ expect(screen.getByRole('button', { name: /Kimi/ })).toBeInTheDocument();
});
});
diff --git a/client/src/hooks/useReviewerModelOptions.js b/client/src/hooks/useReviewerModelOptions.js
index b1e8b2de92..e69792de4b 100644
--- a/client/src/hooks/useReviewerModelOptions.js
+++ b/client/src/hooks/useReviewerModelOptions.js
@@ -1,6 +1,6 @@
import { useEffect, useMemo, useState } from 'react';
import * as api from '../services/api';
-import { filterSelectableModels, selectableModelsForProvider, isAntigravityProvider, isGrokBuildCli, isKimiProvider, antigravityModelEffortLevels } from '../utils/providers';
+import { filterSelectableModels, selectableModelsForProvider, isAntigravityProvider, isCursorProvider, isGrokBuildCli, isKimiProvider, antigravityModelEffortLevels } from '../utils/providers';
import { MODEL_SELECTABLE_REVIEWERS } from '../components/cos/constants';
import { reviewerEffortLevels, normalizeReviewerSlug } from '../lib/reviewerPins';
import { LOCAL_LLM_BACKENDS } from '../lib/localLlmBackends';
@@ -11,6 +11,55 @@ import { LOCAL_LLM_BACKENDS } from '../lib/localLlmBackends';
// listing runs the `mtplx` wrapper, so it stays catalog-sourced and free-text.
const PROBED_LOCAL_BACKENDS = LOCAL_LLM_BACKENDS.map((b) => b.id);
+/**
+ * Every provider record that fronts a reviewer's binary, as predicates in
+ * PREFERENCE ORDER. Two reductions run over each list and they answer different
+ * questions:
+ *
+ * - the **option list** unions every matching record's catalog, because a
+ * reviewer runs one binary and any record fronting that binary lists ids that
+ * binary accepts. Sourcing from a single record made the picker hostage to
+ * that record's staleness — `claude-code` (CLI) listing `claude-sonnet-4-6`
+ * while `claude-code-tui` already listed `claude-sonnet-5` showed the reviewer
+ * the retired tier and hid the current one.
+ * - the **shown default** takes the FIRST match, so a reviewer spawned
+ * non-interactively reports the CLI record's default rather than the TUI's.
+ *
+ * A predicate rather than a bare id wherever the app already recognizes a
+ * provider by more than its shipped id (an `agy` configured by path), so this
+ * classifies the same records the rest of the UI does.
+ *
+ * What is deliberately NOT matched matters as much as what is:
+ * - **No Bedrock/Vertex record.** `claude-code-bedrock` lists
+ * `us.anthropic.*` ids that resolve only under that record's own environment.
+ * - **No `opencode-` preset.** Those enumerate ids that resolve
+ * only under the `OPENCODE_CONFIG_CONTENT` a PortOS-spawned provider injects,
+ * and the reviewer runs a bare `opencode` against the user's OWN config. The
+ * Zen CLI/TUI records are the exception and ARE matched: their ids are the
+ * namespaced `opencode/*` spellings that bare `opencode models` prints, and
+ * the Harnesses page's model refresh fills them from exactly that probe (see
+ * `server/services/harnesses.js#usesHarnessCatalog`), so they are the live
+ * catalog for the account the reviewer will bill.
+ * - **Not `opencode-zen` itself.** That is the HTTP-API record; its bare ids
+ * (`claude-opus-5`) are Zen's API model names, which `opencode -m` cannot
+ * resolve.
+ */
+const REVIEWER_PROVIDER_MATCHERS = Object.freeze({
+ claude: [(p) => p.id === 'claude-code', (p) => p.id === 'claude-code-tui'],
+ codex: [(p) => p.id === 'codex', (p) => p.id === 'codex-tui'],
+ antigravity: [isAntigravityProvider],
+ // `grok` names one binary that ships as BOTH a `cli` and a `tui` provider, and
+ // the reviewer is spawned non-interactively, so the CLI's record wins the
+ // default — the broad predicate follows it for an install that only kept the TUI.
+ grok: [(p) => p.id === 'grok-cli', isGrokBuildCli],
+ cursor: [(p) => p.id === 'cursor-cli', isCursorProvider],
+ kimi: [(p) => p.id === 'kimi-cli', isKimiProvider],
+ opencode: [(p) => p.id === 'opencode-zen-cli', (p) => p.id === 'opencode-zen-tui'],
+ mtplx: [(p) => p.id === 'mtplx'],
+ lmstudio: [(p) => p.id === 'lmstudio'],
+ ollama: [(p) => p.id === 'ollama'],
+});
+
/**
* Selectable model ids per model-taking reviewer, for `ReviewerPicker`'s Model
* column. One hook so all four picker surfaces (Code Review Defaults, TaskAddForm,
@@ -21,15 +70,16 @@ const PROBED_LOCAL_BACKENDS = LOCAL_LLM_BACKENDS.map((b) => b.id);
* - `lmstudio` / `ollama` ids come from `/api/local-llm/status`, so they reflect
* what's actually installed rather than a provider's stale stored `models`.
* - Every other reviewer's tiers come from the provider catalog
- * (`/api/providers`). That includes `mtplx`: its installed checkpoints live
- * behind `/api/local-llm/mtplx/status`, which INVOKES the `mtplx` wrapper (a
- * several-hundred-MB venv bootstrap on a cold version — see
+ * (`/api/providers`), unioned across the records listed in
+ * `REVIEWER_PROVIDER_MATCHERS`. That includes `mtplx`: its installed
+ * checkpoints live behind `/api/local-llm/mtplx/status`, which INVOKES the
+ * `mtplx` wrapper (a several-hundred-MB venv bootstrap on a cold version — see
* `server/lib/mtplxRuntime.js`), and a picker mount must never pay that. The
* shipped provider's catalog plus a free-text field is the honest trade.
*
- * The `claude` list spans BOTH usage modes: the `claude-code` provider tiers and
- * the installed Ollama ids (an Ollama-backed `claude` CLI, where `--model` selects
- * the local model). Deduped, order-preserving.
+ * The `claude` list spans BOTH usage modes: the `claude-code`/`claude-code-tui`
+ * provider tiers and the installed Ollama ids (an Ollama-backed `claude` CLI,
+ * where `--model` selects the local model). Deduped, order-preserving.
*
* `freeText` marks a reviewer whose picker must ALSO accept a typed id, not only
* a pick: an Ollama-backed `claude` can run any locally-installed id, and a
@@ -41,6 +91,14 @@ const PROBED_LOCAL_BACKENDS = LOCAL_LLM_BACKENDS.map((b) => b.id);
* `unavailable` distinguishes "backend is down" from "backend has no models" so
* the empty state can say the useful thing. Absent = not probed (every reviewer
* outside PROBED_LOCAL_BACKENDS, `mtplx` included).
+ *
+ * `providerDisabled[reviewer]` is true when the reviewer HAS provider records on
+ * this install and every one of them is switched off — the signal
+ * `ReviewerPicker` uses to drop it from the Add row, so a machine that never
+ * enabled Kimi or Cursor isn't offered them. Only ever true from a landed fetch:
+ * a null/failed `/api/providers` matches no record, which reads as "nothing
+ * known", never as "switched off".
+ *
* `loaded` flips once both fetches settle, so a consumer can tell "no options
* yet" from "genuinely no options" (an empty list is a real answer, not a
* pre-fetch placeholder).
@@ -51,7 +109,7 @@ const PROBED_LOCAL_BACKENDS = LOCAL_LLM_BACKENDS.map((b) => b.id);
* reviewer returns its static ladder. Lives here rather than in the picker so the
* picker keeps doing no fetching of its own.
*
- * @returns {{ optionsByReviewer: Record, defaultModels: Record, freeText: Record, unavailable: Record, modelEffortLevels: (reviewer: string, model?: string|null) => readonly string[]|null, loaded: boolean, reviewers: string[] }}
+ * @returns {{ optionsByReviewer: Record, defaultModels: Record, freeText: Record, unavailable: Record, providerDisabled: Record, modelEffortLevels: (reviewer: string, model?: string|null) => readonly string[]|null, loaded: boolean, reviewers: string[] }}
*/
export default function useReviewerModelOptions() {
const [localStatus, setLocalStatus] = useState(null);
@@ -78,34 +136,49 @@ export default function useReviewerModelOptions() {
const localIds = (backend) => (localStatus?.[backend]?.models || [])
.map((m) => m.id || m.name)
.filter(Boolean);
- // `match` is a predicate rather than an id so a reviewer whose provider can be
- // recognized by more than its shipped id (an `agy` configured by path) uses the
- // same predicate the rest of the app does. Several matchers = preference order:
- // `grok` names one binary that ships as BOTH a `cli` and a `tui` provider, and
- // the reviewer is spawned non-interactively, so the CLI's catalog wins — the
- // broad predicate is the fallback for an install that only kept the TUI.
- const providerFor = (...matchers) =>
- matchers.reduce((found, match) => found || (providers || []).find(match), null);
- const providerTiers = (...matchers) => {
- const provider = providerFor(...matchers);
- if (!provider) return [];
+ // Every record fronting each reviewer's binary, in matcher-preference order
+ // (not `providers` array order) so `[0]` is the record whose default the
+ // picker shows. De-duped by identity: two matchers commonly overlap
+ // (`grok-cli` is also an `isGrokBuildCli`).
+ //
+ // Resolved ONCE for the whole roster rather than per lookup — the option
+ // list, the shown default, the agy raw catalog and `providerDisabled` all
+ // ask the same question, and a per-call helper re-walked the provider array
+ // for every one of them.
+ const providersByReviewer = Object.fromEntries(
+ Object.entries(REVIEWER_PROVIDER_MATCHERS).map(([reviewer, matchers]) => {
+ const matched = [];
+ for (const match of matchers) {
+ for (const provider of providers || []) {
+ if (match(provider) && !matched.includes(provider)) matched.push(provider);
+ }
+ }
+ return [reviewer, matched];
+ })
+ );
+ const providersFor = (reviewer) => providersByReviewer[reviewer] || [];
+
+ // `selectableModelsForProvider` owns the per-provider normalization (today:
+ // agy's one-id-per-effort-tier catalog collapsed to base ids, so the row's
+ // separate Effort cell stays the effort control). Going through it rather
+ // than special-casing agy here keeps the rule in one place.
+ const selectableModels = (provider) => {
// `models` may be empty on a CLI provider configured with only a
// defaultModel — `[]` is truthy, so a bare `||` wouldn't fall through.
const models = provider.models?.length ? provider.models : [provider.defaultModel];
- // `selectableModelsForProvider` owns the per-provider normalization (today:
- // agy's one-id-per-effort-tier catalog collapsed to base ids, so the row's
- // separate Effort cell stays the effort control). Going through it rather
- // than special-casing agy here keeps the rule in one place.
return filterSelectableModels(selectableModelsForProvider(provider, models));
};
+ const providerTiers = (reviewer) =>
+ Array.from(new Set(providersFor(reviewer).flatMap(selectableModels)));
+
// Show the configured provider default in the picker even when the user has
// not saved a per-reviewer override. A concrete default is useful context;
// configured-default sentinels intentionally resolve to null because the CLI
// owns the choice and there is no model id PortOS can honestly display.
- const providerDefault = (...matchers) => {
- const provider = providerFor(...matchers);
+ const providerDefault = (reviewer) => {
+ const provider = providersFor(reviewer)[0];
if (!provider?.defaultModel) return null;
return filterSelectableModels(
selectableModelsForProvider(provider, [provider.defaultModel])
@@ -115,7 +188,7 @@ export default function useReviewerModelOptions() {
// Local backend model lists come from the live runtime probe, so only show a
// provider default when it is present in that authoritative list.
const localDefault = (backend) => {
- const candidate = providerFor((p) => p.id === backend)?.defaultModel;
+ const candidate = providersFor(backend)[0]?.defaultModel;
return candidate && localIds(backend).includes(candidate) ? candidate : null;
};
@@ -123,52 +196,58 @@ export default function useReviewerModelOptions() {
const optionsByReviewer = {
lmstudio: localIds('lmstudio'),
ollama,
- codex: providerTiers((p) => p.id === 'codex'),
+ codex: providerTiers('codex'),
// Claude tiers first (the common case), then installed Ollama ids for an
// Ollama-backed `claude`. Deduped, order-preserving.
- claude: Array.from(new Set([...providerTiers((p) => p.id === 'claude-code'), ...ollama].filter(Boolean))),
- antigravity: providerTiers(isAntigravityProvider),
+ claude: Array.from(new Set([...providerTiers('claude'), ...ollama].filter(Boolean))),
+ antigravity: providerTiers('antigravity'),
// The shipped grok provider carries only the configured-default sentinel,
// which `filterSelectableModels` strips — so this is legitimately `[]` until
// the user lists real ids on the provider. The Model cell stays useful
// regardless because grok, like every CLI reviewer, is free-text.
- grok: providerTiers((p) => p.id === 'grok-cli', isGrokBuildCli),
- cursor: providerTiers((p) => p.id === 'cursor-cli', (p) => p.id === 'cursor-tui'),
+ grok: providerTiers('grok'),
+ cursor: providerTiers('cursor'),
// Legitimately empty, for grok's documented reason: the shipped kimi
// provider carries only the configured-default sentinel, which
// `filterSelectableModels` strips. Free-text keeps the cell usable.
- kimi: providerTiers((p) => p.id === 'kimi-cli', isKimiProvider),
- // Deliberately NOT sourced from the `opencode-` presets. Those
- // enumerate ids that only resolve under the `OPENCODE_CONFIG_CONTENT` a
- // PortOS-spawned provider injects, and the reviewer runs a bare `opencode`
- // against the user's OWN config — so listing them would offer picks that
- // silently fail. `opencode -m` takes a `provider/model` id the user types.
- opencode: [],
- mtplx: providerTiers((p) => p.id === 'mtplx'),
+ kimi: providerTiers('kimi'),
+ // The namespaced `opencode/*` ids the seeded Zen CLI/TUI records carry,
+ // which the Harnesses page refreshes from `opencode models` — so the cell
+ // is a dropdown of what this account can actually run instead of the plain
+ // text input it used to be. Still free-text underneath: `opencode -m` takes
+ // any `provider/model` the user's own config resolves.
+ opencode: providerTiers('opencode'),
+ mtplx: providerTiers('mtplx'),
};
const defaultModels = {
lmstudio: localDefault('lmstudio'),
ollama: localDefault('ollama'),
- codex: providerDefault((p) => p.id === 'codex'),
- claude: providerDefault((p) => p.id === 'claude-code'),
- antigravity: providerDefault(isAntigravityProvider),
- grok: providerDefault((p) => p.id === 'grok-cli', isGrokBuildCli),
- cursor: providerDefault((p) => p.id === 'cursor-cli', (p) => p.id === 'cursor-tui'),
- kimi: providerDefault((p) => p.id === 'kimi-cli', isKimiProvider),
+ codex: providerDefault('codex'),
+ claude: providerDefault('claude'),
+ antigravity: providerDefault('antigravity'),
+ grok: providerDefault('grok'),
+ cursor: providerDefault('cursor'),
+ kimi: providerDefault('kimi'),
+ // Deliberately null even though the Zen records carry one: the reviewer
+ // spawns a BARE `opencode`, which falls back to whatever the user's own
+ // config names — not to the PortOS record's default. Naming a model here
+ // would claim a default the run won't use.
opencode: null,
- mtplx: providerDefault((p) => p.id === 'mtplx'),
+ mtplx: providerDefault('mtplx'),
};
- // The agy provider's RAW catalog — one id per effort tier
+ // The agy providers' RAW catalog — one id per effort tier
// (`gemini-3.6-flash-low|-medium|-high`), which is exactly what the narrowing
// reads. Deliberately NOT `optionsByReviewer.antigravity`: that list has
// already had the suffixes collapsed away, so it carries no tier information.
- const antigravityCatalog = (providers || []).find(isAntigravityProvider)?.models || [];
+ const antigravityCatalog = Array.from(new Set(
+ providersFor('antigravity').flatMap((provider) => provider.models || [])
+ ));
// The effort ladder a reviewer offers ONCE ITS MODEL IS PINNED. `agy` validates
// the pair, so a model with no `-medium` sibling must not offer `medium`
- // (#3733). `antigravityModelEffortLevels` returns null for "can't tell" — empty
- // catalog, unset model, or the configured-default sentinel — and the full
+ // (#3733). `antigravityModelEffortLevels` returns null for "can't tell" —
+ // empty catalog, unset model, or the configured-default sentinel — and the full
// static ladder stands there, the same null-means-fall-back contract
// `effortLevelsForProvider` uses. `[]` is a real answer: that model has no
// effort tiers at all.
@@ -191,8 +270,9 @@ export default function useReviewerModelOptions() {
// because their catalogs are stored snapshots that can lag a newly-released
// tier — grok's shipped catalog holds no real id at all, so a typed id is the
// ONLY way to pin one (an agy pin may also be typed effort-suffixed — the
- // server splits it). Derived from the rosters so a reviewer added to either
- // one can't silently default to the wrong control.
+ // server splits it) — and `opencode` because a user's own config can declare
+ // namespaces the Zen catalog never lists. Derived from the rosters so a
+ // reviewer added to either one can't silently default to the wrong control.
freeText: Object.fromEntries(
MODEL_SELECTABLE_REVIEWERS.map((r) => [r, !PROBED_LOCAL_BACKENDS.includes(r)])
),
@@ -200,6 +280,19 @@ export default function useReviewerModelOptions() {
lmstudio: localStatus?.lmstudio?.available === false,
ollama: localStatus?.ollama?.available === false,
},
+ // `every` over a NON-EMPTY match list, so the two ways to have no enabled
+ // record stay apart: an install that switched every Kimi record off is
+ // `true` (hide it), while a reviewer with no records at all — or a fetch
+ // that failed or hasn't landed — is `false` (nothing is known, so hide
+ // nothing). `enabled === false` rather than falsiness, which deliberately
+ // reads a record with no `enabled` key as ON — the opposite of
+ // `providerCardState`'s stricter test, because this answer HIDES a control
+ // and incomplete data must never do that.
+ providerDisabled: Object.fromEntries(
+ Object.entries(providersByReviewer).map(([reviewer, matched]) =>
+ [reviewer, matched.length > 0 && matched.every((p) => p.enabled === false)]
+ )
+ ),
loaded,
// Exposed so a consumer can assert it covers every model-taking reviewer.
reviewers: MODEL_SELECTABLE_REVIEWERS,
diff --git a/client/src/hooks/useReviewerModelOptions.test.jsx b/client/src/hooks/useReviewerModelOptions.test.jsx
index 06503d1637..394ae6120d 100644
--- a/client/src/hooks/useReviewerModelOptions.test.jsx
+++ b/client/src/hooks/useReviewerModelOptions.test.jsx
@@ -24,12 +24,21 @@ const providers = [
'gemini-3.1-pro-high',
],
},
- // One `grok` binary ships as both a TUI and a CLI provider; the reviewer is
- // spawned non-interactively, so the CLI's catalog is the one it should offer.
- { id: 'grok-tui', type: 'tui', command: 'grok', models: ['stale-tui-id'] },
+ // One `grok` binary ships as both a TUI and a CLI provider. Both list ids that
+ // binary accepts, so the picker unions them; the CLI's record still owns the
+ // shown DEFAULT, since the reviewer is spawned non-interactively.
+ { id: 'grok-tui', type: 'tui', command: 'grok', models: ['tui-only-id'] },
{ id: 'grok-cli', type: 'cli', command: 'grok', models: ['grok-configured-default', 'grok-code-fast-1'] },
{ id: 'cursor-cli', type: 'cli', command: 'cursor-agent', models: ['auto', 'gpt-5'] },
{ id: 'mtplx', type: 'api', models: ['mtplx-qwen38-27b-optimized-speed'], defaultModel: 'mtplx-qwen38-27b-optimized-speed' },
+ // The seeded OpenCode Zen wrappers, whose namespaced ids the Harnesses page
+ // refreshes from `opencode models` — the reviewer's dropdown source.
+ { id: 'opencode-zen-cli', type: 'cli', command: 'opencode', models: ['opencode/big-pickle'], defaultModel: 'opencode/big-pickle' },
+ { id: 'opencode-zen-tui', type: 'tui', command: 'opencode', models: ['opencode/big-pickle', 'opencode/mimo-v2.5-free'] },
+ // An OpenCode wrapper pointed at a local runtime: its ids resolve only under
+ // the config PortOS injects, so the reviewer (a BARE `opencode`) must not
+ // offer them.
+ { id: 'opencode-ollama', type: 'cli', command: 'opencode', ollamaBacked: true, models: ['qwen3-coder:30b'] },
];
describe('useReviewerModelOptions', () => {
@@ -69,6 +78,28 @@ describe('useReviewerModelOptions', () => {
expect(result.current.defaultModels.antigravity).toBeNull();
});
+ // The reported defect: `claude-code` (CLI) still listed the retired
+ // `claude-sonnet-4-6` while `claude-code-tui` had moved to `claude-sonnet-5`,
+ // and sourcing the picker from the CLI record alone showed the retired tier
+ // and hid the current one. `claude` has no `models` subcommand, so nothing can
+ // refresh that record in place.
+ it('unions the Claude CLI and TUI catalogs so one stale record can\'t hide a live tier', async () => {
+ getProviders.mockResolvedValue({ providers: [
+ { id: 'claude-code', type: 'cli', command: 'claude', models: ['claude-haiku-4-5', 'claude-sonnet-4-6'], defaultModel: 'claude-haiku-4-5' },
+ { id: 'claude-code-tui', type: 'tui', command: 'claude', models: ['claude-sonnet-5', 'claude-opus-5'] },
+ // Bedrock ids resolve only under that record's own environment.
+ { id: 'claude-code-bedrock', type: 'cli', command: 'claude', models: ['us.anthropic.claude-sonnet-5'] },
+ ] });
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ expect(result.current.optionsByReviewer.claude).toEqual([
+ 'claude-haiku-4-5', 'claude-sonnet-4-6', 'claude-sonnet-5', 'claude-opus-5', 'qwen2.5:7b',
+ ]);
+ // The CLI record still owns the shown default — the reviewer is spawned
+ // non-interactively.
+ expect(result.current.defaultModels.claude).toBe('claude-haiku-4-5');
+ });
+
// #3728: `agy --model ` is real, so the antigravity row gets a Model cell.
// Its ids arrive effort-suffixed and would otherwise duplicate the Effort cell
// (and hand agy a `--model X-high --effort high` pair it rejects).
@@ -82,10 +113,14 @@ describe('useReviewerModelOptions', () => {
});
// #3729: `grok --model ` is real, so the grok row gets a Model cell too.
- it('sources grok options from the CLI provider, not the TUI, and drops the sentinel', async () => {
+ // Both records front the same binary, so the option list unions them (CLI
+ // first) rather than letting one record's staleness hide the other's ids —
+ // sourcing from a single record is exactly what hid `claude-sonnet-5` behind
+ // the CLI record's retired `claude-sonnet-4-6`.
+ it('unions the grok CLI and TUI catalogs, CLI first, and drops the sentinel', async () => {
const { result } = renderHook(() => useReviewerModelOptions());
await waitFor(() => expect(result.current.loaded).toBe(true));
- expect(result.current.optionsByReviewer.grok).toEqual(['grok-code-fast-1']);
+ expect(result.current.optionsByReviewer.grok).toEqual(['grok-code-fast-1', 'tui-only-id']);
// Free-text: the shipped grok catalog is sentinel-only, so a typed id is
// often the only way to pin one.
expect(result.current.freeText.grok).toBe(true);
@@ -102,7 +137,70 @@ describe('useReviewerModelOptions', () => {
getProviders.mockResolvedValue({ providers: [providers.find((p) => p.id === 'grok-tui')] });
const { result } = renderHook(() => useReviewerModelOptions());
await waitFor(() => expect(result.current.loaded).toBe(true));
- expect(result.current.optionsByReviewer.grok).toEqual(['stale-tui-id']);
+ expect(result.current.optionsByReviewer.grok).toEqual(['tui-only-id']);
+ });
+
+ // The Model cell used to be a bare text input for opencode: the reviewer runs a
+ // BARE `opencode`, and the only catalogs on hand were the `opencode-`
+ // presets, whose ids resolve solely under the config a PortOS-spawned provider
+ // injects. The seeded Zen wrappers are the exception — their namespaced ids are
+ // what `opencode models` prints, and the Harnesses page refreshes them from
+ // exactly that probe — so those, and only those, feed the dropdown.
+ describe('opencode', () => {
+ it('offers the Zen wrappers\' namespaced ids, unioned across CLI and TUI', async () => {
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ expect(result.current.optionsByReviewer.opencode)
+ .toEqual(['opencode/big-pickle', 'opencode/mimo-v2.5-free']);
+ // Still free-text underneath: a user's own config can declare namespaces
+ // the Zen catalog never lists.
+ expect(result.current.freeText.opencode).toBe(true);
+ });
+
+ it('never offers a local-runtime wrapper\'s ids', async () => {
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ expect(result.current.optionsByReviewer.opencode).not.toContain('qwen3-coder:30b');
+ });
+
+ // The reviewer spawns a bare `opencode`, which falls back to whatever the
+ // user's OWN config names — not to the PortOS record's default. Showing the
+ // record's default would claim a model the run won't use.
+ it('shows no default even though the Zen record carries one', async () => {
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ expect(result.current.defaultModels.opencode).toBeNull();
+ });
+ });
+
+ // The Add row hides what this machine can't run, so "every record switched
+ // off" has to stay apart from "no record at all" and from "fetch failed".
+ describe('providerDisabled', () => {
+ it('is true only when every record fronting the reviewer is switched off', async () => {
+ getProviders.mockResolvedValue({ providers: [
+ { id: 'kimi-cli', type: 'cli', command: 'kimi', enabled: false, models: [] },
+ { id: 'kimi-tui', type: 'tui', command: 'kimi', enabled: false, models: [] },
+ { id: 'grok-cli', type: 'cli', command: 'grok', enabled: false, models: [] },
+ { id: 'grok-tui', type: 'tui', command: 'grok', enabled: true, models: [] },
+ // No `enabled` key at all — a record written before the flag existed
+ // must not read as off.
+ { id: 'codex', type: 'cli', command: 'codex', models: [] },
+ ] });
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ expect(result.current.providerDisabled.kimi).toBe(true);
+ expect(result.current.providerDisabled.grok).toBe(false);
+ expect(result.current.providerDisabled.codex).toBe(false);
+ });
+
+ it('reports nothing disabled when the reviewer has no records, or the fetch failed', async () => {
+ getProviders.mockResolvedValue({ providers: [] });
+ const { result } = renderHook(() => useReviewerModelOptions());
+ await waitFor(() => expect(result.current.loaded).toBe(true));
+ for (const reviewer of MODEL_SELECTABLE_REVIEWERS) {
+ expect(result.current.providerDisabled[reviewer], reviewer).toBe(false);
+ }
+ });
});
// #3733: `agy` validates the model/effort PAIR, so the Effort cell's ladder has
diff --git a/scripts/migrations/337-claude-sonnet-5-additive.js b/scripts/migrations/337-claude-sonnet-5-additive.js
new file mode 100644
index 0000000000..d02b6d03af
--- /dev/null
+++ b/scripts/migrations/337-claude-sonnet-5-additive.js
@@ -0,0 +1,82 @@
+/**
+ * Offer `claude-sonnet-5` on a Claude CLI/TUI record that still lists only the
+ * retired `claude-sonnet-4-6` sonnet tier.
+ *
+ * Migration 153 already made this swap, but ONLY for a `models` array matching
+ * the prior seeded trio exactly — a user who had appended an id to the list (a
+ * Fable tier, say) was classified as "customized" and skipped, and their record
+ * kept the 4-6 tier while the shipped seed and their other Claude records moved
+ * on. `claude` has no `models` subcommand, so nothing in the app can refresh
+ * that record: the reviewer/task model pickers reading it offer the retired
+ * sonnet and cannot offer the current one at all.
+ *
+ * ADDITIVE, deliberately — the opposite policy from 153/206 and from
+ * `makeSeededProviderTierMigration`, because this one runs against lists the
+ * user curated:
+ *
+ * - `claude-sonnet-5` is INSERTED right after `claude-sonnet-4-6`, and the
+ * retired id is KEPT. `claude-sonnet-4-6` still resolves for the CLI, so
+ * dropping an id a user chose to list would remove a working pin; the defect
+ * is the new tier being absent, not the old one being present.
+ * - Tier pointers (`defaultModel`/`lightModel`/`mediumModel`/`heavyModel`) are
+ * left ALONE. They point at an id that still works, and a curated list is
+ * exactly where re-pointing would override a deliberate choice.
+ *
+ * Idempotent by the same condition either way: a record already listing
+ * `claude-sonnet-5` is untouched, so this is a no-op on a seeded install (153/206
+ * or a fresh `data.reference` seed already put it there) and on a second run.
+ */
+
+import { readProvidersDoc, writeJsonAtomic } from './_lib.js';
+
+const PROVIDERS_REL_PATH = 'data/providers.json';
+
+// The four seeded Claude records and the sonnet id each one spells. The Bedrock
+// pair uses the region-qualified form its own environment resolves — inserting a
+// bare `claude-sonnet-5` there would offer an id that record cannot run.
+const TARGETS = [
+ { id: 'claude-code', retired: 'claude-sonnet-4-6', current: 'claude-sonnet-5' },
+ { id: 'claude-code-tui', retired: 'claude-sonnet-4-6', current: 'claude-sonnet-5' },
+ { id: 'claude-code-bedrock', retired: 'us.anthropic.claude-sonnet-4-6', current: 'us.anthropic.claude-sonnet-5' },
+ { id: 'claude-code-tui-bedrock', retired: 'us.anthropic.claude-sonnet-4-6', current: 'us.anthropic.claude-sonnet-5' },
+];
+
+export default {
+ async up({ rootDir }) {
+ const doc = await readProvidersDoc({ rootDir });
+ if (!doc.ok) {
+ if (doc.reason === 'no-file') console.log(`📄 ${PROVIDERS_REL_PATH} not present — skipping (fresh install seeds claude-sonnet-5 from data.reference)`);
+ else if (doc.reason === 'unreadable') console.log(`⚠️ ${PROVIDERS_REL_PATH}: invalid JSON, skipping (${doc.err.message})`);
+ else console.log(`⚠️ ${PROVIDERS_REL_PATH}: unexpected shape, skipping`);
+ return { ok: false, reason: doc.reason, updated: 0 };
+ }
+
+ const { config, providers, path: providersPath } = doc;
+ const touched = [];
+
+ for (const { id, retired, current } of TARGETS) {
+ const provider = providers[id];
+ if (!provider || !Array.isArray(provider.models)) continue;
+ const at = provider.models.indexOf(retired);
+ // Nothing to repair unless the retired id is listed AND the current one
+ // isn't: an already-current record (seeded, or bumped by 153) is a no-op,
+ // and a record that never listed the retired tier is not this bug.
+ if (at === -1 || provider.models.includes(current)) continue;
+ provider.models = [
+ ...provider.models.slice(0, at + 1),
+ current,
+ ...provider.models.slice(at + 1),
+ ];
+ touched.push(id);
+ }
+
+ if (touched.length === 0) {
+ console.log(`✅ ${PROVIDERS_REL_PATH}: Claude sonnet tier already current — no change`);
+ return { ok: true, reason: 'already-current', updated: 0 };
+ }
+
+ await writeJsonAtomic(providersPath, config);
+ console.log(`📝 ${PROVIDERS_REL_PATH}: offered claude-sonnet-5 on ${touched.join(', ')}`);
+ return { ok: true, reason: 'updated', updated: touched.length };
+ },
+};
diff --git a/scripts/migrations/337-claude-sonnet-5-additive.test.js b/scripts/migrations/337-claude-sonnet-5-additive.test.js
new file mode 100644
index 0000000000..47316977fe
--- /dev/null
+++ b/scripts/migrations/337-claude-sonnet-5-additive.test.js
@@ -0,0 +1,125 @@
+/**
+ * Test for migration 337 — offer `claude-sonnet-5` on a Claude CLI/TUI record
+ * that still lists only the retired `claude-sonnet-4-6` tier.
+ */
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, rmSync, writeFileSync, readFileSync, mkdirSync } from 'fs';
+import { tmpdir } from 'os';
+import { join } from 'path';
+
+import migration from './337-claude-sonnet-5-additive.js';
+
+const writeJson = (path, value) => writeFileSync(path, JSON.stringify(value, null, 2) + '\n');
+const readJson = (path) => JSON.parse(readFileSync(path, 'utf-8'));
+
+describe('migration 337 — claude-sonnet-5 additive repair', () => {
+ let rootDir;
+ let providersPath;
+
+ beforeEach(() => {
+ rootDir = mkdtempSync(join(tmpdir(), 'portos-337-'));
+ mkdirSync(join(rootDir, 'data'));
+ providersPath = join(rootDir, 'data', 'providers.json');
+ });
+
+ afterEach(() => rmSync(rootDir, { recursive: true, force: true }));
+
+ const seed = (providers) => writeJson(providersPath, { activeProvider: 'claude-code', providers });
+
+ it('inserts claude-sonnet-5 after the retired tier on a CURATED list 153 skipped', async () => {
+ seed({
+ 'claude-code': {
+ models: ['claude-haiku-4-5', 'claude-sonnet-4-6', 'claude-opus-5', 'claude-fable-5'],
+ defaultModel: 'claude-opus-5',
+ mediumModel: 'claude-sonnet-4-6',
+ },
+ });
+
+ const result = await migration.up({ rootDir });
+
+ expect(result).toMatchObject({ ok: true, reason: 'updated', updated: 1 });
+ const after = readJson(providersPath).providers['claude-code'];
+ expect(after.models).toEqual([
+ 'claude-haiku-4-5', 'claude-sonnet-4-6', 'claude-sonnet-5', 'claude-opus-5', 'claude-fable-5',
+ ]);
+ // Additive: the retired id and every tier pointer survive untouched.
+ expect(after.mediumModel).toBe('claude-sonnet-4-6');
+ expect(after.defaultModel).toBe('claude-opus-5');
+ });
+
+ it('uses each Bedrock record\'s own region-qualified sonnet spelling', async () => {
+ seed({
+ 'claude-code-bedrock': {
+ models: ['us.anthropic.claude-haiku-4-5', 'us.anthropic.claude-sonnet-4-6', 'global.anthropic.claude-opus-5'],
+ },
+ 'claude-code-tui-bedrock': {
+ models: ['us.anthropic.claude-sonnet-4-6'],
+ },
+ });
+
+ const result = await migration.up({ rootDir });
+
+ expect(result.updated).toBe(2);
+ const { providers } = readJson(providersPath);
+ expect(providers['claude-code-bedrock'].models).toEqual([
+ 'us.anthropic.claude-haiku-4-5',
+ 'us.anthropic.claude-sonnet-4-6',
+ 'us.anthropic.claude-sonnet-5',
+ 'global.anthropic.claude-opus-5',
+ ]);
+ expect(providers['claude-code-tui-bedrock'].models).toEqual([
+ 'us.anthropic.claude-sonnet-4-6',
+ 'us.anthropic.claude-sonnet-5',
+ ]);
+ // The bare id must never leak into a Bedrock record — its environment
+ // resolves only the region-qualified form.
+ expect(providers['claude-code-bedrock'].models).not.toContain('claude-sonnet-5');
+ });
+
+ it('is a no-op on an already-current record and on a second run', async () => {
+ seed({
+ 'claude-code': { models: ['claude-haiku-4-5', 'claude-sonnet-5', 'claude-opus-5'] },
+ 'claude-code-tui': { models: ['claude-haiku-4-5', 'claude-sonnet-4-6', 'claude-sonnet-5'] },
+ });
+
+ const first = await migration.up({ rootDir });
+ expect(first).toMatchObject({ ok: true, reason: 'already-current', updated: 0 });
+
+ // And a record it DID repair stays repaired rather than gaining a duplicate.
+ seed({ 'claude-code': { models: ['claude-sonnet-4-6'] } });
+ expect((await migration.up({ rootDir })).updated).toBe(1);
+ const second = await migration.up({ rootDir });
+ expect(second.updated).toBe(0);
+ expect(readJson(providersPath).providers['claude-code'].models)
+ .toEqual(['claude-sonnet-4-6', 'claude-sonnet-5']);
+ });
+
+ it('leaves records outside the four seeded Claude ids alone', async () => {
+ seed({
+ 'claude-ollama': { models: ['claude-sonnet-4-6'] },
+ 'antigravity-cli': { models: ['claude-sonnet-4-6'] },
+ });
+
+ expect((await migration.up({ rootDir })).updated).toBe(0);
+ const { providers } = readJson(providersPath);
+ expect(providers['claude-ollama'].models).toEqual(['claude-sonnet-4-6']);
+ expect(providers['antigravity-cli'].models).toEqual(['claude-sonnet-4-6']);
+ });
+
+ it('skips a missing or malformed providers file without throwing', async () => {
+ expect(await migration.up({ rootDir })).toMatchObject({ ok: false, reason: 'no-file' });
+
+ writeFileSync(providersPath, '{not json');
+ expect(await migration.up({ rootDir })).toMatchObject({ ok: false, reason: 'unreadable' });
+
+ writeJson(providersPath, { activeProvider: 'claude-code' });
+ expect(await migration.up({ rootDir })).toMatchObject({ ok: false, reason: 'bad-shape' });
+ });
+
+ it('skips a record whose models field is not an array', async () => {
+ seed({ 'claude-code': { models: 'claude-sonnet-4-6', defaultModel: 'claude-sonnet-4-6' } });
+
+ expect((await migration.up({ rootDir })).updated).toBe(0);
+ expect(readJson(providersPath).providers['claude-code'].models).toBe('claude-sonnet-4-6');
+ });
+});