From a3ee1d286e95b501b3d9d7bb26367a0edea46166 Mon Sep 17 00:00:00 2001 From: Christian Klotz Date: Sat, 25 Jul 2026 08:19:56 +0300 Subject: [PATCH] fix(coding-agent): expose unavailable scoped models (#7032) * fix(coding-agent): expose unavailable scoped models Keep missing configured models editable in the scoped-model selector. This fixes #6949. * fix(coding-agent): reuse model scope diagnostics * fix(coding-agent): classify model scope diagnostics --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/core/model-resolver.ts | 17 +- .../components/scoped-models-selector.ts | 53 +++--- .../src/modes/interactive/interactive-mode.ts | 48 ++++-- .../coding-agent/test/model-resolver.test.ts | 2 + .../6949-unavailable-scoped-model.test.ts | 160 ++++++++++++++++++ 6 files changed, 244 insertions(+), 37 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0a3215890..a7c2251db 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed compaction and branch summaries for providers whose authentication resolves entirely to request headers ([#5871](https://github.com/earendil-works/pi/issues/5871)) +- Fixed unavailable scoped models being hidden from `/models`, allowing them to be removed without editing settings manually ([#6949](https://github.com/earendil-works/pi/issues/6949)). ## [0.82.0] - 2026-07-24 diff --git a/packages/coding-agent/src/core/model-resolver.ts b/packages/coding-agent/src/core/model-resolver.ts index 02826fc30..b5c21aa4c 100644 --- a/packages/coding-agent/src/core/model-resolver.ts +++ b/packages/coding-agent/src/core/model-resolver.ts @@ -260,6 +260,7 @@ export function parseModelPattern( */ export interface ModelScopeDiagnostic { type: "warning"; + code: "no-match" | "invalid-thinking-level"; message: string; pattern: string; } @@ -309,7 +310,12 @@ export async function resolveModelScopeWithDiagnostics( }); if (matchingModels.length === 0) { - diagnostics.push({ type: "warning", message: `No models match pattern "${pattern}"`, pattern }); + diagnostics.push({ + type: "warning", + code: "no-match", + message: `No models match pattern "${pattern}"`, + pattern, + }); continue; } @@ -324,11 +330,16 @@ export async function resolveModelScopeWithDiagnostics( const { model, thinkingLevel, warning } = parseModelPattern(pattern, availableModels); if (warning) { - diagnostics.push({ type: "warning", message: warning, pattern }); + diagnostics.push({ type: "warning", code: "invalid-thinking-level", message: warning, pattern }); } if (!model) { - diagnostics.push({ type: "warning", message: `No models match pattern "${pattern}"`, pattern }); + diagnostics.push({ + type: "warning", + code: "no-match", + message: `No models match pattern "${pattern}"`, + pattern, + }); continue; } diff --git a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts index 772e3af06..fc5c61133 100644 --- a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts @@ -36,7 +36,7 @@ function enableAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[ for (const id of targets) { if (!result.includes(id)) result.push(id); } - return result.length === allIds.length ? null : result; + return result.length === allIds.length && result.every((id) => allIds.includes(id)) ? null : result; } function clearAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds { @@ -67,7 +67,7 @@ function getSortedIds(enabledIds: EnabledIds, allIds: string[]): string[] { interface ModelItem { fullId: string; - model: Model; + model: Model | undefined; enabled: boolean; } @@ -152,20 +152,20 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl } private buildItems(): ModelItem[] { - // Filter out IDs that no longer have a corresponding model (e.g., after logout) - return getSortedIds(this.enabledIds, this.allIds) - .filter((id) => this.modelsById.has(id)) - .map((id) => ({ - fullId: id, - model: this.modelsById.get(id)!, - enabled: isEnabled(this.enabledIds, id), - })); + return getSortedIds(this.enabledIds, this.allIds).map((id) => ({ + fullId: id, + model: this.modelsById.get(id), + enabled: isEnabled(this.enabledIds, id), + })); } private getFooterText(): string { - const enabledCount = this.enabledIds?.length ?? this.allIds.length; + const enabledCount = this.enabledIds?.filter((id) => this.modelsById.has(id)).length ?? this.allIds.length; + const unavailableCount = this.enabledIds?.filter((id) => !this.modelsById.has(id)).length ?? 0; const allEnabled = this.enabledIds === null; - const countText = allEnabled ? "all enabled" : `${enabledCount}/${this.allIds.length} enabled`; + const countText = allEnabled + ? "all enabled" + : `${enabledCount}/${this.allIds.length} enabled${unavailableCount ? ` · ${unavailableCount} unavailable` : ""}`; const parts = [ `${keyText("tui.select.confirm")} toggle`, `${keyText("app.models.enableAll")} all`, @@ -184,8 +184,10 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl const query = this.searchInput.getValue(); const items = this.buildItems(); this.filteredItems = query - ? fuzzyFilter(items, query, (i) => - getModelSearchText({ id: i.model.id, provider: i.model.provider, name: i.model.name }), + ? fuzzyFilter(items, query, (item) => + item.model + ? getModelSearchText({ id: item.model.id, provider: item.model.provider, name: item.model.name }) + : item.fullId, ) : items; this.selectedIndex = Math.min(this.selectedIndex, Math.max(0, this.filteredItems.length - 1)); @@ -216,9 +218,16 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl const item = this.filteredItems[i]!; const isSelected = i === this.selectedIndex; const prefix = isSelected ? theme.fg("accent", "→ ") : " "; - const modelText = isSelected ? theme.fg("accent", item.model.id) : item.model.id; - const providerBadge = theme.fg("muted", ` [${item.model.provider}]`); - const status = allEnabled ? "" : item.enabled ? theme.fg("success", " ✓") : theme.fg("dim", " ✗"); + const id = item.model?.id ?? item.fullId; + const modelText = isSelected ? theme.fg("accent", id) : id; + const providerBadge = theme.fg("muted", item.model ? ` [${item.model.provider}]` : " [unavailable]"); + const status = item.model + ? allEnabled + ? "" + : item.enabled + ? theme.fg("success", " ✓") + : theme.fg("dim", " ✗") + : theme.fg("dim", " ✗"); this.listContainer.addChild(new Text(`${prefix}${modelText}${providerBadge}${status}`, 0, 0)); } @@ -232,7 +241,13 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl if (this.filteredItems.length > 0) { const selected = this.filteredItems[this.selectedIndex]; this.listContainer.addChild(new Spacer(1)); - this.listContainer.addChild(new Text(theme.fg("muted", ` Model Name: ${selected.model.name}`), 0, 0)); + this.listContainer.addChild( + new Text( + theme.fg("muted", ` ${selected.model ? `Model Name: ${selected.model.name}` : "Model unavailable"}`), + 0, + 0, + ), + ); } } @@ -310,7 +325,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl // Toggle provider of current item if (kb.matches(data, "app.models.toggleProvider")) { const item = this.filteredItems[this.selectedIndex]; - if (item) { + if (item?.model) { const provider = item.model.provider; const providerIds = this.allIds.filter((id) => this.modelsById.get(id)!.provider === provider); const allEnabled = providerIds.every((id) => isEnabled(this.enabledIds, id)); diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index d14582c88..de2872dbd 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -76,7 +76,12 @@ import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/ import { configureHttpDispatcher, formatHttpIdleTimeoutMs } from "../../core/http-dispatcher.ts"; import { type AppKeybinding, KeybindingsManager } from "../../core/keybindings.ts"; import { createCompactionSummaryMessage } from "../../core/messages.ts"; -import { defaultModelPerProvider, findExactModelReferenceMatch, resolveModelScope } from "../../core/model-resolver.ts"; +import { + defaultModelPerProvider, + findExactModelReferenceMatch, + resolveModelScope, + resolveModelScopeWithDiagnostics, +} from "../../core/model-resolver.ts"; import { DefaultPackageManager } from "../../core/package-manager.ts"; import type { ResourceDiagnostic } from "../../core/resource-loader.ts"; import { formatMissingSessionCwdPrompt, MissingSessionCwdError } from "../../core/session-cwd.ts"; @@ -4463,14 +4468,20 @@ export class InteractiveMode { // Get all available models await this.session.modelRuntime.refresh(); const allModels = [...(await this.session.modelRuntime.getAvailable())]; + const allModelIds = new Set(allModels.map((model) => `${model.provider}/${model.id}`)); + const configuredPatterns = this.settingsManager.getEnabledModels(); + const sessionScopedModels = this.session.scopedModels; - if (allModels.length === 0) { + if (allModels.length === 0 && !configuredPatterns?.length && sessionScopedModels.length === 0) { this.showStatus("No models available"); return; } + const configuredScope = configuredPatterns?.length + ? await resolveModelScopeWithDiagnostics(configuredPatterns, this.session.modelRuntime) + : undefined; + // Check if session has scoped models (from previous session-only changes or CLI --models) - const sessionScopedModels = this.session.scopedModels; const hasSessionScope = sessionScopedModels.length > 0; // Build enabled model IDs from session state or settings @@ -4479,19 +4490,25 @@ export class InteractiveMode { if (hasSessionScope) { // Use current session's scoped models currentEnabledIds = sessionScopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`); - } else { - // Fall back to settings - const patterns = this.settingsManager.getEnabledModels(); - if (patterns !== undefined && patterns.length > 0) { - const scopedModels = await resolveModelScope(patterns, this.session.modelRuntime); - currentEnabledIds = scopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`); - } + } else if (configuredScope) { + currentEnabledIds = configuredScope.scopedModels.map( + (scoped) => `${scoped.model.provider}/${scoped.model.id}`, + ); + } + + for (const diagnostic of configuredScope?.diagnostics ?? []) { + if (diagnostic.code !== "no-match") continue; + currentEnabledIds ??= []; + if (!currentEnabledIds.includes(diagnostic.pattern)) currentEnabledIds.push(diagnostic.pattern); } // Helper to update session's scoped models (session-only, no persist) const updateSessionModels = async (enabledIds: string[] | null) => { currentEnabledIds = enabledIds === null ? null : [...enabledIds]; - if (enabledIds && enabledIds.length > 0 && enabledIds.length < allModels.length) { + const hasEnabledAvailableModel = enabledIds?.some((id) => allModelIds.has(id)) ?? false; + const allAvailableModelsEnabled = + enabledIds !== null && [...allModelIds].every((id) => enabledIds.includes(id)); + if (enabledIds && hasEnabledAvailableModel && !allAvailableModelsEnabled) { const newScopedModels = await resolveModelScope(enabledIds, this.session.modelRuntime); this.session.setScopedModels( newScopedModels.map((sm) => ({ @@ -4519,10 +4536,11 @@ export class InteractiveMode { }, onPersist: (enabledIds) => { // Persist to settings - const newPatterns = - enabledIds === null || enabledIds.length === allModels.length - ? undefined // All enabled = clear filter - : enabledIds; + const allEnabled = + enabledIds !== null && + enabledIds.length === allModels.length && + enabledIds.every((id) => allModelIds.has(id)); + const newPatterns = enabledIds === null || allEnabled ? undefined : enabledIds; this.settingsManager.setEnabledModels(newPatterns ? [...newPatterns] : undefined); this.showStatus("Model selection saved to settings"); }, diff --git a/packages/coding-agent/test/model-resolver.test.ts b/packages/coding-agent/test/model-resolver.test.ts index f0f8cfb2f..5c54b2d1d 100644 --- a/packages/coding-agent/test/model-resolver.test.ts +++ b/packages/coding-agent/test/model-resolver.test.ts @@ -225,11 +225,13 @@ describe("resolveModelScopeWithDiagnostics", () => { { type: "warning", message: 'Invalid thinking level "invalid" in pattern "gpt-4o:invalid". Using default instead.', + code: "invalid-thinking-level", pattern: "gpt-4o:invalid", }, { type: "warning", message: 'No models match pattern "missing"', + code: "no-match", pattern: "missing", }, ]); diff --git a/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts b/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts new file mode 100644 index 000000000..212235406 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts @@ -0,0 +1,160 @@ +import type { Api, Model } from "@earendil-works/pi-ai"; +import { setKeybindings } from "@earendil-works/pi-tui"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { KeybindingsManager } from "../../../src/core/keybindings.ts"; +import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.ts"; +import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; +import { initTheme } from "../../../src/modes/interactive/theme/theme.ts"; +import { stripAnsi } from "../../../src/utils/ansi.ts"; +import { createHarness, type Harness } from "../harness.ts"; + +function createInteractiveContext(options: { + allModels: Model[]; + enabledModelIds: string[]; + scopedModels?: Array<{ model: Model }>; +}) { + let selector: ScopedModelsSelectorComponent | undefined; + const setScopedModels = vi.fn(); + const getAvailable = vi.fn().mockResolvedValue(options.allModels); + const context = { + session: { + modelRuntime: { + refresh: vi.fn(), + getAvailable, + }, + scopedModels: options.scopedModels ?? [], + setScopedModels, + }, + settingsManager: { + getEnabledModels: () => options.enabledModelIds, + setEnabledModels: vi.fn(), + }, + showStatus: vi.fn(), + showSelector: (factory: (done: () => void) => { component: ScopedModelsSelectorComponent }) => { + selector = factory(() => {}).component; + }, + updateAvailableProviderCount: vi.fn(), + ui: { requestRender: vi.fn() }, + }; + return { context, getAvailable, getSelector: () => selector, setScopedModels }; +} + +async function showModelsSelector(context: object): Promise { + const show = Reflect.get(InteractiveMode.prototype, "showModelsSelector") as (this: object) => Promise; + await show.call(context); +} + +describe("issue #6949 unavailable scoped models", () => { + const harnesses: Harness[] = []; + + beforeAll(() => { + initTheme("dark"); + }); + + beforeEach(() => { + setKeybindings(new KeybindingsManager()); + }); + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("shows and removes an enabled model without a catalog entry", async () => { + const harness = await createHarness({ models: [{ id: "available", name: "Available" }] }); + harnesses.push(harness); + const availableId = `${harness.models[0].provider}/${harness.models[0].id}`; + const unavailableId = `${harness.models[0].provider}/unavailable`; + const changes: Array = []; + const persisted: Array = []; + const selector = new ScopedModelsSelectorComponent( + { + allModels: [...harness.models], + enabledModelIds: [unavailableId, availableId], + }, + { + onChange: (enabledIds) => { + changes.push(enabledIds); + }, + onPersist: (enabledIds) => { + persisted.push(enabledIds); + }, + onCancel: () => {}, + }, + ); + + expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${unavailableId} [unavailable] ✗`); + selector.handleInput("\r"); + expect(changes).toEqual([[availableId]]); + selector.handleInput("\x13"); + expect(persisted).toEqual([[availableId]]); + }); + + it("passes unmatched settings patterns to the selector with one combined resolution", async () => { + const harness = await createHarness({ models: [{ id: "available", name: "Available" }] }); + harnesses.push(harness); + const unavailableIds = ["unavailable-one", "unavailable-two"].map((id) => `${harness.models[0].provider}/${id}`); + const { context, getAvailable, getSelector } = createInteractiveContext({ + allModels: [], + enabledModelIds: unavailableIds, + }); + + await showModelsSelector(context); + + const selector = getSelector(); + if (!selector) throw new Error("Expected scoped-model selector to open"); + const rendered = stripAnsi(selector.render(100).join("\n")); + for (const unavailableId of unavailableIds) { + expect(rendered).toContain(`${unavailableId} [unavailable] ✗`); + } + expect(getAvailable).toHaveBeenCalledTimes(2); + }); + + it("opens when only a session-scoped model is unavailable", async () => { + const harness = await createHarness({ models: [{ id: "unavailable", name: "Unavailable" }] }); + harnesses.push(harness); + const model = harness.models[0]; + const fullId = `${model.provider}/${model.id}`; + const { context, getSelector } = createInteractiveContext({ + allModels: [], + enabledModelIds: [], + scopedModels: [{ model }], + }); + + await showModelsSelector(context); + + const selector = getSelector(); + if (!selector) throw new Error("Expected scoped-model selector to open"); + expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable] ✗`); + }); + + it("does not clear a partial scope when an enabled model is unavailable", async () => { + const harness = await createHarness({ + models: [ + { id: "one", name: "One" }, + { id: "two", name: "Two" }, + { id: "three", name: "Three" }, + ], + }); + harnesses.push(harness); + const [one, two] = harness.models; + const enabledIds = [one, two].map((model) => `${model.provider}/${model.id}`); + const unavailableId = `${one.provider}/unavailable`; + const { context, getSelector, setScopedModels } = createInteractiveContext({ + allModels: [...harness.models], + enabledModelIds: [...enabledIds, unavailableId], + scopedModels: [{ model: one }, { model: two }], + }); + + await showModelsSelector(context); + const selector = getSelector(); + if (!selector) throw new Error("Expected scoped-model selector to open"); + selector.handleInput("\x1b[1;3B"); + + await vi.waitFor(() => { + expect(setScopedModels).toHaveBeenLastCalledWith([ + { model: two, thinkingLevel: undefined }, + { model: one, thinkingLevel: undefined }, + ]); + }); + }); +});