Skip to content

Add fallback models support and capabilities-gated UI features#2850

Merged
tim-inkeep merged 22 commits intomainfrom
implement/fallback-models-capabilities
Mar 31, 2026
Merged

Add fallback models support and capabilities-gated UI features#2850
tim-inkeep merged 22 commits intomainfrom
implement/fallback-models-capabilities

Conversation

@tim-inkeep
Copy link
Copy Markdown
Contributor

Summary

  • Add first-class fallbackModels field to ModelSettings schema — each model slot (base, structuredOutput, summarizer) can declare an ordered list of fallback models that the AI Gateway tries in sequence when the primary model fails
  • Extend /capabilities endpoint with modelFallback.enabled and costTracking.enabled flags (both derived from AI_GATEWAY_API_KEY presence) — feature-oriented naming so consumers don't need to know about gateway internals
  • Add fallback model selection UI to ModelConfiguration — reuses existing ModelSelector with a new gatewayOnly prop that filters to gateway-routable providers (anthropic, openai, google)
  • Gate Cost nav items in sidebar on costTracking.enabled — cost in traces already self-hides via costUsd != null checks

Details

Schema (agents-core): ModelSettingsSchema gains fallbackModels: z.array(z.string()).optional(). Since ModelSchema and ProjectModelSchema compose ModelSettingsSchema, all three model slots get fallback support automatically. GATEWAY_ROUTABLE_PROVIDERS exported from constants/models.ts for UI consumption.

Runtime (agents-core): ModelFactory.prepareGenerationConfig() translates fallbackModels into providerOptions.gateway.models when AI_GATEWAY_API_KEY is set. When gateway isn't configured, fallbacks are silently ignored. Existing gateway provider options (e.g. gateway.order) are preserved during merge.

API (agents-api): /capabilities response extended with modelFallback and costTracking objects. Handler reads AI_GATEWAY_API_KEY from process.env directly (same pattern as ModelFactory). OpenAPI snapshot updated.

UI (agents-manage-ui):

  • ModelConfiguration renders a "Fallback Models" section below provider options when capabilities.modelFallback.enabled is true — each fallback is a ModelSelector instance with remove button, plus an "Add fallback model" button
  • ModelSelector gains gatewayOnly prop — filters dropdown to only anthropic/openai/google and hides Custom/LLM Gateway sections
  • ModelSection and ProjectModelsSection wire fallbackModels props through for all three slots with inheritance support
  • Sidebar Cost nav items (org + project level) conditionally rendered based on capabilities.costTracking.enabled
  • Serialization, deserialization, form validation, and UI-local ModelSettings type all updated

Test plan

  • model-factory.test.ts — 4 new tests covering fallback translation with/without gateway key, empty array, and merge with existing gateway options
  • capabilities.test.ts — 3 tests updated to verify modelFallback and costTracking fields in both configured and unconfigured states
  • capabilities-query.test.tsx — test data updated with new capability fields
  • OpenAPI snapshot updated
  • Full typecheck passes
  • Manual: verify fallback model UI appears when AI_GATEWAY_API_KEY is set
  • Manual: verify Cost nav hidden when gateway not configured
  • Manual: verify fallback selector only shows anthropic/openai/google models

🤖 Generated with Claude Code

Introduce first-class `fallbackModels` on ModelSettings so each model slot
(base, structuredOutput, summarizer) can declare an ordered list of fallback
models.  At runtime ModelFactory translates the array into
`providerOptions.gateway.models` when AI_GATEWAY_API_KEY is set; otherwise
the field is silently ignored.

Extend the /capabilities endpoint with two feature-oriented flags:
- `modelFallback.enabled` – gates the fallback-model UI
- `costTracking.enabled` – gates the Cost nav in the sidebar

UI changes:
- ModelConfiguration renders a "Fallback Models" section (list of
  ModelSelector instances) when modelFallback is enabled and a primary
  model is selected.  Fallback selectors are filtered to gateway-routable
  providers (anthropic, openai, google) via a new `gatewayOnly` prop on
  ModelSelector.
- Sidebar Cost nav items (org + project level) are conditionally rendered
  based on costTracking.enabled.  Cost columns in traces already self-hide
  when costUsd is null, so no changes needed there.
- Serialization, deserialization, form validation, and the UI-local
  ModelSettings type all carry fallbackModels through.
- Fallback models inherit through the existing project → agent → sub-agent
  chain.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 26, 2026

🦋 Changeset detected

Latest commit: ee38236

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-api Patch
@inkeep/agents-core Patch
@inkeep/agents-manage-ui Patch
@inkeep/agents-cli Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch
@inkeep/agents-email Patch
@inkeep/agents-mcp Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Mar 31, 2026 5:46pm
agents-docs Ready Ready Preview, Comment Mar 31, 2026 5:46pm
agents-manage-ui Ready Ready Preview, Comment Mar 31, 2026 5:46pm

Request Review

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Mar 26, 2026

TL;DR — Adds gateway-backed fallback model support so agents can automatically retry on alternate models when the primary fails, and gates the Cost nav item and fallback-models UI behind runtime capability flags derived from AI_GATEWAY_API_KEY presence.

Key changes

  • Add fallbackModels field to ModelSettings — Extends the core Zod schema and OpenAPI spec with an ordered string array of fallback models at the validation, API, and UI type layers.
  • Extend /capabilities endpoint with modelFallback and costTracking flags — The capabilities route now checks for AI_GATEWAY_API_KEY at runtime and reports whether fallback and cost-tracking features are available.
  • Inject fallback models into gateway providerOptions in ModelFactoryprepareGenerationConfig merges fallbackModels into the gateway provider options when the gateway key is configured, preserving existing gateway settings.
  • Add GATEWAY_ROUTABLE_PROVIDERS constant — New constant restricts the fallback model selector to providers that the AI Gateway can route to (anthropic, openai, google).
  • Add gatewayOnly mode to ModelSelector — Filters the model picker to only gateway-routable providers and hides custom/LLM-gateway options when selecting fallback models.
  • Wire fallback models UI into ModelConfiguration — The shared model configuration component conditionally renders an ordered fallback-model list with add/remove controls, gated behind the modelFallback capability flag.
  • Thread fallbackModels through agent and project model forms — Model section, project models section, serialization, and deserialization all propagate fallback models alongside existing model and providerOptions fields.
  • Gate Cost sidebar nav item behind costTracking capability — The sidebar now conditionally renders the Cost link only when the server reports cost tracking is enabled.
  • Add tests for capabilities endpoint and model factory fallback behavior — New and updated tests cover capability flag responses and prepareGenerationConfig fallback injection logic.

Summary | 20 files | 1 commit | base: mainimplement/fallback-models-capabilities


Capabilities endpoint: modelFallback and costTracking flags

Before: /capabilities only returned sandbox configuration.
After: /capabilities returns modelFallback and costTracking objects, both enabled when AI_GATEWAY_API_KEY is set.

The handler checks process.env.AI_GATEWAY_API_KEY at request time and derives both flags from its presence. The response schema adds two required fields so the UI can feature-gate without hardcoded env checks.

agents-api/src/routes/capabilities.ts · agents-api/src/__tests__/capabilities.test.ts · agents-manage-ui/src/lib/actions/capabilities.ts


Core: fallbackModels schema and model factory integration

Before: ModelSettings had model and providerOptions only; no failover mechanism existed.
After: ModelSettings accepts an optional fallbackModels string array that gets injected into gateway.models provider options at generation time.

The ModelSettingsSchema in agents-core validates the new field. ModelFactory.prepareGenerationConfig merges it into streamProviderOptions.gateway.models only when AI_GATEWAY_API_KEY is present, preserving any existing gateway configuration.

How does fallback injection work at runtime?

When fallbackModels is non-empty and the gateway key exists, the factory spreads existing gateway options and adds a models key with the ordered fallback list. The AI Gateway uses this array to retry failed requests against alternate models in order. Without the gateway key, the field is silently ignored.

packages/agents-core/src/utils/model-factory.ts · packages/agents-core/src/validation/schemas.ts · packages/agents-core/src/constants/models.ts · packages/agents-core/src/__tests__/utils/model-factory.test.ts


UI: Capabilities-gated fallback model editor and cost navigation

Before: The model configuration component had no fallback support; the Cost nav link was always visible.
After: A numbered fallback-model list with add/remove appears when modelFallback.enabled is true; the Cost sidebar link renders only when costTracking.enabled is true.

The ModelConfiguration component queries capabilities and conditionally renders ModelSelector instances in gatewayOnly mode (filtered to GATEWAY_ROUTABLE_PROVIDERS). Fallback models thread through agent sidepane model sections, project form sections, and the serialize/deserialize layer for persistence.

Component Change
ModelConfiguration Renders ordered fallback list, gated on capability
ModelSelector New gatewayOnly prop filters providers
ModelSection / ProjectModelsSection Pass fallbackModels + callbacks for base, structuredOutput, summarizer
AppSidebar Cost link conditional on costTracking.enabled
serialize.ts / deserialize.ts Round-trip fallbackModels through form state

agents-manage-ui/src/components/shared/model-configuration.tsx · agents-manage-ui/src/components/agent/sidepane/nodes/model-selector.tsx · agents-manage-ui/src/components/sidebar-nav/app-sidebar.tsx · agents-manage-ui/src/features/agent/domain/serialize.ts

Pullfrog  | View workflow run | Triggered by Pullfrog | Using Claude Opus𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-structured PR. Schema addition is additive, runtime gating on AI_GATEWAY_API_KEY is correct, UI feature-flags via /capabilities is a good pattern. Two actionable issues below — one is a data integrity bug (empty-string fallback models reaching the gateway), the other is a minor DRY improvement.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

.optional()
.describe('The provider options to use for the project.'),
fallbackModels: z
.array(z.string())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Empty strings pass validation here — z.array(z.string()) accepts ['']. Downstream, ModelFactory.prepareGenerationConfig checks fallbackModels?.length which is truthy for [''], so an empty string will be sent to the gateway as a fallback model name.

Filter or validate elements:

z.array(z.string().min(1))

Alternatively, filter at the runtime layer in prepareGenerationConfig with .filter(Boolean). Either way, at least one layer should guard against it.

Suggested change
.array(z.string())
fallbackModels: z
.array(z.string().min(1))
.optional()
.describe(
'Ordered list of fallback models if the primary fails. Requires AI Gateway. Format: provider/model (e.g. "openai/gpt-5.2").'
),

size="sm"
className="w-full"
onClick={() => {
onFallbackModelsChange([...(fallbackModels ?? []), '']);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This appends an empty string '' as a placeholder for a new fallback. Since the schema currently accepts it and serialization doesn't filter empty strings, saving the form at this point would persist [''] to the API. Even with the z.string().min(1) fix on the schema, the UI should filter empties before calling the parent callback — or the serialization layer should strip them.

Consider filtering in the serializer (serialize.ts) as a safety net:

fallbackModels: modelsData.base.fallbackModels.filter(Boolean),

Comment thread packages/agents-core/src/constants/models.ts Outdated
tooltip="Ordered list of models to try if the primary model fails. Requires AI Gateway."
/>
{(fallbackModels ?? inheritedFallbackModels ?? []).map((model, index) => (
<div key={`${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using array index as key (key={\${editorNamePrefix}-fallback-${index}`}) will cause React reconciliation issues when items are reordered or removed from the middle — the wrong ModelSelectorinstance will retain state from the previous item at that index. Since fallback models are strings, consider using the model value + index as key, e.g.key={`${editorNamePrefix}-fallback-${model}-${index}`}`, or better yet assign stable IDs.

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Mar 26, 2026

TL;DR — Adds gateway-backed fallback model support and provider restriction so agents can automatically retry on alternate models when the primary fails and limit which providers serve requests. Introduces fallbackModels and allowedProviders inheritance through the project → agent → sub-agent hierarchy. Gates the Cost nav item, fallback-models UI, and allowed-providers UI behind runtime capability flags derived from AI_GATEWAY_API_KEY presence. Surfaces provider routing telemetry in the cost dashboard.

Key changes

  • Add fallbackModels and allowedProviders fields to ModelSettings — Extends the core Zod schema and OpenAPI spec with optional ordered string arrays for model failover and provider restriction at the validation, API, and UI type layers.
  • Extend /capabilities endpoint with modelFallback and costTracking flags — The capabilities route checks for AI_GATEWAY_API_KEY at runtime and reports whether fallback and cost-tracking features are available.
  • Inject fallback models and allowed providers into gateway providerOptions in ModelFactoryprepareGenerationConfig merges fallbackModels into gateway.models and allowedProviders into gateway.order/gateway.only when the gateway key is configured; strips provider prefix from model ID when allowedProviders routes traffic.
  • Add inheritGatewayFields to model-resolver.ts for hierarchical field inheritance — Sub-agents inherit fallbackModels and allowedProviders from parent agent and project levels when not explicitly set, with agent taking precedence over project.
  • Forward all ModelSettings fields through model config functionsgetPrimaryModel, getStructuredOutputModel, and getSummarizerModel now spread the full config object instead of cherry-picking model and providerOptions, fixing a bug where new fields were silently dropped.
  • Extract GATEWAY_ROUTABLE_PROVIDERS_SET into shared constant — Moved from ModelFactory into constants/models.ts so both the backend and the UI ModelSelector can restrict fallback choices to providers the AI Gateway can route to.
  • Add gatewayOnly, defaultOpen, and onClose props to ModelSelector — Filters the model picker to only gateway-routable providers, hides custom/LLM-gateway options when selecting fallback models, auto-opens the popover for newly added slots, and fires onClose when dismissed without a selection.
  • Build AllowedProvidersSection with drag-to-reorder UI — New component with radio toggle (all vs. specific), a draggable ordered provider list with add/remove, and inheritance support from parent configurations.
  • Wire fallbackModels and allowedProviders through agent, sub-agent, and project model forms — Both the agent sidepane MetadataEditor and the sub-agent ModelSection propagate both fields across all three model slots with full inheritance chains.
  • Gate Cost sidebar nav and cost pages behind costTracking capability — The sidebar conditionally renders the Cost link only when the server reports cost tracking is enabled; both cost page components call notFound() when the capability is disabled.
  • Add provider routing telemetry and "Cost by Provider" dashboard tablesetGatewayAttributesOnSpan captures gen_ai.response.provider and gen_ai.request.provider from gateway routing metadata; cost dashboard shows a new breakdown table and per-event provider column.
  • Normalize gateway model IDs via overrideModelId middlewaregatewayCostMiddleware strips known provider prefixes (e.g. anthropic/) from model IDs so cost tracking attributes report the bare model name regardless of routing.
  • Add comprehensive documentation for AI Gateway features — New "AI Gateway" docs section covering fallback models, allowed providers, combined usage, and manual gateway provider options; sub-agents docs updated with fallbackModels and allowedProviders fields.
  • Add tests for capabilities, model resolver inheritance, model factory fallback, and overrideModelId — New tests cover gateway field inheritance across project/agent/sub-agent hierarchy, capability flag responses, prepareGenerationConfig fallback/provider injection, and normalizeModelId.

Summary | 35 files | 9 commits | base: mainimplement/fallback-models-capabilities


Capabilities endpoint: modelFallback and costTracking flags

Before: /capabilities only returned sandbox configuration.
After: /capabilities returns modelFallback and costTracking objects, both enabled when AI_GATEWAY_API_KEY is set.

The handler checks process.env.AI_GATEWAY_API_KEY at request time and derives both flags from its presence. The response schema adds two required fields so the UI can feature-gate without hardcoded env checks.

capabilities.ts · capabilities.test.ts · capabilities.ts (UI)


Core: fallbackModels and allowedProviders schema and model factory integration

Before: ModelSettings had model and providerOptions only; no failover or provider-restriction mechanism existed.
After: ModelSettings accepts optional fallbackModels and allowedProviders string arrays that get injected into gateway provider options at generation time, with provider prefix stripping when allowedProviders routes traffic.

The ModelSettingsSchema in agents-core validates both new fields. ModelFactory.prepareGenerationConfig conditionally builds gateway provider options when AI_GATEWAY_API_KEY is present: fallbackModels maps to gateway.models, allowedProviders maps to gateway.order and gateway.only. When allowedProviders is set, createModel strips the provider prefix from the model ID so the gateway can route to the first allowed provider. Both fields are silently ignored without the gateway key.

How do fallback and provider restrictions interact at runtime?

When the gateway key exists, the factory merges both into the same gateway options object. models tells the gateway which alternate models to try on failure, while order/only restrict which providers can serve any request (including fallback attempts). They compose naturally — e.g., setting allowedProviders: ['bedrock', 'anthropic'] with fallbackModels: ['anthropic/claude-haiku-4-5'] routes all traffic through Bedrock or Anthropic and retries on Haiku if the primary fails.

schemas.ts · model-factory.ts · models.ts · model-factory.test.ts


Gateway field inheritance in model resolver

Before: resolveModelConfig returned model settings verbatim from the first level in the hierarchy that defined a base model.
After: An inheritGatewayFields function fills in fallbackModels and allowedProviders from parent levels (agent → project) when the resolved level does not define them, respecting explicit overrides.

The resolver walks sub-agent → agent → project. For each model slot, if the resolved config lacks fallbackModels or allowedProviders, it checks progressively higher levels. Explicit values at lower levels are never overridden. Five new tests cover all inheritance paths and the no-parent edge case.

model-resolver.ts · model-resolver.test.ts · model-config.ts


Provider routing telemetry and cost-by-provider dashboard

Before: Cost middleware only set gen_ai.usage.cost; the dashboard had no provider dimension.
After: setGatewayAttributesOnSpan captures gen_ai.response.provider and gen_ai.request.provider from gateway routing metadata; the cost dashboard shows a "Cost by Provider" breakdown table and a per-event provider column. overrideModelId normalizes gateway model IDs by stripping provider prefixes.

Change Detail
usage-cost-middleware.ts Renamed to setGatewayAttributesOnSpan; reads routing.finalProvider / routing.resolvedProvider from gateway metadata; adds overrideModelId hook and exported normalizeModelId
otel-attributes.ts Two new span keys: GEN_AI_REQUEST_PROVIDER, GEN_AI_RESPONSE_PROVIDER
signoz-stats.ts getUsageCostSummary accepts 'provider' as groupBy; event mapping prefers GEN_AI_RESPONSE_PROVIDER over AI_MODEL_PROVIDER
cost-dashboard.tsx Third breakdown table ("Cost by Provider"), provider column in events table, 3-column grid on xl

usage-cost-middleware.ts · otel-attributes.ts · cost-dashboard.tsx · signoz-stats.ts


UI: Capabilities-gated fallback model editor and allowed providers

Before: The model configuration component had no fallback or provider-restriction support; the Cost nav link was always visible.
After: A numbered fallback-model list with add/remove appears when modelFallback.enabled is true; an allowed-providers section with drag-to-reorder and all/specific radio toggle appears alongside it; the Cost sidebar link and cost pages render only when costTracking.enabled is true.

Component Change
AllowedProvidersSection New component with all/specific radio toggle, draggable ordered list from AVAILABLE_PROVIDERS (6 providers including Bedrock, Azure, Vertex), add/remove controls, and inheritance
FallbackModelsSection Pending-selector pattern with onClose cleanup — only commits to array on selection
ModelConfiguration Renders both allowed-providers and fallback sections gated on modelFallback.enabled and gateway-routability of the selected model
ModelSelector gatewayOnly, defaultOpen, onClose props; filters providers via GATEWAY_ROUTABLE_PROVIDERS_SET, auto-opens popover
ModelSection Sub-agent panel wires fallbackModels + allowedProviders + callbacks for all three model slots with inheritance from agent and project
MetadataEditor Agent panel wires fallbackModels + allowedProviders for all three model slots with project-level inheritance
ProjectModelsSection Project form passes both fields for all model slots
AppSidebar Cost link conditional on costTracking.enabled
Cost pages Both tenant and project cost pages call notFound() when costTracking is disabled
serialize.ts / validation.ts Round-trip allowedProviders and fallbackModels through form state with .filter(Boolean) sanitization

model-configuration.tsx · model-selector.tsx · model-section.tsx · metadata-editor.tsx


Documentation: AI Gateway section in models docs

Before: Gateway usage was documented inline with a providerOptions.gateway.models code example.
After: A dedicated "AI Gateway" section documents fallback models, allowed providers, combined usage, and manual gateway provider options. Sub-agents docs updated with new configurable fields.

The old inline gateway model routing example was replaced with a brief mention linking to the new section. The new docs cover inheritance behavior (project → agent → sub-agent), provider/model format requirements, and interaction semantics when both features are combined.

models.mdx · sub-agents.mdx

Pullfrog  | View workflow run | Triggered by Pullfrog | Using Claude Opus𝕏

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(6) Total Issues | Risk: Medium

🟠⚠️ Major (2) 🟠⚠️

Inline Comments:

  • 🟠 Major: serialize.ts:573-594 Agent-level models serialization missing fallbackModels field
  • 🟠 Major: model-configuration.tsx:250-258 Empty string added to fallbackModels array on "Add fallback model"

🟡 Minor (3) 🟡

Inline Comments:

  • 🟡 Minor: model-configuration.tsx:235-240 Missing accessible name on remove button
  • 🟡 Minor: model-configuration.tsx:214-215 Array index used as key can cause state corruption
  • 🟡 Minor: constants/models.ts:92 GATEWAY_ROUTABLE_PROVIDERS duplicated between constants and model-factory

💭 Consider (1) 💭

💭 1) model-factory.test.ts Missing test for undefined fallbackModels

Issue: Tests cover empty array [] but not undefined for fallbackModels. The implementation checks modelSettings?.fallbackModels?.length which handles undefined differently.

Why: If a future refactor changes the truthiness check, the bug would slip through. Models saved without fallbackModels field should not inject gateway options.

Fix: Add test case:

test('should not inject when fallbackModels is undefined', () => {
  process.env.AI_GATEWAY_API_KEY = 'test-key';
  const config: ModelSettings = { model: 'anthropic/claude-sonnet-4-5' };
  const result = ModelFactory.prepareGenerationConfig(config);
  expect(result.providerOptions?.gateway).toBeUndefined();
});

Refs: model-factory.ts:359


🚫 REQUEST CHANGES

Summary: The implementation is well-structured overall, but the agent-level models serialization bug will cause fallbackModels to be silently dropped when saving agents (as opposed to sub-agents where it works correctly). This is a data loss issue that should be fixed before merge. The empty string injection to fallbackModels is also worth addressing to prevent gateway errors. The minor issues (accessibility, key prop, duplicated constant) can be addressed in this PR or as follow-up.

Discarded (10)
Location Issue Reason Discarded
app-sidebar.tsx:69 Conditional spread creates unstable array reference Minor perf concern, nav doesn't re-render frequently
app-sidebar.tsx:53 Capabilities query in sidebar causes re-renders React Query deduplication handles this; acceptable pattern
model-selector.tsx:67 Missing setHasOpenModelConfig in useEffect deps Pre-existing issue, zustand setters are stable
model-section.tsx:34-71 Repeated inheritance computation logic Pre-existing code duplication, not introduced by this PR
capabilities.ts:39 OpenAPI schema name uses 'Schema' suffix 50/50 convention - both patterns exist in codebase
app-sidebar.tsx:54 costTrackingEnabled vs isWorkAppsEnabled naming Minor naming inconsistency, not worth blocking
model-configuration.tsx:207 Flash when capabilities load Minor CLS, acceptable given React Query caching
serialize.test.ts No tests for fallbackModels serialization Valid concern but Consider-level, not blocking
deserialize.test.ts No round-trip test for fallbackModels Valid concern but Consider-level, not blocking
model-configuration.tsx No React component tests for fallback UI Manual testing noted in PR, acceptable
Reviewers (4)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-standards 2 0 0 0 2 0 0
pr-review-frontend 8 0 0 0 3 0 5
pr-review-tests 4 0 1 0 0 0 3
pr-review-consistency 3 0 0 0 1 0 2
Total 17 0 1 0 6 0 10

Comment on lines +250 to +258
<Button
variant="outline"
size="sm"
className="w-full"
onClick={() => {
onFallbackModelsChange([...(fallbackModels ?? []), '']);
}}
disabled={disabled}
>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 MAJOR: Empty string added to fallbackModels array

Issue: Clicking "Add fallback model" appends an empty string to the fallbackModels array. If the user doesn't select a model before saving, this empty string will be sent to the AI Gateway as part of gateway.models, which may cause gateway errors.

Why: Empty model strings in the fallback list will be passed through ModelFactory.prepareGenerationConfig() to the gateway, potentially causing API errors when the gateway attempts to route to an empty model identifier.

Fix: Filter empty/falsy values before calling onFallbackModelsChange, or add defensive filtering in ModelFactory.prepareGenerationConfig():

Suggested change
<Button
variant="outline"
size="sm"
className="w-full"
onClick={() => {
onFallbackModelsChange([...(fallbackModels ?? []), '']);
}}
disabled={disabled}
>
<Button
variant="outline"
size="sm"
className="w-full"
onClick={() => {
onFallbackModelsChange([...(fallbackModels ?? []), '']);
}}
disabled={disabled}
>

Or add this filter in model-factory.ts at line 365:

models: modelSettings.fallbackModels.filter(m => m && m.trim())

Refs:

Comment on lines +235 to +240
<Button
variant="ghost"
size="icon"
className="h-8 w-8 shrink-0"
onClick={() => {
const models = [...(fallbackModels ?? [])];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Missing accessible name on remove button

Issue: The remove button contains only an X icon without an accessible label. Screen reader users won't understand what this button does or which fallback model it removes.

Why: Violates WCAG 2.1 SC 4.1.2 (Name, Role, Value). Users relying on assistive technology cannot determine the button's purpose.

Fix:

Suggested change
<Button
variant="ghost"
size="icon"
className="h-8 w-8 shrink-0"
onClick={() => {
const models = [...(fallbackModels ?? [])];
<Button
variant="ghost"
size="icon"
className="h-8 w-8 shrink-0"
aria-label={`Remove fallback model ${index + 1}`}
onClick={() => {

Refs:

Comment on lines +214 to +215
{(fallbackModels ?? inheritedFallbackModels ?? []).map((model, index) => (
<div key={`${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Array index used as key can cause state corruption

Issue: Using key={\${editorNamePrefix}-fallback-${index}`}with array index means that when items are removed from the middle of the list, React will reuse DOM nodes incorrectly. TheModelSelector` components have internal state (popover open state, custom input mode), which will get mismatched to the wrong items.

Why: When a user removes fallback model #2 from a list of 3, React will match the old component for index 2 to the new item at index 2 (which was previously index 3), potentially preserving stale UI state.

Fix: Consider using the model value in the key. Since models can be empty strings or duplicates, a combination approach works:

Suggested change
{(fallbackModels ?? inheritedFallbackModels ?? []).map((model, index) => (
<div key={`${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">
{(fallbackModels ?? inheritedFallbackModels ?? []).map((model, index) => (
<div key={`${editorNamePrefix}-fallback-${model || 'empty'}-${index}`} className="flex items-center gap-2">

For a more robust solution, consider storing fallback models as objects with stable IDs.

Refs:

Comment thread packages/agents-core/src/constants/models.ts Outdated
@github-actions github-actions Bot deleted a comment from claude Bot Mar 26, 2026
Use a pending selector pattern: clicking "Add fallback model" shows a
local-only ModelSelector row with defaultOpen. The save callback only
fires when the user actually picks a model, preventing premature
auto-save with empty values.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(3) Total Issues | Risk: Low

Re-review scope: This review covers only the delta since the last automated review (1 commit, 2 files). See previous review for the full initial assessment.

✅ Addressed from Previous Review

The empty string injection bug (previously Major) is fixed by the new FallbackModelsSection component. The showPendingSelector state pattern correctly defers adding to the fallbackModels array until an actual model is selected — clicking "Add fallback model" no longer appends an empty string.

🟡 Minor (3) 🟡

Inline Comments:

  • 🟡 Minor: model-configuration.tsx:38 Array index as key can cause state corruption
  • 🟡 Minor: model-configuration.tsx:58-68 Remove button missing accessible name
  • 🟡 Minor: model-configuration.tsx:93-98 Cancel button missing accessible name

💭 Consider (1) 💭

Inline Comments:

  • 💭 Consider: model-selector.tsx:59-63 Redundant useEffect for defaultOpen synchronization

🕐 Pending Recommendations (3)

These issues were raised in the previous review and remain unresolved:

  • 🟠 serialize.ts:573-594 Agent-level models serialization missing fallbackModels field — this is a data loss issue where fallbackModels won't persist for root agents
  • 🟡 constants/models.ts:92 GATEWAY_ROUTABLE_PROVIDERS duplicated between constants and model-factory
  • 💭 model-factory.test.ts Missing test for undefined fallbackModels

💡 APPROVE WITH SUGGESTIONS

Summary: The delta successfully fixes the empty string injection bug with a clean deferred-selector pattern. The remaining issues are minor accessibility improvements (add aria-label to icon buttons) and a key prop optimization. The pending serialization bug from the prior review (serialize.ts missing fallbackModels for agent-level models) should still be addressed before merge to prevent data loss.

Reviewers (1)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-frontend 4 0 0 0 4 0 0
Total 4 0 0 0 4 0 0

tooltip="Ordered list of models to try if the primary model fails. Requires AI Gateway."
/>
{savedModels.map((model, index) => (
<div key={`${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Array index as key can cause state corruption

Issue: Using index as the key (key={\${editorNamePrefix}-fallback-${index}`}`) will cause React to incorrectly preserve component state when items are removed from the middle of the list.

Why: When a fallback model is removed via splice(index, 1), React sees keys shift (e.g., removing index 1 causes item at index 2 to become index 1). React reuses DOM/state from the old index position, which can cause ModelSelector internal state to show stale data.

Fix: Use the model value as the key:

Suggested change
<div key={`${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">
{savedModels.map((model, index) => (
<div key={model || `${editorNamePrefix}-fallback-${index}`} className="flex items-center gap-2">

Refs:

Comment thread agents-manage-ui/src/components/shared/model-configuration.tsx
Comment thread agents-manage-ui/src/components/shared/model-configuration.tsx
Comment on lines +59 to +63
useEffect(() => {
if (defaultOpen) {
setOpen(true);
}
}, [defaultOpen]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💭 Consider: Redundant useEffect for defaultOpen synchronization

Issue: This useEffect may be redundant since useState(defaultOpen) already sets the initial value. The effect only sets open = true when defaultOpen becomes true, but doesn't handle the reverse case (closing when defaultOpen becomes false).

Why: The current usage works because the pending selector unmounts/remounts, but this creates fragile coupling. If ModelSelector is reused in a context where defaultOpen toggles without remounting, the popover won't close.

Fix: Either remove the effect entirely (if re-mounting is the intended pattern) or make it bidirectional:

useEffect(() => {
  setOpen(defaultOpen);
}, [defaultOpen]);

For this specific use case where re-mounting is intentional, removing the effect entirely would be cleaner.

Refs:

@github-actions github-actions Bot deleted a comment from claude Bot Mar 26, 2026
.optional()
.describe('The provider options to use for the project.'),
fallbackModels: z
.array(z.string().min(1))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.array(z.string().min(1))
.array(z.string().nonempty())

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(1) Total Issues | Risk: Medium

Re-review scope: This review covers only the delta since the previous review (1 commit, 4 files).

✅ Addressed from Previous Reviews

The delta (41f3c2377) successfully addresses 3 issues from prior reviews:

Issue Fix Applied
Empty strings pass fallbackModels validation z.string().min(1) in schemas.ts:225
GATEWAY_ROUTABLE_PROVIDERS duplicated between constants and model-factory Removed local const, imports from constants/models.js
Array index as key causes React state issues Changed to ${model}-${index} composite key in model-configuration.tsx:39

Additionally, .filter(Boolean) was added to all three model slots in serialize.ts (processModels function) as a defense-in-depth measure for filtering empty strings before persistence.

🕐 Pending Recommendations (1)

These issues were raised in prior reviews and remain unresolved:

  • 🟠 serialize.ts:573-594 Agent-level models serialization missing fallbackModels field — the processModels function correctly includes fallbackModels with .filter(Boolean) for sub-agents, but the agent-level models (lines 573-594) still serializes without fallbackModels. This means root agent fallback models will be silently dropped on save.

🟡 Minor Items from Prior Reviews (not blocking)

These accessibility issues from prior reviews remain relevant but are not blocking:

  • Missing aria-label on remove button (model-configuration.tsx:61-73)
  • Missing aria-label on cancel button (model-configuration.tsx:96-103)

🚫 REQUEST CHANGES

Summary: The delta makes good progress — the validation and deduplication fixes are solid. However, the agent-level serialization bug (serialize.ts:573-594) from the initial review still needs to be addressed. Without it, fallbackModels configured at the root agent level will be lost when saving. The fix is straightforward: apply the same pattern used in processModels (lines 78-80, 87-89, 96-98) to the agent-level serialization block.

Discarded (0)

No findings discarded in this delta review.

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
Orchestrator analysis only 0 0 0 0 1 0

Note: Sub-reviewers were not dispatched for this delta review due to the small scope (4 files, ~15 LOC changed). Findings are based on orchestrator analysis of the diff against prior review context.

@itoqa
Copy link
Copy Markdown

itoqa Bot commented Mar 26, 2026

Ito Test Report ❌

19 test cases ran. 5 failed, 14 passed.

Across 19 test cases, 14 passed and 5 failed, with core gateway capability behavior, cost-navigation gating/routing, agent fallback persistence, unsaved-change handling, save-race stability, and security checks (unauthorized API access, XSS, malformed/oversized IDs, and capability-fetch failure handling) all working as expected. The most important confirmed defects are in fallback-model UX and persistence: project settings drops models.*.fallbackModels during save/reload (including duplicate and concurrent-edit scenarios, causing silent data loss despite success feedback), inherited sub-agent fallback rows can be edited before explicit override, and pressing Escape in the selector leaves an orphan pending fallback row.

❌ Failed (5)
Category Summary Screenshot
Edge 🟠 Inherited structured/summarizer fallback settings are editable before explicit override. EDGE-1
Edge 🟠 Duplicate fallback entries can be added, but fallback rows disappear after save/reload due to fallbackModels being omitted from submit payloads. EDGE-6
Edge ⚠️ Saving project drops fallbackModels from the PATCH payload despite success feedback. EDGE-8
Logic 🟠 Pressing Escape closes the fallback model popover but leaves an orphan pending fallback row instead of clearing it. LOGIC-3
Happy-path ⚠️ Fallback model selections are dropped on save/reload because project form serialization omits fallbackModels. ROUTE-4
🟠 Inherited fallback settings unlock before explicit override
  • What failed: The UI allows adding fallback models while the slot is still inherited; expected behavior is locked editing until an explicit model override is set.
  • Impact: Users can unintentionally create local overrides while the UI indicates inheritance, causing configuration drift and confusing save behavior. This can lead to misconfigured model failover chains for sub-agents.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Navigate to a sub-agent where structured or summarizer model settings are inherited from base/project configuration.
    2. Open Advanced Model Options and locate the inherited fallback models area.
    3. Click Add fallback model before setting an explicit structured/summarizer model override.
  • Code analysis: In fallback UI rendering, inherited state is computed and used to disable existing row controls, but the Add fallback model action ignores inherited state. The model section wires inherited fallback values and mutation handlers for structured/summarizer slots, so this unlocked button enables mutation before explicit override.
  • Why this is likely a bug: The code explicitly detects inherited state but fails to enforce that state for the add action, contradicting the intended inherited lock semantics.

Relevant code:

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 27-29)

const savedModels = fallbackModels ?? inheritedFallbackModels ?? [];
const isInherited = !fallbackModels && !!inheritedFallbackModels;

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 106-113)

{!showPendingSelector && (
  <Button
    variant="outline"
    size="sm"
    className="w-full"
    onClick={() => setShowPendingSelector(true)}
    disabled={disabled}
  >

agents-manage-ui/src/components/agent/sidepane/nodes/model-section.tsx (lines 197-201)

fallbackModels={models?.structuredOutput?.fallbackModels}
inheritedFallbackModels={structuredOutputInheritance.fallbackModels}
onFallbackModelsChange={(models) =>
  updatePath('models.structuredOutput.fallbackModels', models.length ? models : undefined)
}
🟠 Duplicate fallback entries and ordering semantics
  • What failed: After reload, fallback rows are gone instead of being deterministically preserved or explicitly rejected with validation.
  • Impact: Users cannot trust saved fallback ordering/contents, and duplicate-entry handling becomes opaque because persisted state is lost. This undermines reliability of model failover configuration workflows.
  • Introduced by this PR: No – pre-existing bug (code not changed in this PR)
  • Steps to reproduce:
    1. In project settings, add two fallback rows with the same model (for example GPT-5.2 twice).
    2. Click Update project and wait for success.
    3. Reload settings and reopen Configure default models.
    4. Observe fallback rows are gone instead of preserved or explicitly rejected.
  • Code analysis: I checked duplicate-entry UI handling and persistence paths. The UI allows duplicate additions, but the root persistence failure is still the submit serializer omitting fallbackModels entirely, so duplicates (and non-duplicates) are both lost.
  • Why this is likely a bug: The implementation explicitly renders and edits fallback rows but the submit path omits that field, so the disappearance is a direct production-code mismatch between UI state and API payload.

Relevant code:

agents-manage-ui/src/components/projects/form/project-form.tsx (lines 65-80)

base: {
  model: data.models.base.model,
  providerOptions: cleanProviderOptions(data.models.base.providerOptions),
},
structuredOutput: data.models?.structuredOutput?.model
  ? {
      model: data.models.structuredOutput.model,
      providerOptions: cleanProviderOptions(data.models.structuredOutput.providerOptions),
    }
  : undefined,
summarizer: data.models?.summarizer?.model
  ? {
      model: data.models.summarizer.model,
      providerOptions: cleanProviderOptions(data.models.summarizer.providerOptions),
    }
  : undefined,

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 315-323)

{capabilities?.modelFallback?.enabled && effectiveModel && onFallbackModelsChange && (
  <FallbackModelsSection
    editorNamePrefix={editorNamePrefix}
    fallbackModels={fallbackModels}
    inheritedFallbackModels={inheritedFallbackModels}
    onFallbackModelsChange={onFallbackModelsChange}
    disabled={disabled}
  />
)}
⚠️ Two-tab concurrent edits to fallback configuration
  • What failed: The UI allows selecting fallback models and shows success on save, but persisted models.*.fallbackModels are removed after save/reload.
  • Impact: Project fallback failover configuration is silently lost after save, leaving agents without intended resiliency. Users receive a success toast, so they can ship incorrect model settings unknowingly.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Navigate to a project settings page and open Default models configuration.
    2. Add fallback models under the base model and click Update project.
    3. Reload the page and read back project model settings.
    4. Observe that fallbackModels are missing despite save success feedback.
  • Code analysis: I reviewed the project settings form state wiring, submit serialization, API PATCH call, and backend update path. The form captures fallback models, but serialization strips them before sending; backend persistence then writes only the stripped payload.
  • Why this is likely a bug: The save path explicitly drops fallbackModels during client-side serialization, directly explaining the reproducible mismatch between successful save feedback and missing persisted fallback data.

Relevant code:

agents-manage-ui/src/components/projects/form/project-models-section.tsx (lines 37-40)

const { field: fallbackModelsField } = useController({
  control,
  name: 'models.base.fallbackModels',
});

agents-manage-ui/src/components/projects/form/project-form.tsx (lines 63-80)

models: {
  ...data.models,
  base: {
    model: data.models.base.model,
    providerOptions: cleanProviderOptions(data.models.base.providerOptions),
  },
  structuredOutput: data.models?.structuredOutput?.model
    ? {
        model: data.models.structuredOutput.model,
        providerOptions: cleanProviderOptions(data.models.structuredOutput.providerOptions),
      }
    : undefined,

packages/agents-core/src/data-access/manage/projects.ts (lines 297-303)

const [updated] = await db
  .update(projects)
  .set({
    ...params.data,
    updatedAt: now,
  })
🟠 Pending fallback row persists after closing selector with Escape
  • What failed: The selector closes, but the unsaved pending fallback row remains visible; expected behavior is to dismiss both popover and pending row when canceling via Escape.
  • Impact: Users can be left with misleading unsaved fallback UI state and may commit unintended edits after canceling. This increases configuration error risk in model failover ordering.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Navigate to project settings and open default model configuration with fallback models enabled.
    2. Click Add fallback model so the pending selector row opens.
    3. Press Escape to close the selector popover.
    4. Observe that the pending fallback row remains visible instead of being dismissed.
  • Code analysis: I reviewed fallback row lifecycle logic in model-configuration.tsx and popover state handling in model-selector.tsx. Pending-row teardown only runs on explicit selection or X-button removal, while Escape only toggles internal popover state and never notifies the parent to clear pending state.
  • Why this is likely a bug: The parent component owns pending-row visibility, but Escape closes only child popover state, creating a clear state-management gap that leaves canceled pending UI behind.

Relevant code:

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 76-104)

{showPendingSelector && (
        <div className="flex items-center gap-2">
          <span className="text-xs text-muted-foreground w-4 shrink-0">
            {savedModels.length + 1}.
          </span>
          <div className="flex-1">
            <ModelSelector
              value=""
              gatewayOnly
              defaultOpen
              onValueChange={(newValue) => {
                setShowPendingSelector(false);
                if (newValue) {
                  onFallbackModelsChange([...(fallbackModels ?? []), newValue]);
                }
              }}

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 96-101)

<Button
            variant="ghost"
            size="icon"
            className="h-8 w-8 shrink-0"
            onClick={() => setShowPendingSelector(false)}
          >

agents-manage-ui/src/components/agent/sidepane/nodes/model-selector.tsx (lines 57-63)

const [open, setOpen] = useState(defaultOpen);

  useEffect(() => {
    if (defaultOpen) {
      setOpen(true);
    }
  }, [defaultOpen]);

  <Popover open={open} onOpenChange={disabled ? undefined : setOpen}>
⚠️ Project settings fallback models save and reload
  • What failed: Saved fallback rows disappear after reload even though save reports success; expected behavior is that fallbackModels persist and rehydrate.
  • Impact: Admin users cannot persist fallback failover policy in project settings, so runtime failover intent is silently lost. This can leave production traffic without configured backup models despite successful-save feedback.
  • Introduced by this PR: No – pre-existing bug (code not changed in this PR)
  • Steps to reproduce:
    1. Open a project settings page with gateway-enabled fallback UI.
    2. Add fallback models under Default models and click Update project.
    3. Reload the settings page and reopen Configure default models.
    4. Observe that fallback rows are missing after a successful save.
  • Code analysis: I reviewed the project settings form submit path, models form bindings, and fallback UI wiring. The UI writes fallbackModels into form state, but submit serialization strips the field for all model slots before PATCH/POST.
  • Why this is likely a bug: The same form flow accepts fallbackModels in state/schema but deterministically drops them in serialized payloads, which directly explains the post-save disappearance without relying on test-only behavior.

Relevant code:

agents-manage-ui/src/components/projects/form/project-form.tsx (lines 61-74)

return {
  ...data,
  models: {
    ...data.models,
    base: {
      model: data.models.base.model,
      providerOptions: cleanProviderOptions(data.models.base.providerOptions),
    },
    structuredOutput: data.models?.structuredOutput?.model
      ? {
          model: data.models.structuredOutput.model,
          providerOptions: cleanProviderOptions(data.models.structuredOutput.providerOptions),
        }
      : undefined,

agents-manage-ui/src/components/projects/form/project-models-section.tsx (lines 37-79)

const { field: fallbackModelsField } = useController({
  control,
  name: 'models.base.fallbackModels',
});

<ModelConfiguration
  fallbackModels={fallbackModelsField.value ?? undefined}
  onFallbackModelsChange={(models) => {
    fallbackModelsField.onChange(models.length ? models : undefined);
  }}
/>

agents-manage-ui/src/components/projects/form/validation.ts (lines 3-7)

const modelSettingsSchema = z.object({
  model: z.string().optional(),
  providerOptions: z.record(z.string(), z.any()).optional().nullable(),
  fallbackModels: z.array(z.string()).optional().nullable(),
});
✅ Passed (14)
Category Summary Screenshot
Adversarial XSS payload in custom model ID persisted as inert text with no script execution. ADV-1
Adversarial Malformed and oversized custom model IDs are treated as opaque values with no executable path. ADV-2
Adversarial Unauthenticated capabilities and project update calls were rejected with 401, and authenticated readback showed no project mutation. ADV-3
Adversarial Cost links were hidden while direct cost deep-links remained accessible with non-sensitive empty/zero data. ADV-4
Adversarial Save replay stress in project settings and agent editor stayed deterministic after double-submit attempts and reload. ADV-5
Edge Fallback selector exposed gateway-routable providers and blocked unsupported openrouter selection. EDGE-2
Edge Mobile viewport fallback controls remained reachable, and project update completed successfully. EDGE-4
Edge Unsaved edits were discarded on back/forward and refresh, with saved values preserved. EDGE-5
Edge Capability-fetch error handling remains defensive and UI gating stays null-safe. EDGE-7
Happy-path Authenticated capabilities check returned gateway-off flags with sandbox present as expected. ROUTE-1
Happy-path Capabilities returned modelFallback/costTracking as enabled and project settings showed fallback controls in gateway-on mode. ROUTE-2
Happy-path Fallback model persisted after save and hard refresh in agent editor. ROUTE-3
Happy-path Verified capability-gated cost navigation in org and project sidebars across gateway-off and gateway-on modes, with correct destination routes. ROUTE-5
Happy-path Capability flags stayed consistent across project settings, Cost navigation, and agent editor gating in one authenticated session. ROUTE-6

Commit: 7d9c6de

View Full Run


Tell us how we did: Give Ito Feedback

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

Re-review scope: This review covers the delta since the previous automated approval (commit 630df345f786ef5ea3). The delta consists entirely of a merge from main with no changes to PR-specific code.

✅ Delta Analysis

The commit (786ef5ea3 — "updated ui") is a merge from main with no changes to the PR's fallback models feature code. Changes brought in from main:

Component Change Relevance to PR
Org Entitlement System (#2845) Adds seat limits, billing UI, entitlement middleware ❌ Unrelated — separate feature
Security Fixes (#2922, #2924, #2927) Anonymous session JWT claims hardening ❌ Unrelated — auth layer
CI Fixes (#2921) Preview environment teardown ❌ Unrelated — infrastructure
Sidebar (app-sidebar.tsx) Adds Billing nav item with entitlement gating ❌ Unrelated — the Cost nav gating logic remains intact
Member Components Entitlement error handling and seat limit UX ❌ Unrelated — org member management

The PR-specific files have no changes:

  • model-configuration.tsx — unchanged
  • model-selector.tsx — unchanged
  • model-factory.ts — unchanged
  • capabilities.ts — unchanged
  • schemas.ts — unchanged

🟡 Minor (1) 🟡

Inline Comments:

  • 🟡 Minor: model-selector.tsx:61-65 Simplify useEffect with direct Popover props (per @dimaMachina's feedback)

🕐 Pending Recommendations (1)

Human reviewer feedback that remains unaddressed:

  • 🟡 model-selector.tsx:125 @dimaMachina: Replace useEffect with direct Popover props — The current pattern uses useState(defaultOpen) + useEffect to sync the defaultOpen prop. The cleaner approach is to pass defaultOpen={defaultOpen} open={open} directly to the Popover component, aligning with Radix's controlled API. This is a minor code quality improvement that can be addressed in this PR or as follow-up.

✅ APPROVE

Summary: The delta contains only a merge from main with no changes to PR-specific code. All prior fallback models functionality has been implemented correctly and reviewed in previous cycles. The outstanding item is @dimaMachina's feedback about simplifying the useEffect pattern in model-selector.tsx — this is a minor code quality improvement that can be addressed in this PR or as a follow-up. The feature is solid and ready to ship! 🚀

Discarded (0)

No findings discarded in this review.

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
Orchestrator analysis only 0 0 0 1 1 0

Note: Sub-reviewers were not dispatched for this delta review as the PR-specific code has no changes since the last approval. The delta consists entirely of a merge from main.

if (defaultOpen) {
setOpen(true);
}
}, [defaultOpen]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Minor: Simplify useEffect with direct Popover props

Issue: Per @dimaMachina's feedback, this useEffect can be simplified by passing defaultOpen directly to the Popover component instead of syncing via state.

Why: The current pattern creates unnecessary indirection. Radix's Popover supports controlled usage where you pass both defaultOpen and open together.

Fix:

// Remove this useEffect block (lines 61-65)

// Then change the Popover props from:
<Popover open={open} onOpenChange={...}>
// To:
<Popover defaultOpen={defaultOpen} open={open} onOpenChange={...}>

Refs:

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

Re-review scope: This review covers the delta since the previous automated approval (commit 786ef5ea3d2b86a73). The delta is a single commit addressing human reviewer feedback.

✅ Delta Analysis

The commit (d2b86a734 — "updated with dima feedback") successfully addresses @dimaMachina's feedback:

Change Assessment
Removed useEffect block (previously lines 61-65) ✅ Correct — eliminates redundant state sync
Added defaultOpen={defaultOpen} prop to Popover (line 119) ✅ Correct — uses Radix's native API for controlled/uncontrolled state

Code quality improvement:
The previous pattern used useState(defaultOpen) + useEffect to sync the defaultOpen prop to internal state. The new pattern passes defaultOpen directly to the Radix Popover component alongside open, which:

  • Eliminates unnecessary re-renders from state synchronization
  • Aligns with Radix Popover's documented API
  • Is cleaner and more maintainable

✅ All Prior Issues Addressed

Issue Status
Empty string validation for fallbackModels ✅ Fixed with z.string().nonempty()
Project-level models serialization ✅ Fixed in project-form.tsx
Agent-level models serialization ✅ Fixed in serialize.ts
Accessibility labels on buttons ✅ Added aria-label attributes
Escape key orphan pending row ✅ Fixed with onClose callback
Inherited fallback settings protection ✅ Fixed with disabled={disabled || isInherited}
GATEWAY_ROUTABLE_PROVIDERS deduplication ✅ Consolidated in constants/models.ts
Cost route deep-link gating ✅ Added notFound() when capability disabled
Model ID normalization in telemetry ✅ Added normalizeModelId()
useEffect simplification (human reviewer) Fixed in this commit

✅ APPROVE

Summary: The delta successfully implements @dimaMachina's feedback by replacing the useEffect state sync pattern with direct defaultOpen prop passing to the Radix Popover component. This is a clean, focused code quality improvement. All prior review issues have been addressed. Ship it! 🚀

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
Orchestrator analysis only 0 0 0 0 0 0

Note: Sub-reviewers were not dispatched for this delta review as the change is a minimal code quality fix (~6 lines) directly addressing human reviewer feedback.

.filter(
([provider]) =>
!gatewayOnly ||
(GATEWAY_ROUTABLE_PROVIDERS as readonly string[]).includes(provider)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@claude can you ensure there are no .includes? .includes is usually a bug

@itoqa
Copy link
Copy Markdown

itoqa Bot commented Mar 31, 2026

Ito Test Report ❌

18 test cases ran. 2 failed, 16 passed.

Overall, 16 of 18 test cases passed, confirming that capability-gated behavior, API contracts, sidebar visibility, deep-link blocking when disabled, enabled cost-page rendering paths, project model-configuration logic, persistence, inheritance/serialization, and adversarial security checks all behaved as expected without mocked route behavior. The key issue was 2 high-severity failures reproducing the same real regression introduced by this PR: tenant/project cost routes can incorrectly hit 404 in enabled environments during fresh load/refresh because the guard calls notFound() while capabilities are still unresolved (null initial query data).

❌ Failed (2)
Category Summary Screenshot
Edge ⚠️ Enabled tenant cost deep-link can render 404 before capabilities finish loading. EDGE-3
Edge ⚠️ Cost pages can render persistent 404 before capabilities resolve, despite enabled cost tracking. EDGE-4
⚠️ Refresh and non-linear navigation around gated cost routes
  • What failed: The tenant cost page returns a not-found state instead of rendering Cost & Token Usage while capability fetch is still in flight.
  • Impact: Admin users can be incorrectly blocked from tenant-level cost dashboards in enabled environments. This breaks critical usage/cost visibility and makes the route appear unavailable until cache state changes.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Set AI_GATEWAY_API_KEY to a non-empty value and restart services.
    2. Open a fresh browser tab or session and navigate directly to /default/cost.
    3. Refresh the page and observe the rendered route state.
  • Stub / mock context: Capability state was exercised in both disabled and enabled modes by toggling AI_GATEWAY_API_KEY and restarting services. Capability endpoint checks used an authenticated bypass token for setup validation, and no route interception or UI data mocking was used for the cost-page behavior itself.
  • Code analysis: I reviewed both cost route pages and the capabilities query hook. The route guard calls notFound() when isLoading is false and capabilities is null/disabled, while the query starts with initialData: null, which can present a non-loading state before real capability data arrives.
  • Why this is likely a bug: The guard can execute notFound() before capability resolution and incorrectly lock out an enabled route, which is production logic and not test-only behavior.

Relevant code:

agents-manage-ui/src/app/[tenantId]/cost/page.tsx (lines 25-29)

const { data: capabilities, isLoading: capabilitiesLoading } = useCapabilitiesQuery();

if (!capabilitiesLoading && !capabilities?.costTracking?.enabled) {
  notFound();
}

agents-manage-ui/src/app/[tenantId]/projects/[projectId]/cost/page.tsx (lines 24-28)

const { data: capabilities, isLoading: capabilitiesLoading } = useCapabilitiesQuery();

if (!capabilitiesLoading && !capabilities?.costTracking?.enabled) {
  notFound();
}

agents-manage-ui/src/lib/query/capabilities.ts (lines 13-27)

return useQuery<Capabilities | null>({
  queryKey: capabilitiesQueryKeys.current,
  async queryFn() {
    const response = await getCapabilitiesAction();
    if (!response.success || !response.data) {
      throw new Error(response.error);
    }

    return response.data;
  },
  enabled,
  initialData: null,
  initialDataUpdatedAt: 0,
  staleTime: 30_000,
});
⚠️ Mobile viewport regression on cost dashboard and model configuration
  • What failed: The pages show 404 Not Found even though the capabilities endpoint reports costTracking.enabled=true; expected behavior is to render the cost dashboard once capabilities load.
  • Impact: Users can be blocked from all cost dashboards in valid enabled environments. This hides operational usage/cost visibility and can halt troubleshooting and budget monitoring workflows.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Set AI gateway capability to enabled in the local environment and sign in to Manage UI.
    2. Open /default/cost directly on first page load.
    3. Open /default/projects/{projectId}/cost directly.
    4. Observe that one or both routes can render 404 before capabilities data resolves.
  • Stub / mock context: Capability setup used a local bypass-authenticated API check and an enabled AI gateway environment flag to keep feature gating deterministic; no route interception or mocked cost-page responses were used during navigation verification.
  • Code analysis: I reviewed the capabilities query hook and both cost page guards; the hook initializes data to null while the pages immediately gate on !isLoading && !capabilities?.costTracking?.enabled, which makes notFound() reachable before real capability data arrives.
  • Why this is likely a bug: The guard treats an unresolved null capabilities value as a definitive disabled state, so valid enabled sessions can incorrectly transition to 404 before the async fetch completes.

Relevant code:

agents-manage-ui/src/lib/query/capabilities.ts (lines 13-27)

return useQuery<Capabilities | null>({
  queryKey: capabilitiesQueryKeys.current,
  async queryFn() {
    const response = await getCapabilitiesAction();
    if (!response.success || !response.data) {
      throw new Error(response.error);
    }

    return response.data;
  },
  enabled,
  initialData: null,
  initialDataUpdatedAt: 0,
  staleTime: 30_000,
});

agents-manage-ui/src/app/[tenantId]/cost/page.tsx (lines 25-29)

const { data: capabilities, isLoading: capabilitiesLoading } = useCapabilitiesQuery();

if (!capabilitiesLoading && !capabilities?.costTracking?.enabled) {
  notFound();
}

agents-manage-ui/src/app/[tenantId]/projects/[projectId]/cost/page.tsx (lines 24-28)

const { data: capabilities, isLoading: capabilitiesLoading } = useCapabilitiesQuery();

if (!capabilitiesLoading && !capabilities?.costTracking?.enabled) {
  notFound();
}
✅ Passed (16)
Category Summary Screenshot
Adversarial Malicious custom model strings were handled as inert text with no alert/dialog execution, and the UI remained stable through reload. ADV-1
Adversarial Manual URL re-entry and navigation-based bypass attempts remained blocked, matching expected capability-gating behavior. ADV-2
Adversarial Concurrent tab saves converged to one consistent persisted state after reload, with no malformed model configuration. ADV-3
Adversarial Tampered project settings payload was handled safely (no 5xx), and settings UI remained stable after reload. ADV-4
Edge Rapid add/remove/re-add fallback interactions stayed stable with no phantom rows and persisted intended values after reload. EDGE-1
Edge Outside-click and explicit-cancel flows removed pending fallback entries, and save payload/reload contained only valid fallback models. EDGE-2
Logic Enabled mode exposed Allowed providers and Fallback models after base-model selection, and disabled mode correctly hid gated sections while retaining base model controls. LOGIC-1
Logic Fallback selector correctly limited options to gateway-routable providers and excluded disallowed provider groups. LOGIC-2
Logic Allowed provider ordering persisted after save/reload, and switching to All providers cleared allowedProviders while preserving valid fallback behavior. LOGIC-3
Logic Sub-agent pane displayed inherited base model/provider indicators and allowed switching structured/summarizer sections to specific providers with editable controls. LOGIC-4
Logic Re-execution confirmed save works when the form is dirty, and serialization included fallbackModels/allowedProviders while cleared/default fields were omitted. LOGIC-5
Happy-path Capabilities endpoint returned HTTP 200 with sandbox present and both modelFallback/costTracking disabled when the gateway key was unset. ROUTE-1
Happy-path Re-execution confirmed /manage/capabilities returned modelFallback.enabled=true and costTracking.enabled=true with gateway capability configured. ROUTE-2
Screen Tenant and project sidebars correctly hid Cost navigation when capability flags were disabled. SCREEN-1
Screen Direct deep-links to tenant and project cost routes stayed blocked and showed 404-style pages through refresh/back-forward checks. SCREEN-2
Screen Tenant and project cost pages rendered successfully and showed the Cost by Provider section with a valid empty state. SCREEN-3

Commit: d2b86a7

View Full Run


Tell us how we did: Give Ito Feedback

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

Re-review scope: This review covers the delta since the previous automated approval (commit d2b86a7348d67eb26b). The delta consists of 4 commits including a main merge and feature enhancements.

✅ Addressing @tim-inkeep's .includes Concern

The delta already fixes the .includesSet.has() pattern throughout the codebase:

File Fix Applied
model-selector.tsx:233 GATEWAY_ROUTABLE_PROVIDERS.includes(provider)GATEWAY_ROUTABLE_PROVIDERS_SET.has(provider)
model-configuration.tsx:46 effectiveProviders.includes(p.value)effectiveSet.has(p.value)
model-factory.ts:209 GATEWAY_ROUTABLE_PROVIDERS.includes(provider)GATEWAY_ROUTABLE_PROVIDERS_SET.has(provider)
usage-cost-middleware.ts:51 GATEWAY_ROUTABLE_PROVIDERS.includes(prefix)GATEWAY_ROUTABLE_PROVIDERS_SET.has(prefix)

The remaining .includes('/') on line 204 of model-selector.tsx is a string method (checking if a model value contains a / character), NOT an array membership check — this is correct usage.

✅ Delta Analysis

The delta includes well-implemented changes:

File Change Assessment
model-resolver.ts New inheritGatewayFields helper and resolveModelConfig function for gateway field inheritance ✅ Clean implementation with proper parent → child inheritance for fallbackModels and allowedProviders
model-resolver.test.ts 150+ lines of comprehensive test coverage ✅ Excellent coverage including inheritance, edge cases, and error handling
metadata-editor.tsx Wires fallbackModels and allowedProviders to agent-level model configuration ✅ Proper form integration with inherited values
model-configuration.tsx Adds isGatewayRoutable check to conditionally show Allowed Providers and Fallback Models sections ✅ Correct gating — only gateway-routable models (anthropic, openai, google, gateway) show these features
models.mdx (docs) Major documentation update with new "AI Gateway" section covering fallback models, allowed providers, and inheritance ✅ Clear, comprehensive documentation with code examples
sub-agents.mdx (docs) Minor doc update referencing gateway features ✅ Consistent
constants/models.ts Exports GATEWAY_ROUTABLE_PROVIDERS_SET as a ReadonlySet<string> ✅ O(1) lookup vs O(n) for array .includes

Code Quality Highlights

  1. resolveModelConfig inheritance logic — Clean implementation that inherits fallbackModels and allowedProviders from parent configurations (project → agent → sub-agent) even when the child overrides the base model. This enables DRY configuration.

  2. Set-based membership testing — Converting from array .includes() to Set.has() provides O(1) lookup performance, which matters for frequently-called code paths like model routing.

  3. Documentation — The new "AI Gateway" section in models.mdx is comprehensive and well-structured, covering:

    • Fallback models with inheritance semantics
    • Allowed providers with priority ordering
    • Combined usage patterns
    • Advanced manual gateway options

🧹 While You're Here (1)

🧹 1) model-factory.ts:287,291 Pre-existing .includes on BUILT_IN_PROVIDERS array

Issue: The parseModelString method still uses .includes() on the BUILT_IN_PROVIDERS array for provider validation. This is pre-existing code (not introduced by this PR) but follows the same pattern that was fixed elsewhere.

Why: While BUILT_IN_PROVIDERS is small (9 items), consistency with the rest of the codebase suggests using a Set for O(1) lookup.

Fix: Consider adding a BUILT_IN_PROVIDERS_SET alongside the existing constant for membership checks in a follow-up PR.

Refs: model-factory.ts:268-278


✅ APPROVE

Summary: The delta successfully addresses @tim-inkeep's concern about .includes usage — all gateway provider membership checks now use Set.has() for O(1) performance. The new resolveModelConfig module with its inheritance logic is well-tested and cleanly implemented. Documentation is comprehensive. The feature is ready to ship! 🚀

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
Orchestrator analysis only 0 0 1 0 0 0

Note: Sub-reviewers were not dispatched for this delta review as the changes are focused and follow established patterns. The delta primarily adds gateway field inheritance and converts .includes to Set.has().

@github-actions github-actions Bot deleted a comment from claude Bot Mar 31, 2026
@tim-inkeep tim-inkeep enabled auto-merge March 31, 2026 17:28
@tim-inkeep tim-inkeep disabled auto-merge March 31, 2026 17:30
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

Re-review scope: This review covers the delta since the previous automated approval (commit 8d67eb26bee38236a6). The delta is a single commit improving documentation wording.

✅ Delta Analysis

The commit (ee38236a6 — "updated docs") is a minor documentation improvement:

File Change Assessment
agents-docs/content/typescript-sdk/models.mdx Updated the "Combining Both" section description from "This tries claude-sonnet-4-5 via Bedrock, then Anthropic. If both fail, it falls through to the fallback models in order:" to "With this configuration, allowedProviders restricts and prioritizes which providers can serve the primary model, while fallbackModels defines alternative models to try if the primary model is unavailable:" ✅ Clearer explanation of the interaction between allowedProviders and fallbackModels

This is a pure documentation clarification with no code changes.

✅ All Prior Issues Addressed

All issues from prior reviews have been resolved:

Issue Status
Empty string validation for fallbackModels ✅ Fixed with z.string().nonempty()
Project-level models serialization ✅ Fixed in project-form.tsx
Agent-level models serialization ✅ Fixed in serialize.ts
Accessibility labels on buttons ✅ Added aria-label attributes
Escape key orphan pending row ✅ Fixed with onClose callback
Inherited fallback settings protection ✅ Fixed with disabled={disabled || isInherited}
GATEWAY_ROUTABLE_PROVIDERS deduplication ✅ Consolidated in constants/models.ts
Cost route deep-link gating ✅ Added notFound() when capability disabled
Model ID normalization in telemetry ✅ Added normalizeModelId()
useEffect simplification (human reviewer) ✅ Fixed per @dimaMachina's feedback
.includesSet.has() (human reviewer) ✅ Fixed per @tim-inkeep's feedback

💭 Consider (1) 💭

💭 1) [tenantId]/cost/page.tsx:27-29 and [projectId]/cost/page.tsx:26-28 Cost route capability guard race condition

Issue: The Ito test report identified a potential race condition where cost pages can show 404 during initial capability loading. The guard pattern:

if (!capabilitiesLoading && !capabilities?.costTracking?.enabled) {
  notFound();
}

can trigger 404 if capabilities is null (initial state from useCapabilitiesQuery) before the fetch completes.

Why: The capabilities query uses initialData: null with initialDataUpdatedAt: 0. During the brief window between hook initialization and query execution, isLoading could be false while data is still null, causing an incorrect 404.

Fix (optional): Add an explicit loading state or check for capabilities !== null:

// Option A: Show nothing while loading
if (capabilitiesLoading) return null;
if (!capabilities?.costTracking?.enabled) notFound();

// Option B: Only check when capabilities are resolved
if (capabilities !== null && !capabilities.costTracking?.enabled) notFound();

Refs:


✅ APPROVE

Summary: The delta is a documentation-only change that improves the explanation of how allowedProviders and fallbackModels interact. All code issues from prior reviews have been addressed. The Ito-identified race condition in cost page capability gating is noted as a "Consider" item — it's an edge case during initial load that could be addressed as a follow-up if it causes user friction. The PR is ready to ship! 🚀

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
Orchestrator analysis only 0 1 0 0 0 0

Note: Sub-reviewers were not dispatched for this delta review as the change is a minimal documentation update (~2 lines changed).

@github-actions
Copy link
Copy Markdown
Contributor

🔎💬 Inkeep AI search and chat service is syncing content for source 'Inkeep Agent Framework Docs'

@itoqa
Copy link
Copy Markdown

itoqa Bot commented Mar 31, 2026

Ito Test Report ❌

18 test cases ran. 1 failed, 17 passed.

Across 18 test cases, 17 passed and 1 failed, indicating generally strong stability with capability/auth, navigation gating, cost dashboard rendering, model configuration flows, and advanced settings behaviors all largely validated in local verification. The single confirmed defect is a medium-severity issue in Project Settings where switching from a gateway-routable base model to a non-gateway model hides gateway-only controls but still submits stale fallbackModels and allowedProviders, while all other critical checks (including 401 protection on unauthenticated capabilities access, correct AI_GATEWAY_API_KEY-driven flag behavior, deep-link blocking when costTracking is disabled, and safe handling of malicious model text) behaved as expected.

❌ Failed (1)
Category Summary Screenshot
Edge 🟠 Switching base model from gateway-routable to non-routable hides gateway-only controls but still submits stale fallbackModels and allowedProviders values. EDGE-4
🟠 Gateway-only fields persist after switching to non-gateway model
  • What failed: Gateway-only controls disappear after the model switch, but previously selected fallbackModels and allowedProviders remain in state and are still serialized into the save request. Expected behavior is that hidden, inapplicable gateway-only fields are cleared or excluded.
  • Impact: Projects can persist stale gateway routing config that no longer matches the selected base model provider. This can cause incorrect or misleading runtime routing behavior and makes saved configuration harder to trust.
  • Introduced by this PR: Yes – this PR modified the relevant code
  • Steps to reproduce:
    1. Open project settings and configure base model with gateway-only fallback/providers.
    2. Switch the base model to a non-gateway model such as azure/*.
    3. Click Update project and inspect the outbound payload for stale fallbackModels and allowedProviders.
  • Stub / mock context: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code analysis: I reviewed model change handling and serialization in the manage UI. The model-switch handler clears only providerOptions, while gateway-only sections are merely hidden based on provider type; serialization then includes any non-empty fallback/provider arrays without checking current model routability.
  • Why this is likely a bug: The current implementation hides gateway-only controls when the model is non-gateway but does not clear those fields, and serialization still emits them, so invalid stale configuration is persisted by design.

Relevant code:

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 385-400)

const handleModelChange = (modelValue: string) => {
  const previousEffectiveModel = value || inheritedValue;
  const newModel = modelValue || undefined;
  const wasInherited = !value && !!inheritedValue;
  const isNowExplicit = !!newModel;

  if (previousEffectiveModel !== newModel || (wasInherited && isNowExplicit)) {
    setInternalProviderOptions(undefined);
    onProviderOptionsChange?.('');
  }

  onModelChange?.(newModel || '');
};

agents-manage-ui/src/components/shared/model-configuration.tsx (lines 497-522)

{capabilities?.modelFallback?.enabled &&
  effectiveModel &&
  isGatewayRoutable &&
  onAllowedProvidersChange && (
    <AllowedProvidersSection
      allowedProviders={allowedProviders}
      inheritedAllowedProviders={inheritedAllowedProviders}
      onAllowedProvidersChange={onAllowedProvidersChange}
      disabled={disabled}
    />
  )}

{capabilities?.modelFallback?.enabled &&
  effectiveModel &&
  isGatewayRoutable &&
  onFallbackModelsChange && (
    <FallbackModelsSection
      editorNamePrefix={editorNamePrefix}
      fallbackModels={fallbackModels}
      inheritedFallbackModels={inheritedFallbackModels}
      onFallbackModelsChange={onFallbackModelsChange}
      disabled={disabled}
    />
  )}

agents-manage-ui/src/components/projects/form/project-form.tsx (lines 65-73)

base: {
  model: data.models.base.model,
  providerOptions: cleanProviderOptions(data.models.base.providerOptions),
  ...(data.models.base.fallbackModels?.length && {
    fallbackModels: data.models.base.fallbackModels.filter(Boolean),
  }),
  ...(data.models.base.allowedProviders?.length && {
    allowedProviders: data.models.base.allowedProviders.filter(Boolean),
  }),
},
✅ Passed (17)
Category Summary Screenshot
Adversarial Malicious custom model IDs were stored and rendered as inert text after save/reload, with no script execution observed. ADV-1
Adversarial Unauthenticated GET /manage/capabilities returned 401 and did not expose capability payload fields. ADV-2
Adversarial Manual URL entry, back/forward, and refresh attempts could not bypass disabled cost gating. ADV-3
Adversarial Two-tab stale saves converged to a coherent final configuration after reload with no crash or console errors. ADV-4
Edge Canceling pending fallback selection by outside click, Escape, and cancel icon left no phantom rows and preserved contiguous numbering. EDGE-1
Edge Drag-and-drop provider priority persisted in both save payload order and reloaded UI order. EDGE-2
Edge Rapid fallback/provider interactions stayed stable with no crashes and no invalid empty entries in saved arrays. EDGE-3
Edge At mobile viewport 390x844, model controls stayed reachable and the save action completed successfully. EDGE-5
Logic Project settings save produced valid JSON with correctly typed fallback and provider arrays for configured model slots. LOGIC-1
Logic Re-run confirmed inheritance display and explicit agent model overrides persist correctly in saved payloads after reload. LOGIC-2
Logic Re-execution passed: saving sub-agent model config forwarded fallback model and allowed provider fields in the update payload. LOGIC-3
Happy-path Verified expected disabled capability flags when the gateway key is absent; earlier blockage was environment-related, not an app defect. ROUTE-1
Happy-path With AI_GATEWAY_API_KEY configured, capabilities returned modelFallback.enabled=true and costTracking.enabled=true. ROUTE-2
Screen Cost links are correctly shown in org and project sidebars when capability flags are enabled. SCREEN-1
Screen Not a real application bug; re-execution confirms org cost deep-link is hard-blocked when capability is disabled. SCREEN-2
Screen Project cost deep-link remained blocked with 404/not-found UI and no cost dashboard content. SCREEN-3
Screen Cost dashboard renders provider breakdown and provider column with expected provider precedence. SCREEN-4

Commit: ee38236

View Full Run


Tell us how we did: Give Ito Feedback

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.

2 participants