fix(agents): surface model catalog warning in sessions_spawn when override model is not in catalog#36191
Conversation
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
There was a problem hiding this comment.
💡 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".
| (e: ModelCatalogEntry) => | ||
| e.provider.toLowerCase() === mProvider.toLowerCase() && | ||
| e.id.toLowerCase() === mModel.toLowerCase(), |
There was a problem hiding this comment.
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 SummaryThis PR adds a Key findings:
Confidence Score: 3/5
Last reviewed commit: e81609c |
| 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(), | ||
| ); |
There was a problem hiding this 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:
| 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.
Summary
sessions_spawnreturnsmodelApplied: truewhen 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: trueis actively misleading — it means "session store was updated" not "model will actually be used."sessions.patchsucceeds with a model override, a catalog lookup validates whether the model exists. If not, amodelWarningfield is added to the spawn result explaining that runtime fallback may occur.modelAppliedsemantics 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)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
sessions_spawnresult now includes an optionalmodelWarningfield 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)
Repro + Verification
Environment
Steps
sessions_spawn({ task: "Identify yourself", model: "google/gemini-3.1-flash-lite-preview" })Expected
modelWarningfield present when model is not in catalogActual
modelApplied: true, no warning, silent fallback to AnthropicmodelApplied: true,modelWarning: "Model ... not in the runtime model catalog ...", caller has visibilityEvidence
7 vitest unit tests covering:
modelWarningsetsplitModelRefedge cases (slash parsing, empty input)Human Verification (required)
Compatibility / Migration
modelWarningis a new optional field; existing consumers ignore it.Failure Recovery (if this breaks)
modelWarningfield disappears from spawn results.src/agents/subagent-spawn.tsRisks and Mitigations
loadModelCatalogcall adds latency to spawn flow.