Skip to content

🤖 feat: let agents discover selectable models with models_list - #4346

Open
ThomasK33 wants to merge 7 commits into
mainfrom
ThomasK33/models-list
Open

ThomasK33 wants to merge 7 commits into
mainfrom
ThomasK33/models-list

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Sep 22, 2026

Copy link
Copy Markdown
Member

Summary

Add models_list, a read-only tool that returns the visible models selectable under the current configuration, with their accepted aliases and thinking levels. It shares the composer picker's selection pipeline, so agents can discover a valid task.model instead of guessing.

The catalog is advisory. It does not probe providers or grant permission to override the model. task.model still says to omit it unless the user requests a specific model, and send-time validation remains authoritative.

Implementation

  1. Extract the existing picker filters into a shared module. Preserve routing, credentials, authoritative catalogs, OAuth, policy, hidden-model filtering, and UI loading behavior.
  2. Register the zero-argument tool through normal tool assembly. Read current backend configuration on each invocation. Return only model IDs, aliases, and thinking levels; return fixed error text on failure.
  3. Keep the extraction and tool addition in separate commits. Follow them with test-only repairs needed to make the required full gate pass.

Validation

  • Phase A: 42 existing hook tests passed before and after extraction; the broader parity gate passed 2,191 tests.
  • Feature coverage: 399 targeted tests passed. Tests cover selector branches, fresh configuration reads, task input compatibility and forwarding for both task kinds, agent policy exposure, and secret-safe results.
  • Final head 1eb6069124 (rebased onto main 80efaa2ac5 after merge-queue admission failed on 🤖 feat: add GPT-6 Sol and Luna model support #4348's GPT-6 catalog rename): make static-check passed. Full make test-unit passed in 1,173 seconds: 20,089 source tests passed, 9 skipped, and 41 Storybook/DOM tests passed. The follow-up commit only updates selectableModels.test.ts fixtures (renamed GPT_6_LUNA key; gateway catalog entries derived from the moving gpt alias).
  • Local full tests used pinned Bun 1.3.5 and process-only GIT_TEMPLATE_DIR=/usr/share/git-core/templates to avoid host-specific Git templates. No test-runner flags, timeout increases, or skipped coverage were added.
  • Remote Coder UAT passed on 31d4e64ab (the recorded/tested SHA; not a rerun on the final head): picker/tool parity; tool availability in exec, plan, explore and sub-agents; fixture-backed launches for both task kinds; disabled-provider negative checks; and desktop/mobile rendering. Requests went to controlled loopback fixtures, not real providers.
  • The mobile recording shows the actual expanded tool card at 390×844 with the sidebar drawer closed, including both model entries after scrolling the result pane. The turn was submitted through the production CLI; this does not validate the mobile Send button. Three earlier evidence attempts were retained; the first round-two video remains disqualified because its launcher did not meet the environment-allowlist requirement. The separately authorized final attempt met the original criterion without source changes.
  • Remote UAT chat. Screenshots, sequentially decoded recording frames, request transcripts, and artifact hashes were independently inspected.
Test-gate repairs and plan corrections

The full run exposed test isolation leaks. The separate repair commit restores Dialog/Tooltip module mocks, uses the shared DOM teardown harness, and targets filesystem error injection at the intended path. Ordered reproductions for the DOM and module-mock failures went from red to green; the final full run passed.

CI then exposed two moving-model fixture assumptions after the base branch promoted the Opus alias to 5.5. Reproducing the exact CI merge tree produced both failures. The final test-only commit derives the gateway allowlist from model metadata and verifies enrichment against the authoritative thinking policy, while keeping fixed-model capability expectations explicit. All 270 selector, AI-service, and thinking-policy tests pass on both the feature tree and repaired CI merge tree. Only two test files differ from the UAT-tested commit; all other tracked files, including product code and dependencies, are identical. Following the user's instruction to continue (standing delegation), the existing UAT evidence was accepted as carried forward to this test-only commit on the basis of the verified two-test-file delta and green final-head validation. The evidence retains its actual tested SHA (31d4e64ab); no extra recording was made.

The retained Storybook budget had fallen behind the current inventory. Its separate repair sets zero-headroom limits at 115 enabled files and 605 estimated snapshots, without removing coverage. An audited inventory and disposable extra-file/extra-export mutations verify both guards still fail on growth. These are estimates, not executed capture counts.

Three plan assumptions were corrected against existing behavior: the hook-doc generator omits zero-input tools, so only the task.model hint changes; custom IDs are trimmed before selection; internal colons are valid model-ID characters. The generator and task normalization behavior are unchanged.

Risks

The shared selection code is on the composer hot path. The extraction preserves its behavior and has explicit parity coverage. Backend discovery reads persisted hidden-model preferences, so it can briefly differ from the UI immediately after a preference change. The catalog is not a promise that a provider will accept a request.

Mobile UAT evidence

Expanded models_list tool card at 390 pixels

Continuous mobile recording: expand the card and scroll both model entries.

36306eae-mobile-expanded-r4.webm

📋 Implementation Plan

Plan: models_list tool — let agents discover valid model values for task

Goal

Add a read-only agent tool, models_list, that returns the models selectable under the user's current configuration (the same set the chat composer's model picker offers), each with the aliases and thinking levels the task tool accepts. Agents can then pass a valid model/thinking to task (both kind: "subagent" and kind: "workspace") instead of guessing. The list is computed by one shared function used by both the UI picker and the tool, so the two cannot drift.

The catalog is advisory: it reflects configuration (credentials present, routing, policy, user-hidden models), not a guarantee that a provider will accept a request. Send-time validation in providerModelFactory stays authoritative. It is also a visible catalog: user-hidden models are omitted even though task.model would still accept them.

Evidence (verified in repo)

  • No backend code computes "selectable models" today. The only implementation is the React hook src/browser/hooks/useModelsFromSettings.ts (useMemo ~L347–418): dedupe(custom models of enabled providers, KNOWN_MODELS) → minus hiddenModels → authoritative-catalog + route availability (isModelAvailable) → OpenAI Codex-OAuth gating on the direct route → policy on the active route. The composer picker (src/browser/features/ChatInput/index.tsx:2941–2959ModelSelector) renders useModelsFromSettings().models as-is (no sorting, no "recent" entries); Settings → Models shows inventory tables, not this list. Parity reference = useModelsFromSettings().models.
  • task.model is validated syntactically only, for both kinds: parseTaskAiOverrides (src/node/services/tools/task.ts:120–147, called at L419 before the kind branch) → normalizeModelInput (src/common/utils/ai/normalizeModelInput.ts) → MODEL_ABBREVIATIONS alias table (src/common/constants/knownModels.ts:307) → normalizeSelectedModel (preserves explicit gateway prefixes) → isValidModelFormat. A well-formed but unavailable model passes creation and fails at first send (api_key_not_found, model_not_available, policy_denied, …), surfacing as a failed task call or an interrupted task.
  • Reusable helpers already in src/common: resolveRoute/isModelAvailable (src/common/routing/resolve.ts:280,305), isProviderModelAccessibleFromAuthoritativeCatalog/isGatewayModelAccessibleFromAuthoritativeCatalog (src/common/utils/providers/gatewayModelCatalog.ts), isCodexOauthAllowedModel/isCodexOauthRequiredModel (src/common/constants/codexOAuth.ts), getThinkingPolicyForModel(model, providersConfig?: ProvidersConfigMap | null): readonly ThinkingLevel[] (src/common/utils/thinking/policy.ts:108, honors mappedToModel), normalizeModelInput, KNOWN_MODELS, DEFAULT_HIDDEN_MODELS, MODEL_ABBREVIATIONS.
  • Browser-only helpers the shared pipeline needs: parseModelString, isModelAllowedByPolicy, isGatewayModelAccessibleForUi in src/browser/utils/policyUi.ts (imports are all @/common; 10 importers incl. src/node/services/providerService.test.ts), plus private helpers in the hook (getCustomModels, getSuggestedModels, filterHiddenModels, dedupeKeepFirst, resolvesToDirectOpenAI, isModelAllowedByPolicyOnActiveRoute; none imported elsewhere).
  • UI inputs and their backend sources: providersConfig = api.providers.getConfig()providerService.getConfig() (ProvidersConfigMap); routePriority/routeOverrides = config.getConfig() with UI defaults ["direct"] / {} (src/browser/hooks/useRouting.ts:19–21,82); hiddenModels = persisted browser state seeded from backend model prefs on mount (src/browser/contexts/WorkspaceContext.tsx:683–705), default DEFAULT_HIDDEN_MODELS; effectivePolicy = usePolicy() when enforced.
  • Backend has the same inputs where tools are assembled: TurnRequestBuilderDependencies (src/node/services/turnRequestBuilder.ts:585–600) has config: Config, providerService: ProviderService, policyService?: PolicyService. Config.loadConfigOrDefault() (src/node/config/index.ts:1456) returns ProjectsConfig (src/common/types/project.ts:79; fields hiddenModels?, routePriority?, routeOverrides?) from a warm in-memory snapshot; it is the accessor every backend read uses (there is no separate getConfig()), and its one-time migration persistence is pre-existing behavior shared by all reads — the tool adds no write path.
  • Single production wiring point: TurnRequestBuilder builds ToolConfiguration at L2247 and calls getToolsForModel at L2518. CLI xum run, ACP, workflows, and the PTC bridge all flow through it (src/cli/run.ts:651–684, src/node/acp/agent.ts:121, src/node/services/workflowContinuation.ts:28, src/node/services/ptc/toolBridge.ts:43 receives already-assembled tools). The only other ToolConfiguration construction (src/node/services/refinement/refineService.ts:1476–1488) builds skill-write tools only and never calls getToolsForModel.
  • Tool plumbing: schema in TOOL_DEFINITIONS (src/common/utils/tools/toolDefinitions.ts, optional resultSchema), arg/result types in src/common/types/tools.ts, implementation src/node/services/tools/<name>.ts (ToolFactory = (config: ToolConfiguration) => Tool), registration in nonRuntimeTools (src/common/utils/tools/tools.ts ~L871), gating in getAvailableTools() baseTools (~L3770). Built-in agents exec.md/plan.md/explore.md use tools.add: [".*"] with explicit removes, so a new tool reaches exec/plan/explore and sub-agents without agent-definition edits (SUBAGENT_HARD_DENY only denies ask_user_question; depth denial covers task* only). UI falls back to GenericToolCall; icon via TOOL_NAME_TO_ICON (src/browser/features/Tools/Shared/ToolPrimitives.tsx:244). docs/hooks/tools.mdx is regenerated from TOOL_DEFINITIONS by bun scripts/gen_docs.ts (make fmt). No test asserts a total tool count or schema token budget.
  • Template: agent_skill_list (src/node/services/tools/agent_skill_list.ts:158): { success: true, … } | { success: false, error }, .strict() schema, .nullish() optionals.
  • Existing test patterns for real assembly/gating: src/common/utils/tools/toolDefinitions.test.ts:683–753 (getAvailableTools), src/common/utils/tools/tools.test.ts (getToolsForModel), src/node/services/toolAssembly.test.ts (applyToolPolicyAndExperiments + resolveToolPolicyForAgent for built-in agents).

Design decisions

  1. Name: models_list (plain snake_case like task_list, agent_skill_list; mux_ prefix is reserved for config/agent-store tools).
  2. Input: z.object({}).strict() — no parameters, no includeHidden option.
  3. Output (ModelsListToolResultSchema, also attached as resultSchema). The domain type lives in the shared module; the schema only mirrors it (type-only dependency, no cycle):
    // src/common/utils/ai/selectableModels.ts
    export interface AvailableModel {
      model: string;                  // normalized selection ID accepted by task.model (explicit gateway prefixes preserved)
      aliases: string[];              // MODEL_ABBREVIATIONS keys that normalize to `model` (e.g. ["sonnet"]); [] for custom models
      thinkingLevels: ThinkingLevel[]; // Xum's thinking policy for this model (getThinkingPolicyForModel), not a provider capability probe
    }
    // src/common/utils/tools/toolDefinitions.ts
    const AvailableModelSchema = z.object({
      model: z.string(), aliases: z.array(z.string()), thinkingLevels: z.array(ThinkingLevelSchema), // ThinkingLevelSchema exists in src/common/types/thinking.ts
    }).strict() satisfies z.ZodType<AvailableModel>;
    const ModelsListToolResultSchema = z.discriminatedUnion("success", [
      z.object({ success: z.literal(true), models: z.array(AvailableModelSchema) }).strict(),
      z.object({ success: z.literal(false), error: z.string() }).strict(),
    ]);
    getThinkingPolicyForModel returns a readonly array → spread into a mutable array when building the entry. Nothing else (no display names, context windows, pricing, defaultModel, credentials, provider config).
  4. Semantics = composer-picker parity, defined as "valid, normalized, selectable picker entries". computeSelectableModels reproduces the hook pipeline exactly (including the providersConfig == null "still loading → skip availability filters" branch, used only by the UI) and returns raw picker strings. The enrichment step then, for each raw entry raw with selectable = new Set(rawList):
    • n = normalizeModelInput(raw).model; if null (e.g. a persisted custom ID with a second colon) → omit (persisted custom IDs are input data, not programmer invariants);
    • if n !== raw (normalization changed the identity — normalizeModelInput trims and canonicalizes; explicit gateway selections such as openrouter:… are preserved by normalizeSelectedModel, so in practice this is e.g. a persisted custom entry with surrounding whitespace) → emit n only if selectable.has(n), i.e. the emitted identity itself passed routing, credential, catalog, policy and hidden-model checks; otherwise omit rather than advertise an unchecked replacement;
    • dedupe after normalization (two raw entries normalizing to the same ID collapse into one);
    • aliases = MODEL_ABBREVIATIONS keys with normalizeModelInput(alias).model === n.
      Omissions are reported through an optional onSkipped?: (raw: string, reason: "malformed" | "unchecked_identity") => void callback so the Node boundary can log.debug; the shared module itself imports no logger.
  5. Single source of truth. The hook and the tool both call computeSelectableModels; the hook's own code shrinks to input plumbing.
  6. Service access: one optional closure on ToolConfiguration, listAvailableModels?: () => AvailableModel[], built in TurnRequestBuilder at the existing construction site. The closure reads providerService.getConfig(), config.loadConfigOrDefault(), and the effective policy (policyService?.isEnforced() ? policyService.getEffectivePolicy() : null) at each invocation — no cache, no new service class, no PolicyService passed into shared code. Absent closure (test contexts, refineService) → { success: false, error: "Model catalog unavailable in this context" } (mirrors task without taskService). A throwing closure → fixed message { success: false, error: "Failed to compute the model catalog" } with the exception logged via log.error on the Node side — exception text (which could echo configuration) never reaches the model.
  7. Backend config is the source even for SSH/remote workspaces (models are resolved on the Xum host, not in the workspace runtime); the tool never probes providers.
  8. No agent-definition changes; no ptcExcluded; generic CLI formatter and UI renderer; one icon line.
  9. Prompt cost: description ≤ ~60 words; it is added to every request's tool schema.
  10. Discovery vs. override permission are separate. models_list may be called whenever the agent needs to know which models exist (e.g. the user asks "which models are available?"). The override restriction stays where it is: the task.model description keeps its "omit unless the user explicitly instructed a specific model" sentence verbatim and gains only the hint "Use models_list to see valid values."

Delivery: one change set, two gated phases

Work lands on this branch as two commits (Phase A, Phase B). Publication (PR, gh stack split) only when requested; the phases are independently reviewable if a split is wanted later.

