fix #84887: avoid duplicate provider prefix in runtime LLM allowlist diagnostics by zhangguiping-xydt · Pull Request #84946 · openclaw/openclaw

zhangguiping-xydt · 2026-05-21T12:24:14Z

Summary

Problem: when api.runtime.llm.complete resolves a selection whose modelId already starts with the provider segment (e.g. OpenRouter resolves provider="openrouter", modelId="openrouter/gpt-5.4-mini"), the runtime LLM policy path interpolated ${provider}/${model} after normalizeModelRef(), producing openrouter/openrouter/gpt-5.4-mini. The malformed ref both missed the user's allowlist and showed up in the denial diagnostic, sending plugin authors chasing the wrong allowlist key.
Solution: route both the allowlist normalization site and the resolved-override formatting through the existing modelKey() helper, which leaves the model id untouched when it already starts with the provider segment.
What changed: src/plugins/runtime/runtime-llm.runtime.ts (import + two interpolation sites); regression coverage added in src/plugins/runtime/runtime-llm.runtime.test.ts.
What did NOT change: normalizeModelRef() semantics, the parallel normalizeAllowedModelRef in src/gateway/server-plugins.ts (subagent fallback policy), and any other model-ref formatter — kept the change scoped to the runtime LLM policy path the issue narrowed.

Real behavior proof

Behavior or issue addressed: plugin runtime LLM allowlist denial diagnostics reported openrouter/openrouter/gpt-5.4-mini for an actionable openrouter/gpt-5.4-mini selection, leading users to add the wrong allowlist entries.
Real environment tested: Linux 4.19 x86_64, Node v22.22.0, OpenClaw worktree at HEAD ee9f3b6 (rebased onto origin/main bde07dd).

Exact steps or command run after this patch:

pnpm install
node --import tsx .automation/proof_repro.ts
node scripts/run-vitest.mjs run src/plugins/runtime/runtime-llm.runtime.test.ts

proof_repro.ts imports the real source helpers and exercises the exact normalization path the runtime LLM policy uses:

import { modelKey } from "../src/agents/model-ref-shared.ts";
import { normalizeModelRef } from "../src/agents/model-selection-normalize.ts";
const normalized = normalizeModelRef("openrouter", "openrouter/gpt-5.4-mini");
const oldRef = `${normalized.provider}/${normalized.model}`;     // pre-fix interpolation
const newRef = modelKey(normalized.provider, normalized.model);  // post-fix
const allowlist = new Set(["openrouter/gpt-5.4-mini"]);
console.log({ oldRef, newRef, oldHit: allowlist.has(oldRef), newHit: allowlist.has(newRef) });

Evidence after fix:

Issue #84887 — runtime LLM allowlist double-prefix repro

--- OpenRouter provider-qualified model id (the bug shape) ---
  input:                provider=openrouter, modelId=openrouter/gpt-5.4-mini
  normalizeModelRef:    { provider: "openrouter", model: "openrouter/gpt-5.4-mini" }
  OLD interpolation:    "openrouter/openrouter/gpt-5.4-mini"
  NEW modelKey():       "openrouter/gpt-5.4-mini"
  allowlist Set:        ["openrouter/gpt-5.4-mini"]
  OLD allowlist match:  false
  NEW allowlist match:  true

--- Bare model id (existing happy path — must not regress) ---
  input:                provider=openai-codex, modelId=gpt-5.4-mini
  normalizeModelRef:    { provider: "openai-codex", model: "gpt-5.4-mini" }
  OLD interpolation:    "openai-codex/gpt-5.4-mini"
  NEW modelKey():       "openai-codex/gpt-5.4-mini"
  OLD == NEW:           true (no regression on the happy path)

--- Provider-qualified id whose prefix differs from provider (must not strip) ---
  input:                provider=openrouter, modelId=openai/gpt-5.4-mini
  normalizeModelRef:    { provider: "openrouter", model: "openai/gpt-5.4-mini" }
  OLD interpolation:    "openrouter/openai/gpt-5.4-mini"
  NEW modelKey():       "openrouter/openai/gpt-5.4-mini"
  OLD == NEW:           true (modelKey only collapses when the modelId already starts with the provider)

 RUN  v4.1.7 /media/vdc/code/ai/openclaw
 Test Files  1 passed (1)
      Tests  19 passed (19)

