fix(agents): preserve qualified model alias targets by mushuiyu886 · Pull Request #88237 · openclaw/openclaw

mushuiyu886 · 2026-05-30T05:08:47Z

Summary

Problem: a fully qualified agents.defaults.model.primary value could be reverse-matched through a bare alias target in agents.defaults.models, causing the resolver to return openai/<alias-key> instead of preserving the configured provider/model ref.
Solution: only use a reverse alias key for slash-form primary refs when the alias key is also slash-form; bare aliases no longer override explicit provider/model refs.
What changed: src/agents/model-selection-shared.ts now preserves slash-form primary refs that match bare alias targets, with regression coverage in src/agents/model-selection.test.ts.
What did NOT change: slash-form alias keys still normalize slash-form alias targets as before.

Real behavior proof

Behavior or issue addressed: agents.defaults.model.primary: "nemotron-bolt/nemotron-3-super-120b" is no longer re-resolved through bare alias key nemotron to openai/nemotron.
Real environment tested: Linux worktree at issue-88218, Node via repository test runner, production resolveConfiguredModelRef called directly with a representative config.

Exact steps or command run after this patch:

node --import tsx --input-type=module <<'JS'
import { resolveConfiguredModelRef } from "./src/agents/model-selection-shared.ts";

const cfg = {
  agents: {
    defaults: {
      model: { primary: "nemotron-bolt/nemotron-3-super-120b" },
      models: {
        nemotron: { alias: "nemotron-bolt/nemotron-3-super-120b" },
      },
    },
  },
  models: {
    providers: {
      "nemotron-bolt": {
        api: "openai-completions",
        models: [{ id: "nemotron-3-super-120b", name: "Nemotron" }],
      },
    },
  },
};

const resolved = resolveConfiguredModelRef({
  cfg,
  defaultProvider: "openai",
  defaultModel: "gpt-5.4",
});
console.log(JSON.stringify({
  primary: cfg.agents.defaults.model.primary,
  aliasKey: "nemotron",
  aliasTarget: cfg.agents.defaults.models.nemotron.alias,
  resolved,
}, null, 2));
JS

Evidence after fix: Copied terminal console output from the command above:

{
  "primary": "nemotron-bolt/nemotron-3-super-120b",
  "aliasKey": "nemotron",
  "aliasTarget": "nemotron-bolt/nemotron-3-super-120b",
  "resolved": {
    "provider": "nemotron-bolt",
    "model": "nemotron-3-super-120b"
  }
}

Observed result after fix: the resolver preserves the configured nemotron-bolt provider and nemotron-3-super-120b model instead of returning the bare alias under the default provider.
What was not tested: a live provider call; the bug is in local model-reference resolution before any provider request is made.

Regression Test Plan

Coverage level: unit tests plus direct production-function proof.
Target test file: src/agents/model-selection.test.ts.
Scenario locked in: slash-form primary refs matching a bare alias target are preserved.
Why this is the smallest reliable guardrail: it exercises the resolver branch that previously preferred the bare reverse alias over the explicit provider/model ref.

Verification

node scripts/run-vitest.mjs src/agents/model-selection.test.ts src/agents/model-selection-resolve.test.ts src/commands/models/set.test.ts src/commands/models/list.configured.test.ts
node scripts/run-oxlint.mjs src/agents/model-selection-shared.ts src/agents/model-selection.test.ts
git diff --check
pnpm changed:lanes --json
pnpm check:changed

Root Cause

Root cause: reverse alias matching ran before preserving explicit slash-form refs, so a bare alias key could replace an already-qualified provider/model target.
Missing detection / guardrail: existing alias tests covered slash-form alias keys, but not bare alias keys whose target exactly matched a configured slash-form primary ref.

clawsweeper · 2026-05-30T05:11:50Z

Codex review: needs maintainer review before merge. Reviewed May 30, 2026, 12:11 PM ET / 16:11 UTC.

Summary
The PR changes resolveConfiguredModelRef so a slash-form primary model ref is preserved when it matches a bare configured alias target, and adds a regression test.

PR surface: Source +3, Tests +34. Total +37 across 2 files.

Reproducibility: yes. current-main source inspection shows the resolver reverse-matches alias targets before parsing slash-form primary refs, matching the linked report. I did not execute tests because this review is read-only.

Review metrics: 1 noteworthy metric.

Model-default precedence: 1 resolver precedence changed. The change affects which provider/model wins for existing configs where a primary ref equals an alias target, so maintainers should notice the upgrade behavior before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Have a maintainer choose this PR or fix(models): prefer exact configured provider refs before aliases #88232 as the canonical fix and explicitly accept the resolver precedence change.

Risk before merge

[P1] This intentionally changes upgrade behavior for configs where a slash-form agents.defaults.model.primary equals a bare agents.defaults.models alias target; maintainers should accept that explicit provider/model refs win over bare reverse aliases.
[P1] fix(models): prefer exact configured provider refs before aliases #88232 targets the same linked issue with a broader competing fix, so one canonical PR should be selected before either lands.

Maintainer options:

Choose One Canonical Fix
Select this PR or fix(models): prefer exact configured provider refs before aliases #88232 as the canonical landing path, then close or rebase the other so the resolver precedence change lands once.
Accept Explicit-Ref Precedence
If maintainers agree that an already-qualified primary ref should beat a bare reverse alias target, merge the selected PR after the normal checks and owner review.
Prefer The Broader Sibling
If the broader test coverage or exact-provider approach in the sibling PR is preferred, pause or close this PR after linking the canonical path.

Next step before merge

[P2] Needs maintainer selection between this PR and the sibling fix, plus explicit approval of the compatibility-sensitive provider/model precedence change.

Security
Cleared: The diff only changes resolver logic and a colocated unit test; it does not touch dependencies, workflows, secrets, install scripts, or other supply-chain surfaces.

Review details

Best possible solution:

Land one canonical fix that preserves already-qualified primary refs ahead of bare reverse aliases, with regression coverage and explicit maintainer acceptance of the config-resolution precedence.

Do we have a high-confidence way to reproduce the issue?

Yes: current-main source inspection shows the resolver reverse-matches alias targets before parsing slash-form primary refs, matching the linked report. I did not execute tests because this review is read-only.

Is this the best way to solve the issue?

Mostly yes: the patch is a narrow maintainable fix that preserves explicit slash-form refs while keeping slash-form alias behavior. The remaining question is maintainer selection between this PR and the sibling fix plus explicit acceptance of the precedence change.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4155ac1c0d7c.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a representative production-function resolver call showing the configured provider/model is preserved.

Label justifications:

P1: The linked bug can route valid custom-provider agent configs to openai/<alias-key> and break startup/model selection for real users.
merge-risk: 🚨 compatibility: The PR changes precedence for existing agents.defaults.model.primary and agents.defaults.models configurations that previously reverse-matched alias targets.
merge-risk: 🚨 auth-provider: The changed resolver path can alter the selected provider/model ref, which affects provider routing and auth-provider selection before runtime calls.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a representative production-function resolver call showing the configured provider/model is preserved.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a representative production-function resolver call showing the configured provider/model is preserved.

Evidence reviewed

PR surface:

Source +3, Tests +34. Total +37 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	4	1	+3
Tests	1	34	0	+34
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	38	1	+37

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/agents/model-selection.test.ts src/agents/model-selection-resolve.test.ts src/commands/models/set.test.ts src/commands/models/list.configured.test.ts.
[P1] node scripts/run-oxlint.mjs src/agents/model-selection-shared.ts src/agents/model-selection.test.ts.
[P1] git diff --check.

What I checked:

Root policy read: Root AGENTS.md was read fully; it marks provider routing, auth/provider selection, config defaults, and fallback behavior as compatibility-sensitive review surfaces. (AGENTS.md:21, 4155ac1c0d7c)
Scoped agents policy read: The touched files are under src/agents, and the scoped guide was read fully; it mainly adds agent test performance guidance and does not conflict with this review. (src/agents/AGENTS.md:1, 4155ac1c0d7c)
Current-main source behavior: Current main finds a reverse alias candidate before the slash-form primary ref is parsed, so a matching bare alias key can be parsed under the default provider before the explicit provider/model ref path runs. (src/agents/model-selection-shared.ts:689, 4155ac1c0d7c)
Explicit ref parsing path: The existing parser can preserve configured provider refs via resolveExactConfiguredProviderRef, so skipping the bare reverse alias for slash-form primaries reaches the intended provider/model parsing path. (src/agents/model-selection-shared.ts:367, 4155ac1c0d7c)
PR implementation diff: The PR gates reverse alias resolution so it only applies to bare primary refs or slash-form alias keys, preserving slash-form primary refs that collide with bare alias targets. (src/agents/model-selection-shared.ts:692, 5ef1d873ee95)
Regression coverage: The PR adds a focused test for nemotron-bolt/nemotron-3-super-120b as a slash-form primary matching a bare nemotron alias target, expecting the custom provider/model to be preserved. (src/agents/model-selection.test.ts:1855, 5ef1d873ee95)

Likely related people:

steipete: Peter Steinberger has the heaviest history on the shared model-selection files, including the commit that introduced src/agents/model-selection-shared.ts and later allowlist parsing refactors. (role: feature-history owner; confidence: high; commits: 310d2db31241, 3eed32108187, 27ae826f6525; files: src/agents/model-selection-shared.ts, src/agents/model-selection.test.ts)
vincentkoc: Current-main blame for the resolver region points at Vincent Koc's recent model metadata commit that carried the current shared file and tests into main. (role: recent area contributor; confidence: medium; commits: e780a6b7baca; files: src/agents/model-selection-shared.ts, src/agents/model-selection.test.ts)
Steven Palmer: Steven Palmer authored the related open PR that changes the same resolver behavior for the same linked alias-target precedence bug. (role: parallel fix author; confidence: medium; commits: 2886e87f683b; files: src/agents/model-selection-shared.ts, src/agents/model-selection.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-05-30T17:46:37Z

Closing this as a duplicate of #88232, which landed the canonical fix for #88218 in 86e33d6.

Thanks for jumping on the same model alias precedence issue.

fix(agents): preserve qualified model alias targets

5ef1d87

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026

openclaw-barnacle Bot removed the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 30, 2026

clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label May 30, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 30, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 30, 2026

clawsweeper Bot mentioned this pull request May 30, 2026

fix(models): prefer exact configured provider refs before aliases #88232

Merged

Merge branch 'main' into feat/issue-88218

6c475bf

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 30, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 30, 2026

steipete closed this May 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): preserve qualified model alias targets#88237

fix(agents): preserve qualified model alias targets#88237
mushuiyu886 wants to merge 2 commits into
openclaw:mainfrom
mushuiyu886:feat/issue-88218

mushuiyu886 commented May 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 30, 2026 •

edited

Loading

Uh oh!

steipete commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

mushuiyu886 commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Regression Test Plan

Verification

Root Cause

Uh oh!

clawsweeper Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mushuiyu886 commented May 30, 2026 •

edited

Loading

clawsweeper Bot commented May 30, 2026 •

edited

Loading