Skip to content

fix(agents): surface model catalog warning in sessions_spawn when override model is not in catalog#36191

Closed
Sid-Qin wants to merge 1 commit intoopenclaw:mainfrom
Sid-Qin:fix/spawn-model-override-gemini-36134
Closed

fix(agents): surface model catalog warning in sessions_spawn when override model is not in catalog#36191
Sid-Qin wants to merge 1 commit intoopenclaw:mainfrom
Sid-Qin:fix/spawn-model-override-gemini-36134

Conversation

@Sid-Qin
Copy link
Copy Markdown
Contributor

@Sid-Qin Sid-Qin commented Mar 5, 2026

Summary

  • Problem: sessions_spawn returns modelApplied: true when the session patch succeeds, but the model may not exist in the runtime model catalog. The agent silently falls back to the default model at runtime (e.g., Sonnet instead of Gemini), and the caller has no indication. modelApplied: true is actively misleading — it means "session store was updated" not "model will actually be used."
  • Why it matters: Users running multi-model pipelines (e.g., cost-optimized Guardian workflows using Gemini 3.1 Flash Lite at $1.50/MTok) unknowingly run on Sonnet ($15/MTok) — a 10x cost increase with zero visibility.
  • What changed: After sessions.patch succeeds with a model override, a catalog lookup validates whether the model exists. If not, a modelWarning field is added to the spawn result explaining that runtime fallback may occur.
  • What did NOT change (scope boundary): modelApplied semantics are preserved (still reflects session-patch success). No changes to model resolution, fallback routing, or gateway API. The underlying model resolution for Gemini 3.1 is addressed separately in fix(agents): add google/google-vertex forward-compat for gemini-3.1 fallback models #36173.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

sessions_spawn result now includes an optional modelWarning field when the overridden model is not found in the runtime model catalog. Example:

{
  "status": "accepted",
  "modelApplied": true,
  "modelWarning": "Model \"google/gemini-3.1-flash-lite-preview\" was applied to the session but is not in the runtime model catalog. The agent may fall back to the default model at runtime."
}

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: macOS / Linux
  • Runtime: Node.js 22+

Steps

  1. Configure OpenClaw with anthropic and google auth profiles
  2. Call sessions_spawn({ task: "Identify yourself", model: "google/gemini-3.1-flash-lite-preview" })
  3. Check spawn result

Expected

  • modelWarning field present when model is not in catalog

Actual

  • Before: modelApplied: true, no warning, silent fallback to Anthropic
  • After: modelApplied: true, modelWarning: "Model ... not in the runtime model catalog ...", caller has visibility

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

7 vitest unit tests covering:

  • Model not in catalog → modelWarning set
  • Model in catalog → no warning
  • No model override → no warning
  • splitModelRef edge cases (slash parsing, empty input)

Human Verification (required)

  • Verified scenarios: Catalog miss detection, catalog hit (no warning), no model override path.
  • Edge cases checked: Empty/undefined model refs, bare model names without provider prefix.
  • What you did not verify: Live multi-agent pipeline with actual Gemini 3.1 API (requires production credentials and Google auth).

Compatibility / Migration

  • Backward compatible? Yes — modelWarning is a new optional field; existing consumers ignore it.
  • Config/env changes? No
  • Migration needed? No

Failure Recovery (if this breaks)

  • How to disable/revert: Revert this commit. modelWarning field disappears from spawn results.
  • Files/config to restore: src/agents/subagent-spawn.ts

Risks and Mitigations

  • Risk: loadModelCatalog call adds latency to spawn flow.
    • Mitigation: The catalog is cached at module scope after first load. The check is wrapped in try/catch so catalog load failures do not block spawn.
  • Risk: Forward-compat models (resolved at runtime but not in catalog) trigger false positive warnings.

sessions_spawn returns modelApplied: true when the session patch
succeeds, but does not verify that the model exists in the runtime
catalog. When a model like google/gemini-3.1-flash-lite-preview is
not in the catalog, the agent silently falls back to the default
model at runtime while reporting modelApplied: true — giving the
caller no indication that their override was ineffective.

Add a post-patch catalog check that sets a modelWarning field on
the spawn result when the overridden model is not found in the
runtime model catalog. This gives callers clear visibility into
potential runtime fallback behavior.

Closes openclaw#36134

Made-with: Cursor
Copy link
Copy Markdown

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

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: e81609c524

ℹ️ 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 on lines +455 to +457
(e: ModelCatalogEntry) =>
e.provider.toLowerCase() === mProvider.toLowerCase() &&
e.id.toLowerCase() === mModel.toLowerCase(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize model ref before catalog presence check

The catalog lookup compares resolvedModel pieces directly (splitModelRef) against ModelCatalogEntry.id/provider, so overrides like openai/gpt-5@work (auth-profile suffix) are treated as missing even when openai/gpt-5 exists, and modelWarning is incorrectly emitted. This is inconsistent with runtime model resolution, which strips trailing auth profiles via splitTrailingAuthProfile in resolveModelRefFromString (src/agents/model-selection.ts), so callers can get a false fallback warning for valid models.

Useful? React with 👍 / 👎.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR adds a modelWarning field to the SpawnSubagentResult type, populated after a successful sessions.patch when the user-overridden model is not found in the runtime model catalog. This gives callers visibility into silent fallback situations (e.g., Gemini configured but Sonnet used at runtime) without changing existing modelApplied semantics or blocking spawn.

Key findings:

  • New optional modelWarning?: string field added to SpawnSubagentResult and populated in spawnSubagentDirect when a model override is not found in the catalog.
  • New exported splitModelRef utility parses "provider/model" refs; tests cover slash-separated, bare, and empty inputs.
  • Catalog lookup is wrapped in try/catch so failures do not block spawn — appropriate for a best-effort advisory check.
  • Auth-profile suffix bug: splitModelRef does not strip trailing @profile segments (e.g., "google/gemini-flash@my-profile") before comparing the model ID against e.id in the catalog. This produces a false-positive warning for valid model refs that carry an auth-profile suffix. The existing splitTrailingAuthProfile helper from ./model-ref-profile.ts should be applied to mModel before the catalog comparison.

Confidence Score: 3/5

  • Safe for core spawn behavior, but has a false-positive warning bug for auth-profile model refs that should be fixed before wider rollout.
  • The core logic — patching the session then doing a best-effort catalog lookup — is correct and non-blocking. The auth-profile suffix edge case (e.g., "google/gemini-flash@my-profile") causes the catalog ID comparison to include @my-profile in the match, resulting in a false-positive modelWarning for users who legitimately use auth-profile suffixes. This is not a regression in spawn behavior (the session is still patched correctly), but it does undermine the accuracy of the new warning signal, which is the primary value this PR delivers.
  • src/agents/subagent-spawn.ts — the catalog ID comparison at lines 450–457 needs splitTrailingAuthProfile applied to strip auth-profile suffixes before matching.

Last reviewed commit: e81609c

Comment on lines +450 to +458
const { provider: mProvider, model: mModel } = splitModelRef(resolvedModel);
if (mProvider && mModel) {
try {
const catalog = await loadModelCatalog({ config: cfg });
const inCatalog = catalog.some(
(e: ModelCatalogEntry) =>
e.provider.toLowerCase() === mProvider.toLowerCase() &&
e.id.toLowerCase() === mModel.toLowerCase(),
);
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.

Auth-profile suffix causes false-positive catalog miss warning

splitModelRef does not strip trailing @profile suffixes before comparing against catalog IDs. If a caller passes a model ref with an auth profile (e.g., "google/gemini-flash@my-profile"), mModel becomes "gemini-flash@my-profile". This will never match the clean catalog entry "gemini-flash", producing an incorrect warning even though the model is valid.

splitTrailingAuthProfile already exists in ./model-ref-profile.ts for exactly this purpose — it should be called on mModel before the catalog comparison:

Suggested change
const { provider: mProvider, model: mModel } = splitModelRef(resolvedModel);
if (mProvider && mModel) {
try {
const catalog = await loadModelCatalog({ config: cfg });
const inCatalog = catalog.some(
(e: ModelCatalogEntry) =>
e.provider.toLowerCase() === mProvider.toLowerCase() &&
e.id.toLowerCase() === mModel.toLowerCase(),
);
const { provider: mProvider, model: mModelRaw } = splitModelRef(resolvedModel);
if (mProvider && mModelRaw) {
try {
const { model: mModel } = splitTrailingAuthProfile(mModelRaw);
const catalog = await loadModelCatalog({ config: cfg });
const inCatalog = catalog.some(
(e: ModelCatalogEntry) =>
e.provider.toLowerCase() === mProvider.toLowerCase() &&
e.id.toLowerCase() === mModel.toLowerCase(),
);

(You will also need to import splitTrailingAuthProfile from "./model-ref-profile.ts" at the top of the file.)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/subagent-spawn.ts
Line: 450-458

Comment:
Auth-profile suffix causes false-positive catalog miss warning

`splitModelRef` does not strip trailing `@profile` suffixes before comparing against catalog IDs. If a caller passes a model ref with an auth profile (e.g., `"google/gemini-flash@my-profile"`), `mModel` becomes `"gemini-flash@my-profile"`. This will never match the clean catalog entry `"gemini-flash"`, producing an incorrect warning even though the model is valid.

`splitTrailingAuthProfile` already exists in `./model-ref-profile.ts` for exactly this purpose — it should be called on `mModel` before the catalog comparison:

```suggestion
      const { provider: mProvider, model: mModelRaw } = splitModelRef(resolvedModel);
      if (mProvider && mModelRaw) {
        try {
          const { model: mModel } = splitTrailingAuthProfile(mModelRaw);
          const catalog = await loadModelCatalog({ config: cfg });
          const inCatalog = catalog.some(
            (e: ModelCatalogEntry) =>
              e.provider.toLowerCase() === mProvider.toLowerCase() &&
              e.id.toLowerCase() === mModel.toLowerCase(),
          );
```

(You will also need to import `splitTrailingAuthProfile` from `"./model-ref-profile.ts"` at the top of the file.)

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: sessions_spawn silently falls back to Anthropic (set fallback model) when targeting Google Gemini models on spawned subagent tasks.

2 participants