Observed result after fix: the OpenRouter-shaped selection now resolves to the actionable openrouter/gpt-5.4-mini, hits the user's allowlist, and the denial diagnostic (when the model is genuinely outside the allowlist) reports "openrouter/gpt-5.5" instead of "openrouter/openrouter/gpt-5.5". The bare-id and prefix-mismatch shapes are unchanged.
What was not tested: no known gaps

Regression Test Plan

Coverage level: Unit test (vitest)
Target test file: src/plugins/runtime/runtime-llm.runtime.test.ts
Scenario locked in: provider-qualified OpenRouter selection (provider="openrouter", modelId="openrouter/gpt-5.4-mini") — one case asserts the allowlist hit no longer doubles the prefix; a second case asserts the denial message contains "openrouter/gpt-5.5" and never openrouter/openrouter/.
Why this is the smallest reliable guardrail: the bug is a string-format regression on the policy path; pinning both the hit and the denial message text keeps any future refactor from re-introducing the double prefix without forcing a heavyweight gateway-level fixture.

Root Cause

Root cause: normalizeAllowedModelRef() and the resolved-override formatter both rebuilt a model ref via ${normalized.provider}/${normalized.model} after normalizeModelRef(), and normalizeModelRef() does not collapse a provider segment that the modelId already carries (its job is to normalize provider/model ids, not to dedupe the prefix). The allowlist Set therefore stored openrouter/openrouter/gpt-5.4-mini while the user-facing actionable ref was openrouter/gpt-5.4-mini.
Missing detection / guardrail: there was no test on the runtime LLM policy path that exercised a selection whose modelId already begins with the provider — every existing case used bare ids like openai-codex/gpt-5.4-mini. The new tests close that gap by pinning the OpenRouter shape on both the hit and denial paths.

…stics normalizeAllowedModelRef() and the resolved override ref interpolated ${provider}/${model} after normalizeModelRef(), so a provider-qualified model id like openrouter/gpt-5.4-mini surfaced as openrouter/openrouter/gpt-5.4-mini in the allowlist set and policy denial message, masking the actionable model ref. Route both sites through modelKey() (src/agents/model-ref-shared.ts) so the provider segment is collapsed when the model id already starts with it. Add regression tests covering allowlist hit and denial paths for the OpenRouter shape. Fixes openclaw#84887

clawsweeper · 2026-05-21T12:26:00Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR switches two runtime LLM allowlist/diagnostic model-ref formatting sites to modelKey() and adds regression tests for provider-qualified OpenRouter model ids.

Reproducibility: yes. Current main has a clear source-level reproduction path: provider openrouter plus model id openrouter/gpt-5.4-mini is formatted by raw provider/model interpolation in the runtime LLM policy path, producing the double-prefix shape.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: Focused runtime formatter fix with terminal proof, targeted regression tests, and no blocking findings found in read-only review.

Rank-up moves:

none

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.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal output from a source-helper repro plus focused regression test output, which is adequate for this synchronous host-side formatter bug.

Risk before merge

The PR overlaps with fix(plugins): dedupe provider-prefixed model refs in runtime LLM allowlist policy #84940 and the broader fix: model ref normalization, Vertex transport routing, symlink workspace dirs #84934, so maintainers should choose one landing path to avoid duplicate/conflicting fixes.
I did not rerun the contributed Vitest command in this read-only review; CI or maintainer validation should confirm the supplied output on the PR head.

Maintainer options:

Decide the mitigation before merge
Land this narrow runtime LLM formatter fix after normal CI/maintainer review, then close the linked issue; handle gateway fallback normalization separately or by choosing fix(plugins): dedupe provider-prefixed model refs in runtime LLM allowlist policy #84940 if maintainers want the broader scope.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No automated repair lane is needed because the PR already contains the focused repair and no discrete review finding was identified.

Security
Cleared: The diff reuses an existing model-ref formatter and adds tests; no dependency, CI, secret-handling, auth, or supply-chain surface changes were found.

Review details

Best possible solution:

Land this narrow runtime LLM formatter fix after normal CI/maintainer review, then close the linked issue; handle gateway fallback normalization separately or by choosing #84940 if maintainers want the broader scope.

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

Yes. Current main has a clear source-level reproduction path: provider openrouter plus model id openrouter/gpt-5.4-mini is formatted by raw provider/model interpolation in the runtime LLM policy path, producing the double-prefix shape.

Is this the best way to solve the issue?

Yes. Reusing existing modelKey() at the two runtime LLM policy formatting sites is the narrowest maintainable OpenClaw-side fix for this scoped issue.

Label changes:

add P2: The PR fixes a real plugin runtime LLM diagnostics and allowlist bug with limited blast radius and a focused code path.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a source-helper repro plus focused regression test output, which is adequate for this synchronous host-side formatter bug.

Label justifications:

P2: The PR fixes a real plugin runtime LLM diagnostics and allowlist bug with limited blast radius and a focused code path.
rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🐚 platinum hermit, patch quality is 🐚 platinum hermit, and Focused runtime formatter fix with terminal proof, targeted regression tests, and no blocking findings found in read-only review.
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 source-helper repro plus focused regression test output, which is adequate for this synchronous host-side formatter bug.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a source-helper repro plus focused regression test output, which is adequate for this synchronous host-side formatter bug.

What I checked:

Current main allowlist formatting can double-prefix: Current main normalizes an allowed model ref, then rebuilds it as ${normalized.provider}/${normalized.model}, which can turn provider openrouter plus model openrouter/gpt-5.4-mini into openrouter/openrouter/gpt-5.4-mini. (src/plugins/runtime/runtime-llm.runtime.ts:237, 6ccca4ae9529)
Current main diagnostic formatting uses the same raw concat: The resolved model override path formats resolvedModelRef with the same provider/model interpolation before the allowlist check and error message. (src/plugins/runtime/runtime-llm.runtime.ts:401, 6ccca4ae9529)
Existing helper has the requested behavior: modelKey trims provider/model and returns the model id unchanged when it already starts with the same provider prefix, matching the intended canonical ref. (src/agents/model-ref-shared.ts:17, 6ccca4ae9529)
PR applies modelKey at both runtime LLM policy sites: The PR imports modelKey and replaces both runtime LLM raw interpolation sites with modelKey calls. (src/plugins/runtime/runtime-llm.runtime.ts:236, ee9f3b6a52b4)
PR adds focused regression coverage: The PR adds tests for the allowlist-hit path and the denial-message path for provider-qualified OpenRouter model ids. (src/plugins/runtime/runtime-llm.runtime.test.ts:306, ee9f3b6a52b4)
Linked issue scope was narrowed: The reporter clarified that Lossless provider precedence is tracked upstream and this OpenClaw item is about the host-side double-prefix diagnostic only.

Likely related people:

giodl73-repo: PR 80056 and git blame tie the current runtime LLM policy code, tests, and modelKey helper to merge commit a30ac3f. (role: introduced current runtime/model-ref path; confidence: high; commits: a30ac3f8d7cb; files: src/plugins/runtime/runtime-llm.runtime.ts, src/plugins/runtime/runtime-llm.runtime.test.ts, src/agents/model-ref-shared.ts)
vincentkoc: Recent OpenRouter alias and helper cleanup commits touched nearby provider/model selection behavior that informs this normalization path. (role: adjacent OpenRouter/model-selection contributor; confidence: medium; commits: d13869aab94d, 88c49f9e68fc; files: src/agents/model-selection-normalize.ts, src/agents/model-selection.test.ts, extensions/openrouter/stream.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6ccca4ae9529.

clawsweeper · 2026-05-21T12:32:16Z

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Pearl Diff Drake

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🌱 uncommon.
Trait: sleeps inside passing CI.
Image traits: location review cove; accessory rollback rope; palette moss green and polished brass; mood proud; pose pointing at a small proof artifact; shell woven fiber shell; lighting subtle sparkle highlights; background small green status lights.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Pearl Diff Drake in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

vincentkoc · 2026-05-22T13:36:04Z

Maintainer verification before merge:

Inspected live PR diff with ghx pr diff 84946.
Inspected PR metadata/proof with ghx pr view 84946 --json body,closingIssuesReferences,labels,mergeable,maintainerCanModify,statusCheckRollup.
Compared against the maintainer recut in diagnostics-pr84946 with git diff origin/main...HEAD -- src/plugins/runtime/runtime-llm.runtime.ts src/plugins/runtime/runtime-llm.runtime.test.ts; the live PR is the same scoped runtime LLM formatter fix.
ClawSweeper review marks proof as sufficient and the PR has proof: sufficient.

Known proof gap: I did not run a fresh local pnpm/Vitest command from this maintainer machine. The project instruction for this sweep is Crabbox/Testbox-only validation, and the current runner auth is unavailable (blacksmith testbox is not authenticated; direct AWS Crabbox has no credentials/IMDS role). Given this is a tiny host-side formatter regression with supplied terminal proof and focused tests, I’m landing the narrow PR and leaving broader gateway fallback normalization out of this merge.

nxmxbbd · 2026-05-22T17:25:35Z

@vincentkoc FYI following up on the broader gateway fallback piece noted in your merge comment as out of scope for this narrow runtime LLM merge: I opened #85445 for that separate path.

Facts checked there:

fix #84887: avoid duplicate provider prefix in runtime LLM allowlist diagnostics #84946 changed only the runtime LLM policy files and closed Runtime LLM allowlist diagnostics can double-prefix provider-qualified model refs #84887.
The gateway fallback policy in src/gateway/server-plugins.ts still had raw provider/model key concatenation on the configured allowlist side and requested override side.
The concrete OpenRouter repro is the same double-provider shape: provider: "openrouter" with a provider-prefixed model such as openrouter/gpt-5.5 could produce a deny diagnostic containing openrouter/openrouter/....
fix: model ref normalization, Vertex transport routing, symlink workspace dirs #84934 overlaps but only normalizes the requested side; in my local repro that shape false-denies the matching allow case because the configured allowlist key remains raw-concatenated.
fix(gateway): canonicalize fallback model override keys #85445 updates both comparison sides to use modelKey() and includes both deny-diagnostic and matching-allow regression coverage.

ClawSweeper has now accepted the production-source gateway fallback smoke proof on #85445 (proof: sufficient, status: 👀 ready for maintainer look). No action needed on this merged PR; just linking the follow-up so the unresolved gateway path is tracked separately.

…stics (openclaw#84946) normalizeAllowedModelRef() and the resolved override ref interpolated ${provider}/${model} after normalizeModelRef(), so a provider-qualified model id like openrouter/gpt-5.4-mini surfaced as openrouter/openrouter/gpt-5.4-mini in the allowlist set and policy denial message, masking the actionable model ref. Route both sites through modelKey() (src/agents/model-ref-shared.ts) so the provider segment is collapsed when the model id already starts with it. Add regression tests covering allowlist hit and denial paths for the OpenRouter shape. Fixes openclaw#84887

openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels May 21, 2026

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

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. P2 Normal backlog priority with limited blast radius. labels May 21, 2026

clawsweeper Bot mentioned this pull request May 21, 2026

fix: model ref normalization, Vertex transport routing, symlink workspace dirs #84934

Open

8 tasks

vincentkoc self-assigned this May 22, 2026

vincentkoc merged commit 937a756 into openclaw:main May 22, 2026
162 of 170 checks passed

vincentkoc mentioned this pull request May 22, 2026

fix(plugins): dedupe provider-prefixed model refs in runtime LLM allowlist policy #84940

Closed

19 tasks

github-actions Bot mentioned this pull request May 22, 2026

📡 Upstream Digest — 2026-05-22 14:22 UTC curtismercier/openclaw-mods#916

Open

nxmxbbd mentioned this pull request May 22, 2026

fix(gateway): canonicalize fallback model override keys #85445

Closed

25 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix #84887: avoid duplicate provider prefix in runtime LLM allowlist diagnostics#84946

fix #84887: avoid duplicate provider prefix in runtime LLM allowlist diagnostics#84946
vincentkoc merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-84887

zhangguiping-xydt commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026

Uh oh!

vincentkoc commented May 22, 2026

Uh oh!

Uh oh!

nxmxbbd commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

zhangguiping-xydt commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Regression Test Plan

Root Cause

Uh oh!

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

Uh oh!

Uh oh!

clawsweeper Bot commented May 21, 2026

Hatch command

Uh oh!

vincentkoc commented May 22, 2026

Uh oh!

Uh oh!

nxmxbbd commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhangguiping-xydt commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading