Skip to content

Commit 9787dee

Browse files
hasysclaude
andauthored
AAP-69248: WCA OAuth ignores custom Lightspeed URL configuration (#2675)
refactor: simplify getBaseUri by removing WCA fallback logic Address PR review feedback by fully adopting fail-fast strategy: 1. Remove WCA-specific fallback to WCA_API_ENDPOINT_DEFAULT - Trust llmProviderSettings.get() to return defaults via provider factory - Add explicit error when apiEndpoint is not configured (defensive) 2. Remove fallback to legacy settings schema - Only use llmProviderSettings after migration - Check llmProviderSettings initialization first (fail early) 3. Update all tests to match new behavior - Tests now expect provider factory defaults - Add new tests for empty endpoint error scenarios - Remove dependency on old settings structure in mocks This simplification: - Removes 15+ lines of redundant code - Eliminates technical debt from legacy settings - Provides clearer error messages for misconfiguration - Follows fail-fast principle for better debugging All 1544 tests passing, lint checks pass. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 2243796 commit 9787dee

File tree

6 files changed

+455
-24
lines changed

6 files changed

+455
-24
lines changed

.config/dictionary.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Lightspeed
1919
Maciążek
2020
Npmjs
2121
OVSX
22+
Ollama
2223
PKGMGR
2324
PYTHONBREAKPOINT
2425
PYTHONPYCACHEPREFIX

src/features/lightspeed/api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class LightSpeedAPI {
104104
Object.assign(headers, { Authorization: `Bearer ${authToken}` });
105105
}
106106

107-
const baseUrl = `${getBaseUri(this.settingsManager)}/api`;
107+
const baseUrl = `${await getBaseUri(this.settingsManager)}/api`;
108108

109109
return await fetch(`${baseUrl}/${endpoint}`, {
110110
method: "POST",

src/features/lightspeed/lightSpeedOAuthProvider.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ export class LightSpeedAuthenticationProvider
290290
["redirect_uri", this._externalRedirectUri],
291291
]);
292292

293-
const base_uri = getBaseUri(this.settingsManager);
293+
const base_uri = await getBaseUri(this.settingsManager);
294294
if (!base_uri) {
295295
throw new Error(
296296
"Please enter the Ansible Lightspeed URL under the Ansible Lightspeed settings!",
@@ -383,7 +383,7 @@ export class LightSpeedAuthenticationProvider
383383
const fetch = getFetch();
384384

385385
const response = await fetch(
386-
`${getBaseUri(this.settingsManager)}/o/token/`,
386+
`${await getBaseUri(this.settingsManager)}/o/token/`,
387387
{
388388
method: "POST",
389389
signal: AbortSignal.timeout(ANSIBLE_LIGHTSPEED_API_TIMEOUT),
@@ -424,6 +424,8 @@ export class LightSpeedAuthenticationProvider
424424
name: err.name,
425425
message: err.message,
426426
stack: err.stack,
427+
cause: err.cause, // undici errors often have a cause
428+
code: (err as NodeJS.ErrnoException).code, // network error codes
427429
},
428430
);
429431
throw err;
@@ -453,7 +455,7 @@ export class LightSpeedAuthenticationProvider
453455
const fetch = getFetch();
454456

455457
const response = await fetch(
456-
`${getBaseUri(this.settingsManager)}/o/token/`,
458+
`${await getBaseUri(this.settingsManager)}/o/token/`,
457459
{
458460
method: "POST",
459461
signal: AbortSignal.timeout(ANSIBLE_LIGHTSPEED_API_TIMEOUT),
@@ -500,6 +502,8 @@ export class LightSpeedAuthenticationProvider
500502
name: err.name,
501503
message: err.message,
502504
stack: err.stack,
505+
cause: err.cause, // undici errors often have a cause
506+
code: (err as NodeJS.ErrnoException).code, // network error codes
503507
},
504508
);
505509
throw err;

src/features/lightspeed/lightspeedUser.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,18 @@ export class LightspeedUser {
8484
? ExtensionHost.Local
8585
: ExtensionHost.Remote
8686
: ExtensionHost.WebWorker;
87-
this.logAuthProviderDebugHints();
8887
this._getUserInfoCache = { time: 0, token: "", locked: false };
88+
// Log debug hints asynchronously (don't block constructor)
89+
this.logAuthProviderDebugHints().catch((error) => {
90+
this._logger.error(
91+
`[ansible-lightspeed-user] Failed to log auth provider debug hints: ${error}`,
92+
);
93+
});
8994
}
9095

91-
private logAuthProviderDebugHints() {
96+
private async logAuthProviderDebugHints(): Promise<void> {
9297
const provider = this._settingsManager.settings.lightSpeedService.provider;
93-
const lightspeedUri = getBaseUri(this._settingsManager);
98+
const lightspeedUri = await getBaseUri(this._settingsManager);
9499
this._logger.info(
95100
`[ansible-lightspeed-user] Initializing LightspeedUser with provider: ${provider}, URI: ${lightspeedUri || "(none)"}, extension host: ${this._extensionHost}`,
96101
);
@@ -143,7 +148,7 @@ export class LightspeedUser {
143148

144149
try {
145150
const response = await fetch(
146-
`${getBaseUri(this._settingsManager)}${LIGHTSPEED_ME_AUTH_URL}`,
151+
`${await getBaseUri(this._settingsManager)}${LIGHTSPEED_ME_AUTH_URL}`,
147152
{
148153
method: "GET",
149154
signal: AbortSignal.timeout(ANSIBLE_LIGHTSPEED_API_TIMEOUT),
@@ -197,7 +202,7 @@ export class LightspeedUser {
197202
const fetch = getFetch();
198203

199204
const response = await fetch(
200-
`${getBaseUri(this._settingsManager)}${LIGHTSPEED_MARKDOWN_ME_AUTH_URL}`,
205+
`${await getBaseUri(this._settingsManager)}${LIGHTSPEED_MARKDOWN_ME_AUTH_URL}`,
201206
{
202207
method: "GET",
203208
signal: AbortSignal.timeout(ANSIBLE_LIGHTSPEED_API_TIMEOUT),
@@ -258,7 +263,7 @@ export class LightspeedUser {
258263
return [AuthProviderType.rhsso, AuthProviderType.lightspeed];
259264
}
260265
}
261-
const lightspeedUri = getBaseUri(this._settingsManager);
266+
const lightspeedUri = await getBaseUri(this._settingsManager);
262267
// Prefer RHSSO when we know Lightspeed auth will be broken.
263268
// Prefer Lightspeed auth all other times.
264269
if (

src/features/lightspeed/utils/webUtils.ts

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@ import {
77
LightspeedUserDetails,
88
LightspeedSessionInfo,
99
} from "@src/interfaces/lightspeed";
10-
import {
11-
LIGHTSPEED_USER_TYPE,
12-
WCA_API_ENDPOINT_DEFAULT,
13-
} from "@src/definitions/lightspeed";
10+
import { LIGHTSPEED_USER_TYPE } from "@src/definitions/lightspeed";
1411
import { lightSpeedManager } from "@src/extension";
1512

1613
// Also defined in package.json in "".contributes.authentication"
@@ -50,22 +47,37 @@ export function calculateTokenExpiryTime(expiresIn: number) {
5047
}
5148

5249
/* Get base uri in a correct formatted way */
53-
export function getBaseUri(
50+
export async function getBaseUri(
5451
settingsManager: SettingsManager,
5552
providerOverride?: string,
56-
): string {
57-
// Check the provider from globalState via lightSpeedManager if available
53+
): Promise<string> {
54+
if (!lightSpeedManager?.llmProviderSettings) {
55+
throw new Error(
56+
"LLM provider settings not initialized. Extension may not have loaded correctly.",
57+
);
58+
}
59+
60+
// Get provider from llmProviderSettings (after migration from legacy settings)
5861
const provider =
59-
providerOverride ||
60-
lightSpeedManager?.llmProviderSettings?.getProvider() ||
61-
settingsManager.settings.lightSpeedService.provider;
62+
providerOverride || lightSpeedManager.llmProviderSettings.getProvider();
63+
64+
if (!provider) {
65+
throw new Error("Provider is not configured");
66+
}
67+
68+
// Get endpoint from provider-specific settings
69+
// Migration from legacy settings happens during extension activation via migrateFromSettingsJson()
70+
// Provider factory ensures default endpoints are returned when not configured
71+
const baseUri = (
72+
await lightSpeedManager.llmProviderSettings.get(provider, "apiEndpoint")
73+
).trim();
6274

63-
// For WCA, always use the default WCA endpoint
64-
if (provider === "wca") {
65-
return WCA_API_ENDPOINT_DEFAULT;
75+
if (!baseUri) {
76+
throw new Error(
77+
`API endpoint not configured for provider "${provider}". Check provider settings.`,
78+
);
6679
}
6780

68-
const baseUri = settingsManager.settings.lightSpeedService.apiEndpoint.trim();
6981
return baseUri.endsWith("/") ? baseUri.slice(0, -1) : baseUri;
7082
}
7183

0 commit comments

Comments
 (0)