fix: simplify default models revert flow (KHA-272)#39
fix: simplify default models revert flow (KHA-272)#39KHAEntertainment wants to merge 1 commit intomainfrom
Conversation
…nthropic defaults On proxy stop, remove all ThroneKeeper env vars from .claude/settings.json and let Claude Code use its built-in defaults. This eliminates the fetchAnthropicDefaults() API call during revert, which failed for OAuth (Pro/Max) users and returned outdated 4.5-era model names. Also updates hardcoded fallbacks to current 4.6 -latest aliases. Closes: KHA-272 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Hi @KHAEntertainment, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
|
🤖 I'm sorry @KHAEntertainment, but I was unable to process your request. Please see the logs for more details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extensions/thronekeeper/src/extension.ts (1)
499-503:⚠️ Potential issue | 🟡 MinorAdd
claudeThrone.storeKimiKeyto the extension's command contributions.
claudeThrone.storeKimiKeyis registered in code but missing frompackage.json'scontributes.commandsarray. Without this declaration, the command won't appear reliably in the Command Palette or menus. (claudeThrone.storeMinimaxKeyis already properly declared.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/thronekeeper/src/extension.ts` around lines 499 - 503, The extension registers the command storeKimiKey via the function/variable storeKimiKey (command id 'claudeThrone.storeKimiKey') but that id is missing from package.json contributes.commands; add an entry for 'claudeThrone.storeKimiKey' to the contributes.commands array (mirror the existing 'claudeThrone.storeMinimaxKey' entry including a suitable title and category) so the command is exposed in the Command Palette and menus.
🧹 Nitpick comments (1)
extensions/thronekeeper/src/extension.ts (1)
1144-1153: Consolidate duplicate subscription lists to prevent drift.This block duplicates disposables already pushed earlier (Line 527 onward). It works, but maintaining two lists is like keeping two source-of-truth ledgers—eventually they diverge. Prefer a single
context.subscriptions.push(...)block.As per coding guidelines "
**/*.ts: Event listeners are not duplicated (check cleanup)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/thronekeeper/src/extension.ts` around lines 1144 - 1153, The second context.subscriptions.push(...) duplicates disposables already pushed earlier (openPanel, storeOpenRouterKey, storeOpenAIKey, storeTogetherKey, storeDeepseekKey, storeGlmKey, storeKimiKey, storeMinimaxKey, storeCustomKey), so remove this duplicate block and consolidate into a single subscription registration; either extend the existing push at the original location to include any missing symbols or centralize all disposables into one array and call context.subscriptions.push(...thatArray) so there is only one authoritative subscription list for cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/thronekeeper/src/extension.ts`:
- Around line 126-128: The unconditional console.log in fetchAnthropicDefaults
printing models.map((m: any) => m.id) is noisy; wrap this raw model-ID dump
behind the debug flag and use the Fastify logger instead of console. Replace the
direct console.log with a conditional that checks the extension debug setting
(e.g. process.env.DEBUG or the extension's debug config) and call
fastify.log.debug(...) (or the injected logger) to emit the message only when
debug is enabled, referencing the same models variable and the existing
fetchAnthropicDefaults function.
---
Outside diff comments:
In `@extensions/thronekeeper/src/extension.ts`:
- Around line 499-503: The extension registers the command storeKimiKey via the
function/variable storeKimiKey (command id 'claudeThrone.storeKimiKey') but that
id is missing from package.json contributes.commands; add an entry for
'claudeThrone.storeKimiKey' to the contributes.commands array (mirror the
existing 'claudeThrone.storeMinimaxKey' entry including a suitable title and
category) so the command is exposed in the Command Palette and menus.
---
Nitpick comments:
In `@extensions/thronekeeper/src/extension.ts`:
- Around line 1144-1153: The second context.subscriptions.push(...) duplicates
disposables already pushed earlier (openPanel, storeOpenRouterKey,
storeOpenAIKey, storeTogetherKey, storeDeepseekKey, storeGlmKey, storeKimiKey,
storeMinimaxKey, storeCustomKey), so remove this duplicate block and consolidate
into a single subscription registration; either extend the existing push at the
original location to include any missing symbols or centralize all disposables
into one array and call context.subscriptions.push(...thatArray) so there is
only one authoritative subscription list for cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18b3748a-d074-4bc5-adee-fffbc4ebcfb7
📒 Files selected for processing (1)
extensions/thronekeeper/src/extension.ts
| // DEBUG: Log all raw model IDs before filtering | ||
| console.log(`[fetchAnthropicDefaults] All ${models.length} models from API:`, models.map((m: any) => m.id)); | ||
|
|
There was a problem hiding this comment.
Gate raw model-ID dump behind debug mode.
Line 127 logs the full model-id list unconditionally; that’s noisy in normal runs and can flood extension logs like leaving a faucet open. Please guard it behind the debug setting.
💡 Suggested patch
- // DEBUG: Log all raw model IDs before filtering
- console.log(`[fetchAnthropicDefaults] All ${models.length} models from API:`, models.map((m: any) => m.id));
+ const debug = vscode.workspace.getConfiguration('claudeThrone').get<boolean>('proxy.debug', false)
+ if (debug) {
+ console.log(
+ `[fetchAnthropicDefaults] All ${models.length} models from API:`,
+ models.map((m: any) => m.id)
+ )
+ }As per coding guidelines "Logging: use Fastify's logger; gate verbose output behind DEBUG."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/thronekeeper/src/extension.ts` around lines 126 - 128, The
unconditional console.log in fetchAnthropicDefaults printing models.map((m: any)
=> m.id) is noisy; wrap this raw model-ID dump behind the debug flag and use the
Fastify logger instead of console. Replace the direct console.log with a
conditional that checks the extension debug setting (e.g. process.env.DEBUG or
the extension's debug config) and call fastify.log.debug(...) (or the injected
logger) to emit the message only when debug is enabled, referencing the same
models variable and the existing fetchAnthropicDefaults function.
1 participant
Summary
revertApplyto null-out all ThroneKeeper overrides instead of calling the Anthropic API during proxy stop-latestaliasesContext
fetchAnthropicDefaults()was called on every proxy stop to get current model names for restoring.claude/settings.json. This failed for OAuth (Pro/Max) users and returned outdated model names (4.5 era when 4.6 is current).The CLI flow already uses the simpler null-out approach successfully.
Changes
revertApplysimplified — RemovedfetchAnthropicDefaults()API call, cache-age check, and cache update. Single atomic null-out of all ThroneKeeper env vars.claude-opus-4-6-latest,claude-sonnet-4-6-latest,claude-haiku-4-5-latestTest plan
npm run compile— TypeScript compiles cleansettings.jsonhas models setsettings.jsonCloses: KHA-272
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor