feat: display error on selection of invalid model (#594)

Up-to-date of #78 

Fixes #32

addressed requested changes @tibo-openai :) made sense to me


though, previous rationale with passing the state up was assuming there
could be a future need to have a shared state with all available models
being available to the parent

codex-cli/src/components/chat/terminal-chat.tsx

@@ -31,6 +31,7 @@ import DiffOverlay from "../diff-overlay.js";
 import HelpOverlay from "../help-overlay.js";
 import HistoryOverlay from "../history-overlay.js";
 import ModelOverlay from "../model-overlay.js";
+import chalk from "chalk";
 import { Box, Text } from "ink";
 import { spawn } from "node:child_process";
 import OpenAI from "openai";
@@ -575,7 +576,7 @@ export default function TerminalChat({
             providers={config.providers}
             currentProvider={provider}
             hasLastResponse={Boolean(lastResponseId)}
-            onSelect={(newModel) => {
+            onSelect={(allModels, newModel) => {
               log(
                 "TerminalChat: interruptAgent invoked – calling agent.cancel()",
               );
@@ -585,6 +586,20 @@ export default function TerminalChat({
               agent?.cancel();
               setLoading(false);
 
+              if (!allModels?.includes(newModel)) {
+                // eslint-disable-next-line no-console
+                console.error(
+                  chalk.bold.red(
+                    `Model "${chalk.yellow(
+                      newModel,
+                    )}" is not available for provider "${chalk.yellow(
+                      provider,
+                    )}".`,
+                  ),
+                );
+                return;
+              }
+
               setModel(newModel);
               setLastResponseId((prev) =>
                 prev && newModel !== model ? null : prev,

codex-cli/src/components/model-overlay.tsx

@@ -19,7 +19,7 @@ type Props = {
   currentProvider?: string;
   hasLastResponse: boolean;
   providers?: Record<string, { name: string; baseURL: string; envKey: string }>;
-  onSelect: (model: string) => void;
+  onSelect: (allModels: Array<string>, model: string) => void;
   onSelectProvider?: (provider: string) => void;
   onExit: () => void;
 };
@@ -153,7 +153,12 @@ export default function ModelOverlay({
       }
       initialItems={items}
       currentValue={currentModel}
-      onSelect={onSelect}
+      onSelect={() =>
+        onSelect(
+          items?.map((m) => m.value),
+          currentModel,
+        )
+      }
       onExit={onExit}
     />
   );

codex-cli/tests/terminal-chat-model-selection.test.tsx

@@ -0,0 +1,130 @@
+/* eslint-disable no-console */
+import { renderTui } from "./ui-test-helpers.js";
+import React from "react";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import chalk from "chalk";
+import ModelOverlay from "src/components/model-overlay.js";
+
+// Mock the necessary dependencies
+vi.mock("../src/utils/logger/log.js", () => ({
+  log: vi.fn(),
+}));
+
+vi.mock("chalk", () => ({
+  default: {
+    bold: {
+      red: vi.fn((msg) => `[bold-red]${msg}[/bold-red]`),
+    },
+    yellow: vi.fn((msg) => `[yellow]${msg}[/yellow]`),
+  },
+}));
+
+describe("Model Selection Error Handling", () => {
+  // Create a console.error spy with proper typing
+  let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
+
+  beforeEach(() => {
+    consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    vi.clearAllMocks();
+    consoleErrorSpy.mockRestore();
+  });
+
+  it("should display error with chalk formatting when selecting unavailable model", () => {
+    // Setup
+    const allModels = ["gpt-4", "gpt-3.5-turbo"];
+    const currentModel = "gpt-4";
+    const unavailableModel = "gpt-invalid";
+    const currentProvider = "openai";
+
+    renderTui(
+      <ModelOverlay
+        currentModel={currentModel}
+        providers={{ openai: { name: "OpenAI", baseURL: "", envKey: "test" } }}
+        currentProvider={currentProvider}
+        hasLastResponse={false}
+        onSelect={(models, newModel) => {
+          if (!models?.includes(newModel)) {
+            console.error(
+              chalk.bold.red(
+                `Model "${chalk.yellow(
+                  newModel,
+                )}" is not available for provider "${chalk.yellow(
+                  currentProvider,
+                )}".`,
+              ),
+            );
+            return;
+          }
+        }}
+        onSelectProvider={() => {}}
+        onExit={() => {}}
+      />,
+    );
+
+    const onSelectHandler = vi.fn((models, newModel) => {
+      if (!models?.includes(newModel)) {
+        console.error(
+          chalk.bold.red(
+            `Model "${chalk.yellow(
+              newModel,
+            )}" is not available for provider "${chalk.yellow(
+              currentProvider,
+            )}".`,
+          ),
+        );
+        return;
+      }
+    });
+
+    onSelectHandler(allModels, unavailableModel);
+
+    expect(consoleErrorSpy).toHaveBeenCalled();
+    expect(chalk.bold.red).toHaveBeenCalled();
+    expect(chalk.yellow).toHaveBeenCalledWith(unavailableModel);
+    expect(chalk.yellow).toHaveBeenCalledWith(currentProvider);
+
+    expect(consoleErrorSpy).toHaveBeenCalledWith(
+      `[bold-red]Model "[yellow]${unavailableModel}[/yellow]" is not available for provider "[yellow]${currentProvider}[/yellow]".[/bold-red]`,
+    );
+  });
+
+  it("should not proceed with model change when model is unavailable", () => {
+    const mockSetModel = vi.fn();
+    const mockSetLastResponseId = vi.fn();
+    const mockSaveConfig = vi.fn();
+    const mockSetItems = vi.fn();
+    const mockSetOverlayMode = vi.fn();
+
+    const onSelectHandler = vi.fn((allModels, newModel) => {
+      if (!allModels?.includes(newModel)) {
+        console.error(
+          chalk.bold.red(
+            `Model "${chalk.yellow(
+              newModel,
+            )}" is not available for provider "${chalk.yellow("openai")}".`,
+          ),
+        );
+        return;
+      }
+
+      mockSetModel(newModel);
+      mockSetLastResponseId(null);
+      mockSaveConfig({});
+      mockSetItems((prev: Array<unknown>) => [...prev, {}]);
+      mockSetOverlayMode("none");
+    });
+
+    onSelectHandler(["gpt-4", "gpt-3.5-turbo"], "gpt-invalid");
+
+    expect(mockSetModel).not.toHaveBeenCalled();
+    expect(mockSetLastResponseId).not.toHaveBeenCalled();
+    expect(mockSaveConfig).not.toHaveBeenCalled();
+    expect(mockSetItems).not.toHaveBeenCalled();
+    expect(mockSetOverlayMode).not.toHaveBeenCalled();
+
+    expect(consoleErrorSpy).toHaveBeenCalled();
+  });
+});