Phase A — pure refactor, zero behavior change: shared selectable-models pipeline

  1. Create src/common/utils/policy/modelPolicy.ts with parseModelString, isModelAllowedByPolicy, isGatewayModelAccessibleForUi moved verbatim from src/browser/utils/policyUi.ts; policyUi.ts re-exports them so its 10 importers and policyUi.test.ts are untouched (getAllowedProvidersForUi stays in the browser file).
    → verify: make typecheck; bun test src/browser/utils/policyUi.test.ts.
  2. Move DEFAULT_ROUTE_PRIORITY (["direct"]) from useRouting.ts to src/common/routing/resolve.ts (reuse an existing direct-route constant there if one exists) so UI and backend default identically; useRouting.ts imports it.
  3. Create src/common/utils/ai/selectableModels.ts:
    export interface SelectableModelsInput {
      providersConfig: ProvidersConfigMap | null; // null = still loading (UI only): skip availability filters
      hiddenModels: string[];
      effectivePolicy: EffectivePolicy | null;    // null = policy not enforced
      routePriority: string[];                    // matches resolveRoute/isModelAvailable signatures (not readonly)
      routeOverrides: Record<string, string>;
    }
    export function computeSelectableModels(input: SelectableModelsInput): string[];
    Move BUILT_IN_MODELS, getCustomModels, getSuggestedModels, filterHiddenModels, dedupeKeepFirst, resolvesToDirectOpenAI, isModelAllowedByPolicyOnActiveRoute, and the isConfigured / isGatewayModelAccessible / isAuthoritativeProviderModelAccessible predicates from the hook into this file without logic changes (predicates become plain closures over providersConfig/effectivePolicy). Export getSuggestedModels/filterHiddenModels only if the hook still needs them (getAllCustomModels/providerHiddenModels bucket stays in the hook).
  4. useModelsFromSettings: replace the useMemo body with computeSelectableModels({ providersConfig: config, hiddenModels, effectivePolicy, routePriority, routeOverrides }); keep providerHiddenModels / ensureModelInSettings logic in place, reusing the moved predicates where it needs them.
  5. Guard: rg '@/browser' src/common stays empty (lint no-restricted-imports boundary also enforces this).

Gate A (must pass before Phase B): make typecheck && make lint && bun test src/browser/hooks/useModelsFromSettings.test.ts src/browser/utils src/common/utils/ai src/common/routing — the 40 existing hook tests run before (baseline = this branch's pre-change HEAD 9a6a2e2bf, recorded before the first edit) and after the extraction with identical results.

Tests added in Phase A — src/common/utils/ai/selectableModels.test.ts, each with an explicit expected list (never "compare two calls"):

  • custom models: included only from enabled providers; mux-gateway and github-copilot entries skipped; a custom entry equal to a built-in ID deduped (custom first); object entries ({ id, mappedToModel }) surface by id.
  • hidden: model in hiddenModels excluded; empty list is a no-op.
  • route availability: built-in model of an unconfigured/disabled provider excluded; same model included when a configured gateway (openrouter / coder) can route it; routeOverrides pinning a model to an unavailable gateway falls back per resolveRoute (assert the actual isModelAvailable outcome); explicit gateway-prefixed IDs (coder:openai/gpt-…) survive.
  • authoritative catalogs: coder discoveredModels present/absent and removedModels exclusion; github-copilot catalog gating of a built-in model.
  • Codex OAuth on the direct OpenAI route: key+oauth, key-only, oauth-only, neither × required/allowed model; a gateway-routed OpenAI model is not gated.
  • policy: model denied under canonical identity but allowed via its active gateway route is included; denied on the active route excluded; effectivePolicy: null skips filtering.
  • providersConfig: null: returns the hidden-filtered suggested list (loading semantics), still policy-filtered when a policy is present.
    (Move equivalent cases out of useModelsFromSettings.test.ts only when they test pure pipeline logic; leave cases that exercise React state, persistence, or ensureModelInSettings.)

Phase B — the tool

  1. src/common/utils/tools/toolDefinitions.ts
    • Add AvailableModelSchema, ModelsListToolResultSchema (Design §3) near the other result schemas (import ThinkingLevelSchema from src/common/types/thinking.ts; import type { AvailableModel } from the shared module).
    • Add models_list: { description, schema: z.object({}).strict(), resultSchema: ModelsListToolResultSchema }. Description: "List models selectable under the current configuration, with aliases and thinking levels. Hidden models are omitted. This is a configuration snapshot, not a provider availability probe. Use returned IDs when a model override is requested; otherwise leave task.model unset."
    • Add "models_list" to baseTools in getAvailableTools().
    • Append "Use models_list to see valid values." to the task model description.
  2. src/common/types/tools.ts: ModelsListToolArgs, ModelsListToolResult (z.infer); re-export type AvailableModel from the shared module for tool consumers.
  3. src/common/utils/ai/selectableModels.ts: add the pure enrichment step (Design §4)
    export function listAvailableModels(
      input: SelectableModelsInput & { providersConfig: ProvidersConfigMap },
      onSkipped?: (raw: string, reason: "malformed" | "unchecked_identity") => void
    ): AvailableModel[]
    = computeSelectableModels → normalize / recheck / dedupe per §4 → aliasesthinkingLevels = [...getThinkingPolicyForModel(model, providersConfig)]. assert(thinkingLevels.length > 0) (the policy helper always returns a non-empty fallback; this documents the assumption). No logger import; no runtime import of tool-definition modules.
  4. src/common/utils/tools/tools.ts: add listAvailableModels?: () => AvailableModel[] to ToolConfiguration with a doc comment (import type); register models_list: createModelsListTool(config) in nonRuntimeTools.
  5. src/node/services/turnRequestBuilder.ts (~L2247): wire the closure:
    listAvailableModels: () => {
      const appConfig = this.dependencies.config.loadConfigOrDefault();
      const policy = this.dependencies.policyService;
      return listAvailableModels(
        {
          providersConfig: this.dependencies.providerService.getConfig(),
          hiddenModels: appConfig.hiddenModels ?? [...DEFAULT_HIDDEN_MODELS],
          routePriority: appConfig.routePriority ?? [...DEFAULT_ROUTE_PRIORITY],
          routeOverrides: appConfig.routeOverrides ?? {},
          effectivePolicy: policy?.isEnforced() ? policy.getEffectivePolicy() : null,
        },
        (raw, reason) => log.debug(`[models_list] skipped ${raw}: ${reason}`)
      );
    },
  6. src/node/services/tools/models_list.ts (new): createModelsListTool: ToolFactoryconfig.listAvailableModels == null{ success: false, error: "Model catalog unavailable in this context" }; else call it inside try/catch; on throw log.error(...) and return the fixed { success: false, error: "Failed to compute the model catalog" }; otherwise { success: true, models } (an initialized configuration with nothing selectable yields models: [], still success: true).
  7. src/browser/features/Tools/Shared/ToolPrimitives.tsx: models_list: <lucide icon already imported there, e.g. Cpu/Boxes> in TOOL_NAME_TO_ICON.
  8. make fmt → commit the regenerated docs/hooks/tools.mdx row.

Tests added in Phase B:

  • src/common/utils/ai/selectableModels.test.ts (extend): listAvailableModels with a fixture ProvidersConfigMap (anthropic configured, openai unconfigured, openrouter configured with custom entry anthropic/claude-sonnet-5, custom fixture provider with ["fixture-echo", "fixture-echo " (trailing space), { id: "fixture-mapped", mappedToModel: "anthropic:claude-opus-4-7" }, "bad:id:colon", "lonely "], one hidden built-in) → explicit expected entries: aliases only on the built-in they normalize to (sonnetanthropic:claude-sonnet-5), [] on custom models; malformed fixture:bad:id:colon omitted (onSkipped called with "malformed") while the rest survive; changed-identity regression (an input demonstrably changed by normalizeModelInput): fixture:fixture-echo normalizes to fixture:fixture-echo, which is independently selectable → one deduped entry; fixture:lonely normalizes to fixture:lonely, which is not in the selectable set → omitted with "unchecked_identity"; explicit gateway selection preserved: openrouter:anthropic/claude-sonnet-5 is emitted unchanged (not collapsed into the canonical ID) — assert the actual normalizeModelInput output rather than assuming; fixture-mapped gets the mapped target's thinking policy; a model whose policy excludes off vs one on the default policy; every entry passes AvailableModelSchema.strict().parse. Do not change task normalization to make a fixture pass; adjust expectations to observed behavior.
  • src/node/services/tools/models_list.test.ts: (a) no closure → { success: false, error: "Model catalog unavailable in this context" }; (b) closure result returned and ModelsListToolResultSchema.parse succeeds; (c) closure throws an error whose message contains a fixture secret → result is the fixed failure message and does not contain the secret; (d) closure returns []{ success: true, models: [] }; (e) task-input compatibility: for every returned entry, parseTaskAiOverrides({ model, thinking }) accepts model, each alias, and each thinkingLevel; (f) serialized success result contains none of the fixture's secret strings (apiKey, baseUrl, header values).
  • Production closure test in src/node/services/turnRequestBuilder coverage (pattern: aiService.test.ts getToolsForModelSpy / workspaceService.multiProject.test.ts capturedToolConfig): capture the real ToolConfiguration, assert listAvailableModels is present, call it with fake providerService.getConfig(), config.loadConfigOrDefault(), and policyService whose return values are mutated between invocations (provider configured→disabled, routePriority gateway added, a model added to hiddenModels, policy enforced→denying) and assert each subsequent call returns the explicit expected list — the tool is instantiated once, the closure recomputes.
  • Task-handler forwarding test in src/node/services/tools/task.test.ts (existing createTaskTool harness with stubbed taskService / workspaceTurnManager): call the public task handler with a models_list-shaped entry (model = canonical ID, then its alias) and a listed thinking level for kind: "subagent" and kind: "workspace" (mode: "new"), asserting the stubs receive the normalized modelString and resolved thinkingLevel. No new full-stack harness.
  • Assembly/gating: in src/node/services/toolAssembly.test.ts (existing resolveToolPolicyForAgent pattern) assert models_list survives the exec, plan, and explore policies and the sub-agent hard-deny; extend toolDefinitions.test.ts:683–753 if it enumerates exposed base tools.

Gate B: new tests green; make static-check; full make test-unit (the repo's required gate — no touched-suite substitute; if the known Bun 1.3.5 Wasm SIGSEGV appears, rerun with the documented invocation-scoped BUN_JSC_useWasmIPInt=0 and report it); then the hands-on dogfood below.

Files touched (product code)

File Change Net new LoC (moved code excluded)
src/common/utils/policy/modelPolicy.ts (new) + src/browser/utils/policyUi.ts move 3 helpers, re-export +3
src/common/routing/resolve.ts + src/browser/hooks/useRouting.ts shared DEFAULT_ROUTE_PRIORITY +1
src/common/utils/ai/selectableModels.ts (new) computeSelectableModels (moved, not counted) + AvailableModel + listAvailableModels with normalize/recheck/dedupe/onSkipped (new) +60
src/browser/hooks/useModelsFromSettings.ts replace moved pipeline with one call (moved deletions not counted) +6
src/common/utils/tools/toolDefinitions.ts schemas, tool entry, baseTools, task hint +32
src/common/types/tools.ts types +6
src/common/utils/tools/tools.ts ToolConfiguration.listAvailableModels, registration +6
src/node/services/turnRequestBuilder.ts closure + skip logging +18
src/node/services/tools/models_list.ts (new) tool with fixed error messages +35
src/browser/features/Tools/Shared/ToolPrimitives.tsx icon +1
docs/hooks/tools.mdx generated n/a

Estimated net new product LoC: ≈ +170 (range 150–200), excluding moved code, tests (≈ +350), and generated docs.

Acceptance criteria

  1. models_list is exposed through the real assembly path for exec, plan, explore, and sub-agents (test + dogfood), and returns { success: true, models } with model, aliases, thinkingLevels per entry; { success: false, error } where no catalog closure exists.
  2. For the same backend state, the tool's model set equals the valid, normalized, selectable entries of the settled composer picker (computeSelectableModels is the shared implementation; malformed picker entries and changed identities that are not themselves selectable are the only omissions; dogfood compares the rendered picker with the tool output).
  3. Task-input compatibility: every returned model, alias, and thinking level is accepted by parseTaskAiOverrides (unit test), and launching task with a returned model succeeds against controlled loopback fixtures for both kind: "subagent" and kind: "workspace" (dogfood). Real-provider acceptance is not claimed.
  4. Hidden models, direct-only models of unconfigured/disabled providers, authoritative-catalog-removed gateway models, and policy-denied models are absent; a gateway-routable model appears when its gateway is configured. Malformed persisted custom IDs are skipped, not fatal.
  5. Neither success nor failure output contains credentials or provider configuration: strict schemas bound the shape, string fields carry only model IDs / aliases / level names, and failure messages are fixed constants (tests c, f).
  6. useModelsFromSettings.test.ts results are identical before/after Phase A; make static-check and full make test-unit pass on the final commit; docs/hooks/tools.mdx gained exactly the models_list row.

Dogfooding (deterministic, loopback fixtures by default)

No real provider key is used unless the user explicitly authorizes it. Everything runs against an owned temp XUM_ROOT and a loopback OpenAI-compatible SSE fixture; every request must reach only that fixture.

  1. Isolated environment. Read the dev-server-sandbox skill. Start the dev server under an allowlisted environment (env -i PATH=… HOME=… XUM_ROOT=<temp> … plus only the variables the sandbox script needs) so no inherited ANTHROPIC_*/OPENAI_*/Coder gateway credential can make a real provider "configured". Seed providers.jsonc with two custom openai-compatible providers: fixture (baseUrl = loopback fixture, apiKey: "fixture", models: ["fixture-driver", "fixture-echo", "fixture-hidden"]) and fixture-off (same baseUrl, isEnabled: false, models: ["never"]); config.json hiddenModels: ["fixture:fixture-hidden"], default model fixture:fixture-driver. Before sending anything, verify the effective snapshot with xum api providers get-config (only fixture is isConfigured && isEnabled; no built-in provider configured) and xum api config get-config (routePriority ["direct"]). Keep fixture scripts, logs, and artifacts outside the checkout.
  2. Fixture script (state-driven, not history-substring). The fixture dispatches on the latest message of each request (role + tool-call/tool-result state) and on the requested model:
    • parent, latest user message = discovery prompt → tool call models_list {}; latest = tool result of models_list → text turn echoing the result JSON, then stop.
    • parent, latest user message = spawn prompt → tool call task { agentId: "explore", title: "Echo", prompt: "reply ok", model: "fixture:fixture-echo", thinking: "low" } (second variant: kind: "workspace", workspace: { mode: "new" }); latest = tool result of task → text summary, stop.
    • child / workspace turn (model fixture-echo, latest user message contains "reply ok") → text ok (sub-agents auto-report after the first turn; the workspace turn completes on the text).
    • parent, latest user message = negative prompt → tool call task { …, model: "fixture-off:never" }; latest = tool result (error) → text summary, stop.
    • anything else → a fixed text unexpected request, logged with the request body, so a loop is visible immediately.
      The fixture logs every request (model, latest message kind) to a file that is checked at the end: only expected shapes, no repeats.
  3. Open the app with agent-browser under an owned --session <name> (never close --all); agent-browser record start (ffmpeg present) before the steps below.
  4. Open the composer model picker (settled, no search text); agent-browser snapshot -i to capture the rendered entries; screenshot → picker.png. Expected: fixture:fixture-driver, fixture:fixture-echo; no built-ins, no fixture-off model, no hidden model.
  5. Send the discovery prompt ("Call the models_list tool and paste the JSON result verbatim, then stop."). Expand the tool card; snapshot + screenshot → models_list.png. Expected set equals step 4; aliases: []; thinkingLevels = default policy.
  6. Send the spawn prompt (sub-agent) → child starts, replies ok, reports → task_subagent.png. Send the workspace-kind spawn prompt → new workspace turn completes with oktask_workspace.png. Send the negative prompt → task fails at launch with provider_disabled for fixture-off:nevertask_negative.png (a model the catalog correctly omitted; no real provider is involved).
  7. 390 px check (required): resize the viewport to 390×844, close the sidebar drawer (tap backdrop), expand the tool card → models_list_mobile.png; assert via agent-browser eval that document.documentElement.scrollWidth <= window.innerWidth (long IDs wrap/truncate, no right-edge overflow).
  8. Stop the recording → dogfood.webm; inspect frames with ffmpeg -vf fps=1 to confirm steps 4–7 are visible; confirm from the fixture log that every request hit the fixture and matched an expected shape; attach_file the PNGs and video in the report.
  9. Cleanup: stop the dev server and fixture processes you started, agent-browser close --session <name>, confirm no owned PIDs/ports remain, delete the temp XUM_ROOT only if it holds nothing needed for the report.

Risks and accepted trade-offs

  • Refactor of a hot UI path (Phase A): mitigated by moving code without logic changes, baseline-vs-after runs of the 40 hook tests, and explicit-expectation tests for the pure function.
  • Hidden-models source: UI reads persisted browser state seeded from config.json; backend reads config.json. Both are kept in sync by updateModelPreferences; a transient mismatch right after hide/unhide is acceptable.
  • Advisory list: task.model still accepts any well-formed string; validating task.model against the catalog is out of scope.
  • Config read side effects: loadConfigOrDefault() may persist one-time migrations, but that is existing behavior of every backend read; the tool introduces no new writes.
  • Real-launch coverage is hands-on, not a new automated real-stack suite: adding a fixture-backed end-to-end test harness is heavier than the feature; unit tests cover input compatibility, task-handler forwarding for both kinds, and production wiring; dogfood covers actual launches.
  • Changed-identity omission: a picker entry whose normalized form differs and is not itself selectable (e.g. a whitespace-padded custom ID with no clean counterpart) is omitted rather than emitted in either form. Conservative reading of parity; the picker still shows the raw entry. Acceptable because task would rewrite the raw form anyway and such entries indicate a configuration typo.
  • Token cost: one more tool schema in every request; kept minimal.

Out of scope / follow-ups

  • defaultModel, display names, context windows, or cost data in the output; a dedicated tool card or CLI formatter.
  • Rejecting unavailable task.model values at task creation.
  • Exposing the catalog over oRPC/CLI (xum api models list).

Generated with xum • Model: coder:openai/gpt-6-astra • Thinking: high • Cost: $171.28

@mintlify

mintlify Bot commented Sep 22, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
Mux 🟢 Ready View Preview Sep 23, 2026, 8:42 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-23T09:05:22.045715Z 593f879 New commits
🔒 Security Review Completed 2026-09-23T09:07:33.479686Z 593f879 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 31d4e64ab0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 31d4e64ab0

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33

Copy link
Copy Markdown
Member Author

Final validation checkpoint — not merged

  1. Final head: 530d9ebd3d015b80fe0fca9b92cb64760d11a143. Local static checks and the full unit gate passed (20,047 source tests, 9 skips; 41 UI tests). The 270 targeted tests passed on both the feature tree and repaired CI merge tree.
  2. All required CI checks are green. One unchanged-head retry of Integration and Required passed. The first attempt's live-model MCP assertion failure is preserved; it did not recur, but its root cause is unproven. No source change or local live-provider invocation was used for the retry.
  3. Exact-head Codex Code and Security reviews completed clean. The current-head board and the later approval reaction were verified independently of the helper's older approval comment. There are zero review threads, zero unresolved findings, and no review work in flight. Combined public review count: 6/6; no further review triggers were sent.
  4. The clean-context advisory recommendation is conditional readiness. The remaining acceptance decision is whether to carry forward remote UAT from 31d4e64ab0d050ef6d4782d7eee9122a3f097135: the final commit changes only two test files, and every other tracked file, including product code and dependencies, is byte-identical. The screenshots and video retain their actual tested SHA. No fifth UAT attempt was made.

Pausing for explicit acceptance of that UAT carry-forward. No merge was performed. All owned monitoring processes have ended.


Generated with xum • Model: coder:openai/gpt-6-astra • Thinking: high • Cost: $230.67

@ThomasK33

Copy link
Copy Markdown
Member Author

Readiness record — ready, not merged

Following the instruction to continue, I accepted carrying the remote UAT evidence forward across the test-only commit. The evidence retains its actual tested SHA, 31d4e64ab; it was not re-executed on 530d9ebd3d. Basis: only src/common/utils/ai/selectableModels.test.ts and src/node/services/aiService.test.ts differ, and all product and dependency files are identical.

Re-verified just now on 530d9ebd3d: required CI green, Codex Code and Security reviews completed clean on this head, zero review threads, review count 6/6. No further UAT, reviews, or merge.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $236.49

@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 23, 2026
@ThomasK33
ThomasK33 force-pushed the ThomasK33/models-list branch from 530d9eb to 1eb6069 Compare September 23, 2026 06:33
@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 23, 2026
@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 23, 2026
Pure refactor with no behavior change, preparing a backend models_list tool
that must offer exactly what the composer picker offers.

- Move parseModelString / isModelAllowedByPolicy / isGatewayModelAccessibleForUi
  verbatim to src/common/utils/policy/modelPolicy.ts; policyUi.ts re-exports.
- Share DEFAULT_ROUTE_PRIORITY from src/common/routing so UI and backend default
  identically.
- Move the useModelsFromSettings selector pipeline (custom models, hidden filter,
  authoritative catalogs, route availability, direct-route Codex OAuth gating,
  policy on the active route) into computeSelectableModels in
  src/common/utils/ai/selectableModels.ts; the hook now calls it.
- Add explicit-expectation tests for the pure pipeline.

The 42 existing useModelsFromSettings tests pass unchanged before and after.

---

_Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$unknown`_

<!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=unknown -->
…values

Read-only agent tool returning the models selectable under the current
configuration — the same set the composer picker offers — each with the
aliases and thinking levels the task tool accepts, so agents can pass a valid
model/thinking to task instead of guessing.

- computeSelectableModels (shared with the picker) → listAvailableModels:
  normalize via normalizeModelInput, drop malformed persisted IDs, re-check
  changed identities against the selectable set, dedupe, attach
  MODEL_ABBREVIATIONS aliases and getThinkingPolicyForModel levels.
- Strict input (no parameters) and strict result schemas; failure messages are
  fixed constants so exception text never reaches the model.
- ToolConfiguration.listAvailableModels closure wired in TurnRequestBuilder,
  reading providerService.getConfig(), config.loadConfigOrDefault() and the
  enforced policy on every call (no cache).
- Exposed through baseTools; the task.model description gains a hint to use
  models_list. Generated docs/skill snapshot regenerated via the repo scripts.
- Tests: pipeline enrichment, tool result/leak/failure paths, task-input
  compatibility (parseTaskAiOverrides), both task kinds forwarding, exec/plan/
  explore + sub-agent gating, and the production closure recomputing across
  provider/config/policy mutations.

Checkpoint commit: the full make test-unit gate was red on unrelated
pre-existing host failures at this point; not accepted as validated.

_Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$9.23`_

<!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=9.23 -->
Main now selects GROK_47 instead of GROK_46. Keep the explicit provider fixture
aligned with that catalog entry after rebasing; no selection behavior changes.

Validation: 399 targeted tests, typecheck, ESLint, and Prettier pass.

---

_Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$55.44`_

<!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=55.44 -->
bun test runs every suite in one process, so file-scope stubs and globals leak
into later files. Four suites leaked state that only surfaces in the full
`bun test src` order (each victim passes isolated):

- useSmoothStreamingText.test: set window/document to undefined on teardown,
  leaving WorkspaceFooterBar's popup tests without a usable document
  (3 failures). Use the shared installDom/cleanup harness instead
  (same change as 6e89d52).
- WorkspaceHeartbeatModal.test: open-only Dialog stub never restored; every
  later uncontrolled dialog rendered nothing (WorkflowLongText x3,
  WorkflowRunHeader x1). Snapshot the real module before mocking and restore.
- MessageWindow.test: Tooltip stub with TooltipContent: () => null never
  restored; MemoryTab's tooltip block could never appear. Restore Tooltip
  and ChatHostContext after the suite.
- agent_skill_delete.test: a one-shot mockRejectedValueOnce on the shared
  fs.rm can be consumed by any other in-flight rm from an earlier suite
  (several callers swallow rm errors), letting the tool's own rm succeed.
  Reject the legacy-manifest path specifically; assertions unchanged.

Exact-order replays: 7-file footer prefix 3 fail -> 41 pass; 7-file
Workflow/MemoryTab prefix 5 fail -> 58 pass; agent_skill_delete 47 pass.

_Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$13.36`_

<!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=13.36 -->

Signed-off-by: Thomas Kosiewski <tk@coder.com>
CI tests the merge tree, where the opus alias now points to Opus 5.5.
Keep enrichment assertions tied to the authoritative thinking policy and
derive the gateway allowlist entry from the selected model metadata.
Fixed-model fixtures retain explicit capability expectations.

Reproduced both failures on CI merge 3c5df45 with Bun 1.3.5; all 270
selector, AI service and thinking-policy tests pass on both trees after
the correction. No production or dependency files change.

---

_Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$199.44`_

<!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=199.44 -->
Merge-queue admission failed after #4348 renamed GPT_56_LUNA to GPT_6_LUNA
and moved the `gpt` alias to gpt-6-sol. Swap the renamed key in the explicit
picker-order lists and derive the gateway catalog entries for the routed
`gpt` built-in from KNOWN_MODELS, so the gating assertions keep testing
routing instead of one historical model ID. No production files change.

---

_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$237.64`_

<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=237.64 -->

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c9f7dd4f80

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/turnRequestBuilder.ts Outdated
When a policy refresh moves a running turn into the blocked state,
PolicyService reports enforcement with no effective policy and denies every
model. The models_list closure passed that null policy to shared filtering,
which reads null as "unenforced", so it advertised configured models the
runtime would reject. Return an empty catalog in that state instead.

Addresses Codex review thread PRRT_kwDOPxxmWM6lFBNA.

---

_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$257.67`_

<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=257.67 -->

This branch was successfully deployed

1 active (outdated) deployment
staging - docs c9f7dd4f Deployed Sep 23, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant