-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat: use on prem proxy in CLI #7466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 3 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| apiKeyLocation: (selectedModel as any).apiKeyLocation, | ||
| orgScopeId: organizationId, | ||
| proxyUrl: undefined, | ||
| proxyUrl: (selectedModel as any).onPremProxyUrl ?? undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignores the useOnPremProxy flag; should only set proxyUrl when on-prem proxy usage is enabled.
(Based on your team's feedback about correctly honoring on-prem proxy settings in the CLI.)
Prompt for AI agents
Address the following comment on extensions/cli/src/services/ModelService.ts at line 105:
<comment>Ignores the useOnPremProxy flag; should only set proxyUrl when on-prem proxy usage is enabled.
(Based on your team's feedback about correctly honoring on-prem proxy settings in the CLI.)</comment>
<file context>
@@ -102,7 +102,7 @@ export class ModelService
apiKeyLocation: (selectedModel as any).apiKeyLocation,
orgScopeId: organizationId,
- proxyUrl: undefined,
+ proxyUrl: (selectedModel as any).onPremProxyUrl ?? undefined,
},
}
</file context>
| apiKeyLocation: (selectedModel as any).apiKeyLocation, | ||
| orgScopeId: organizationId, | ||
| proxyUrl: undefined, | ||
| proxyUrl: (selectedModel as any).onPremProxyUrl ?? undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty string onPremProxyUrl will be treated as a base URL and can crash URL construction; coalesce falsy to undefined to avoid invalid base URLs.
(Based on your team's feedback about correctly using on-prem proxy settings in the CLI without introducing runtime errors.)
Prompt for AI agents
Address the following comment on extensions/cli/src/services/ModelService.ts at line 105:
<comment>Empty string onPremProxyUrl will be treated as a base URL and can crash URL construction; coalesce falsy to undefined to avoid invalid base URLs.
(Based on your team's feedback about correctly using on-prem proxy settings in the CLI without introducing runtime errors.)</comment>
<file context>
@@ -102,7 +102,7 @@ export class ModelService
apiKeyLocation: (selectedModel as any).apiKeyLocation,
orgScopeId: organizationId,
- proxyUrl: undefined,
+ proxyUrl: (selectedModel as any).onPremProxyUrl ?? undefined,
},
}
</file context>
| proxyUrl: (selectedModel as any).onPremProxyUrl ?? undefined, | |
| proxyUrl: (selectedModel as any).onPremProxyUrl || undefined, |
|
|
||
| // Unroll locally if not logged in | ||
| if (!(apiClient as any).configuration.apiKey) { | ||
| if (!(apiClient as any).configuration.accessToken) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly accessing configuration.accessToken can throw if configuration is undefined (e.g., with a mock client). Use optional chaining to avoid a runtime crash.
Prompt for AI agents
Address the following comment on extensions/cli/src/configLoader.ts at line 356:
<comment>Directly accessing configuration.accessToken can throw if configuration is undefined (e.g., with a mock client). Use optional chaining to avoid a runtime crash.</comment>
<file context>
@@ -353,7 +353,7 @@ async function loadAssistantSlug(
// Unroll locally if not logged in
- if (!(apiClient as any).configuration.apiKey) {
+ if (!(apiClient as any).configuration.accessToken) {
return await unrollAssistantWithConfig(
{
</file context>
| if (!(apiClient as any).configuration.accessToken) { | |
| if (!((apiClient as any).configuration?.accessToken)) { |
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Correctly use on prem proxy settings in CLI
Summary by cubic
Routes CLI model requests through the configured on‑prem proxy. Also fixes login detection and trims noisy terminal output in tool results.