fix(commands): handle "/model default" as reset to configured default#13993
Closed
0xRaini wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix(commands): handle "/model default" as reset to configured default#139930xRaini wants to merge 1 commit intoopenclaw:mainfrom
0xRaini wants to merge 1 commit intoopenclaw:mainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+339
to
+342
| // "/model default" resets to the configured default model | ||
| if (raw.toLowerCase() === "default") { | ||
| return { | ||
| modelSelection: { |
Contributor
There was a problem hiding this comment.
Missing regression test
This adds new user-facing parsing behavior for /model default (case-insensitive) that clears the model override via isDefault: true, but there’s no corresponding test in src/auto-reply/reply/directive-handling.model.test.ts. Please add coverage for /model default (and ideally /model DEFAULT) to ensure future changes don’t reintroduce the allowlist error path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/directive-handling.model.ts
Line: 339:342
Comment:
**Missing regression test**
This adds new user-facing parsing behavior for `/model default` (case-insensitive) that clears the model override via `isDefault: true`, but there’s no corresponding test in `src/auto-reply/reply/directive-handling.model.test.ts`. Please add coverage for `/model default` (and ideally `/model DEFAULT`) to ensure future changes don’t reintroduce the allowlist error path.
How can I resolve this? If you propose a fix, please make it concise.
TWFBusiness
added a commit
to TWFBusiness/openclaw-tw
that referenced
this pull request
Feb 11, 2026
…pstream fixes - Remove Discord, Telegram, Slack, Signal, iMessage, BlueBubbles extensions - Remove @buape/carbon, grammy, and other gateway-specific dependencies - Add session-based login system (cookie auth, rate limiting) - Add type stubs for optional deps (baileys, ciao, qrcode-terminal) - Apply upstream PRs: XSS fix (openclaw#13952), orphan tool_results (openclaw#13974), Anthropic tool ID sanitization (openclaw#13976), BM25 score fix (openclaw#14005), redaction fix (openclaw#14006), subagent timeout (openclaw#13996), context window guard (openclaw#13986), model default reset (openclaw#13993), cache token accounting (openclaw#13853) - Fix all TypeScript compilation errors from channel removal Co-Authored-By: Claude Opus 4.6 <[email protected]>
When users type "/model default" to reset their model override, the directive handler passes "default" to resolveModelRefFromString which resolves it as "anthropic/default" — a non-existent model. This fails the allowlist check with an unhelpful error. Add an early return for "default" that resolves to the configured default provider and model, matching the behavior of session_status model=default. Fixes #13982
This comment was marked as spam.
This comment was marked as spam.
Member
|
Banning user due to spam/slop + copying other users PRs and pinging users |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
/model defaultis interpreted as a model name, resolving toanthropic/defaultwhich fails the allowlist check:Users expect
/model defaultto reset their model override back to the configured default.Fixes #13982
Solution
Add an early return in
resolveModelDirectiveFromInline()that handles"default"(case-insensitive) by returning the configured default provider and model withisDefault: true, which clears the model override.This matches the existing behavior of
session_statustool withmodel=default.AI Disclosure
This PR was authored with AI assistance.
Greptile Overview
Greptile Summary
This PR adjusts chat
/modeldirective parsing so that/model defaultis treated as “reset to configured default” rather than a literal model name (which previously resolved toanthropic/defaultand failed allowlist validation). The change is implemented as an early return inresolveModelSelectionFromDirective, returning the configured default provider/model withisDefault: trueso session overrides are cleared (matching the existingsession_statustool behavior formodel=default).Main gap: the new behavior isn’t covered by the existing directive-handling tests, so it’s easy for a future refactor to regress back to the allowlist error path.
Confidence Score: 4/5
session_statussemantics, and uses the existingisDefaultmechanism to clear overrides. The main risk is behavioral regression without tests for/model default(including case-insensitivity).