feat: update supercode-cli version to 0.1.77 and enhance BYOK provider management:#217
Conversation
…r management: - Bumped version in package.json to 0.1.77. - Improved handling of BYOK environment variables for multiple providers. - Updated input rendering in chat to include placeholder text when input is empty. - Refactored API key retrieval to streamline session-only BYOK logic across various commands.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughChangesBYOK session key flow
CLI release and prompt updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant cli-config
participant Environment
participant Provider
CLI->>cli-config: saveProviderApiKey(provider, key)
cli-config->>Environment: write provider BYOK env variable
CLI->>cli-config: getByokSessionKey(provider)
cli-config->>Environment: read prod/dev BYOK env variable
cli-config-->>Provider: return session key
Provider-->>CLI: configure selected provider
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/supercode-cli/server/src/cli/ai/chat/chat.ts`:
- Line 1046: Update the placeholder string literals in the visibleLen
expressions near lines 1046 and 1068 to use double-quoted strings, escaping the
embedded double quotes while preserving the existing text and behavior.
In `@apps/supercode-cli/server/src/lib/cli-config.ts`:
- Around line 108-114: Update getByokSessionKey in
apps/supercode-cli/server/src/lib/cli-config.ts to fall back to the standard API
key mapping when configured BYOK override variables are absent. In
apps/supercode-cli/server/src/cli/commands/ai/init.ts, remove the duplicated
BYOK_PROVIDER_VARS mapping, import getByokSessionKey from src/lib/cli-config.ts,
and replace the manual environment check with !getByokSessionKey(sp).
🪄 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: 975cfad2-95da-4959-a238-4172b5ffd4c7
📒 Files selected for processing (7)
apps/supercode-cli/server/package.jsonapps/supercode-cli/server/src/cli/ai/chat/chat.tsapps/supercode-cli/server/src/cli/ai/provider.tsapps/supercode-cli/server/src/cli/commands/ai/init.tsapps/supercode-cli/server/src/cli/commands/slashCommands/connect.tsapps/supercode-cli/server/src/cli/commands/slashCommands/model.tsapps/supercode-cli/server/src/lib/cli-config.ts
| const promptLen = getStdoutPromptLen() | ||
| stdinPromptLen = promptLen | ||
| const totalChars = promptLen + stdinInput.length | ||
| const visibleLen = stdinInput.length || 'Ask anything... "Fix broken tests"'.length |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use double-quoted strings for the new placeholder literals.
Lines 1046 and 1068 introduce single-quoted strings, contrary to the project guideline. Convert both literals to double quotes while escaping the embedded quotes.
Proposed fix
- const visibleLen = stdinInput.length || 'Ask anything... "Fix broken tests"'.length
+ const visibleLen = stdinInput.length || "Ask anything... \"Fix broken tests\"".length
...
- const inputText = stdinInput || chalk.hex(theme.greenDim)('Ask anything... "Fix broken tests"')
+ const inputText = stdinInput || chalk.hex(theme.greenDim)("Ask anything... \"Fix broken tests\"")As per coding guidelines: **/*.{ts,tsx,js,mjs,cjs} must use double quotes for strings.
Also applies to: 1068-1068
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/cli/ai/chat/chat.ts` at line 1046, Update the
placeholder string literals in the visibleLen expressions near lines 1046 and
1068 to use double-quoted strings, escaping the embedded double quotes while
preserving the existing text and behavior.
Source: Coding guidelines
| export function getByokSessionKey(provider: ModelProvider): string | undefined { | ||
| const override = BYOK_ENV_OVERRIDES[provider] | ||
| if (override) { | ||
| return process.env[override.prod] || process.env[override.dev] | ||
| } | ||
| return "CONCENTRATE_BYOK_PROD_KEY" | ||
| return process.env[API_KEY_ENV_MAP[provider]] || undefined | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing fallback to standard API keys for BYOK providers.
Both files fail to account for the standard environment variables (e.g., GOOGLE_GENERATIVE_AI_API_KEY) when a provider has BYOK overrides configured but they are absent from the environment. This results in the user's provider being unexpectedly reset to supercode on startup, as well as erroneous prompts for an API key in /connect and /model, even when a valid standard key is active.
apps/supercode-cli/server/src/lib/cli-config.ts#L108-L114: UpdategetByokSessionKeyto explicitly return the standard fallback key when the BYOK overrides are unset.apps/supercode-cli/server/src/cli/commands/ai/init.ts#L92-L104: Remove the duplicatedBYOK_PROVIDER_VARSmapping and replace the manual environment check with!getByokSessionKey(sp)to properly respect fallback keys. (EnsuregetByokSessionKeyis imported fromsrc/lib/cli-config.ts).
🛠️ Proposed fixes
apps/supercode-cli/server/src/lib/cli-config.ts
export function getByokSessionKey(provider: ModelProvider): string | undefined {
const override = BYOK_ENV_OVERRIDES[provider]
if (override) {
- return process.env[override.prod] || process.env[override.dev]
+ const val = process.env[override.prod] || process.env[override.dev]
+ if (val) return val
}
return process.env[API_KEY_ENV_MAP[provider]] || undefined
}apps/supercode-cli/server/src/cli/commands/ai/init.ts
- const BYOK_PROVIDER_VARS: Record<string, string[]> = {
- concentrateai: ["CONCENTRATE_BYOK_PROD_KEY", "CONCENTRATE_BYOK_DEV_KEY"],
- mergedev: ["MERGE_DEV_BYOK_PROD_KEY", "MERGE_DEV_BYOK_DEV_KEY"],
- google: ["GOOGLE_BYOK_PROD_KEY", "GOOGLE_BYOK_DEV_KEY"],
- openrouter: ["OPENROUTER_BYOK_PROD_KEY", "OPENROUTER_BYOK_DEV_KEY"],
- nvidia: ["NVIDIA_BYOK_PROD_KEY", "NVIDIA_BYOK_DEV_KEY"],
- }
const sp = stored.provider
- const byokVars = sp && BYOK_PROVIDER_VARS[sp]
- if (byokVars && !byokVars.some((v) => process.env[v])) {
+ if (sp && sp !== "supercode" && !getByokSessionKey(sp)) {
stored.provider = "supercode"
await saveCliConfig({ provider: "supercode", model: stored.model })
}📍 Affects 2 files
apps/supercode-cli/server/src/lib/cli-config.ts#L108-L114(this comment)apps/supercode-cli/server/src/cli/commands/ai/init.ts#L92-L104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/lib/cli-config.ts` around lines 108 - 114,
Update getByokSessionKey in apps/supercode-cli/server/src/lib/cli-config.ts to
fall back to the standard API key mapping when configured BYOK override
variables are absent. In apps/supercode-cli/server/src/cli/commands/ai/init.ts,
remove the duplicated BYOK_PROVIDER_VARS mapping, import getByokSessionKey from
src/lib/cli-config.ts, and replace the manual environment check with
!getByokSessionKey(sp).
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes