Skip to content

Commit fc31260

Browse files
authored
fix: OAuth model cleanup, skill nav, UI polish (#551)
* fix: OAuth disconnect clears models, skill detail back nav, UI polish - OAuth disconnect now deletes provider model list and syncs config, so models don't linger in the selector after revoking OAuth - OAuth-expired providers' models are excluded from listModels() - Skill detail "Back to Skills" uses navigate(-1) to return to the previous page instead of always resetting to the initial tab - Skills page persists active tab (Yours/ClawHub) in URL search params so navigating back preserves the selected tab - GitHub import tab input disabled (coming soon) - Added space between "GitHub Issues" link and "反馈" in disclaimer * fix: add space between GitHub Issues link and disclaimer suffix * fix: guard OAuth disconnect to only delete provider when actually connected
1 parent 1c368ed commit fc31260

7 files changed

Lines changed: 180 additions & 28 deletions

File tree

apps/controller/src/app/container.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export async function createContainer(): Promise<ControllerContainer> {
124124
openclawSyncService,
125125
openclawProcess,
126126
);
127+
modelProviderService.setAuthService(openclawAuthService);
127128
const runtimeModelStateService = new RuntimeModelStateService(env);
128129

129130
// Wire cloud state change callback to sync refreshed cloud inventory without

apps/controller/src/routes/provider-oauth-routes.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,18 @@ export function registerProviderOAuthRoutes(
135135
}),
136136
async (c) => {
137137
const { providerId } = c.req.valid("param");
138+
const wasConnected = (
139+
await container.openclawAuthService.getProviderOAuthStatus(providerId)
140+
).connected;
138141
const ok =
139142
await container.openclawAuthService.disconnectOAuth(providerId);
143+
if (ok && wasConnected) {
144+
// Remove the provider's stored model list so models don't linger
145+
// in the model selector after OAuth is revoked.
146+
await container.modelProviderService.deleteProvider(providerId);
147+
await container.modelProviderService.ensureValidDefaultModel();
148+
await container.openclawSyncService.syncAll();
149+
}
140150
return c.json({ ok }, 200);
141151
},
142152
);

apps/controller/src/services/model-provider-service.ts

Lines changed: 76 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { isSupportedByokProviderId } from "../lib/byok-providers.js";
1414
import { logger } from "../lib/logger.js";
1515
import type { OpenClawProcessManager } from "../runtime/openclaw-process.js";
1616
import type { NexuConfigStore } from "../store/nexu-config-store.js";
17+
import type { OpenClawAuthService } from "./openclaw-auth-service.js";
1718
import type { OpenClawSyncService } from "./openclaw-sync-service.js";
1819

1920
export interface ModelAutoSelectResult {
@@ -255,7 +256,12 @@ function getOpenClawCommandSpec(env: ControllerEnv): {
255256
};
256257
}
257258

259+
// Providers that support OAuth login (no API key needed).
260+
const OAUTH_PROVIDER_IDS = new Set(["openai"]);
261+
258262
export class ModelProviderService {
263+
private openclawAuthService: OpenClawAuthService | null = null;
264+
259265
private miniMaxOauthAbortController: AbortController | null = null;
260266

261267
private miniMaxOauthBrowserUrl: string | null = null;
@@ -287,35 +293,89 @@ export class ModelProviderService {
287293
private readonly openclawProcess: OpenClawProcessManager,
288294
) {}
289295

296+
/**
297+
* Inject the auth service after construction to avoid circular deps.
298+
*/
299+
setAuthService(authService: OpenClawAuthService): void {
300+
this.openclawAuthService = authService;
301+
}
302+
290303
async listModels() {
291304
await this.refreshMiniMaxOauthModelsIfNeeded();
292305

293306
const config = await this.configStore.getConfig();
294307
const desktopCloud = await this.configStore.getDesktopCloudStatus();
295-
const cloudModels: Model[] = (desktopCloud.models ?? []).map((model) => ({
296-
id: model.id,
297-
name: model.name || model.id,
298-
provider: "nexu",
299-
description: "Cloud model via Nexu Link",
300-
}));
301-
302308
const providers = config.providers.filter(
303309
(provider) =>
304310
provider.enabled && isSupportedByokProviderId(provider.providerId),
305311
);
306-
const byokModels: Model[] = providers.flatMap((provider) =>
307-
provider.models.map((modelId) => ({
308-
id: `${provider.providerId}/${modelId}`,
309-
name: modelId,
310-
provider: provider.providerId,
311-
})),
312+
const { cloudModels, byokModels } = await this.getAvailableModels(
313+
providers,
314+
desktopCloud,
312315
);
313316

314317
return {
315318
models: [...cloudModels, ...byokModels],
316319
};
317320
}
318321

322+
private async getAvailableModels(
323+
providers: ReadonlyArray<{
324+
providerId: string;
325+
apiKey: string | null;
326+
models: string[];
327+
}>,
328+
desktopCloud: {
329+
models?: Array<{ id: string; name?: string | null }> | null;
330+
},
331+
): Promise<{ cloudModels: Model[]; byokModels: Model[] }> {
332+
const cloudModels: Model[] = (desktopCloud.models ?? []).map((model) => ({
333+
id: model.id,
334+
name: model.name || model.id,
335+
provider: "nexu",
336+
description: "Cloud model via Nexu Link",
337+
}));
338+
339+
// Exclude OAuth-only providers whose token has expired
340+
const expiredOAuthProviderIds =
341+
await this.getExpiredOAuthProviderIds(providers);
342+
343+
const byokModels: Model[] = providers
344+
.filter((provider) => !expiredOAuthProviderIds.has(provider.providerId))
345+
.flatMap((provider) =>
346+
provider.models.map((modelId) => ({
347+
id: `${provider.providerId}/${modelId}`,
348+
name: modelId,
349+
provider: provider.providerId,
350+
})),
351+
);
352+
353+
return { cloudModels, byokModels };
354+
}
355+
356+
/**
357+
* Returns provider IDs that use OAuth (no API key) and whose token is expired.
358+
*/
359+
private async getExpiredOAuthProviderIds(
360+
providers: ReadonlyArray<{ providerId: string; apiKey: string | null }>,
361+
): Promise<Set<string>> {
362+
if (!this.openclawAuthService) return new Set();
363+
364+
const expired = new Set<string>();
365+
for (const provider of providers) {
366+
if (provider.apiKey || !OAUTH_PROVIDER_IDS.has(provider.providerId)) {
367+
continue;
368+
}
369+
const status = await this.openclawAuthService.getProviderOAuthStatus(
370+
provider.providerId,
371+
);
372+
if (!status.connected) {
373+
expired.add(provider.providerId);
374+
}
375+
}
376+
return expired;
377+
}
378+
319379
async listProviders() {
320380
await this.refreshMiniMaxOauthModelsIfNeeded();
321381

@@ -576,18 +636,9 @@ export class ModelProviderService {
576636
return "unknown";
577637
}
578638

579-
const cloudModels: Model[] = (desktopCloud.models ?? []).map((model) => ({
580-
id: model.id,
581-
name: model.name || model.id,
582-
provider: "nexu",
583-
description: "Cloud model via Nexu Link",
584-
}));
585-
const byokModels: Model[] = providers.flatMap((provider) =>
586-
provider.models.map((modelId) => ({
587-
id: `${provider.providerId}/${modelId}`,
588-
name: modelId,
589-
provider: provider.providerId,
590-
})),
639+
const { cloudModels, byokModels } = await this.getAvailableModels(
640+
providers,
641+
desktopCloud,
591642
);
592643
const knownModels = [...cloudModels, ...byokModels];
593644

apps/controller/tests/provider-oauth-routes.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ function createTestContainer(rootDir: string): ControllerContainer {
9191
{} as ControllerContainer["runtimeModelStateService"],
9292
modelProviderService: {
9393
upsertProvider,
94+
deleteProvider: vi.fn(async () => true),
95+
ensureValidDefaultModel: vi.fn(async () => null),
9496
} as unknown as ControllerContainer["modelProviderService"],
9597
integrationService: {} as ControllerContainer["integrationService"],
9698
localUserService: {} as ControllerContainer["localUserService"],
@@ -173,4 +175,84 @@ describe("provider OAuth routes", () => {
173175
models: ["gpt-5.4"],
174176
});
175177
});
178+
179+
it("disconnect removes provider models and syncs config", async () => {
180+
const container = createTestContainer("/tmp/nexu-provider-oauth-routes");
181+
(
182+
container.openclawAuthService.getProviderOAuthStatus as ReturnType<
183+
typeof vi.fn
184+
>
185+
).mockResolvedValueOnce({ connected: true });
186+
(
187+
container.openclawAuthService.disconnectOAuth as ReturnType<typeof vi.fn>
188+
).mockResolvedValueOnce(true);
189+
const app = createApp(container);
190+
191+
const response = await app.request(
192+
"/api/v1/providers/openai/oauth/disconnect",
193+
{ method: "POST" },
194+
);
195+
196+
expect(response.status).toBe(200);
197+
await expect(response.json()).resolves.toEqual({ ok: true });
198+
expect(container.modelProviderService.deleteProvider).toHaveBeenCalledWith(
199+
"openai",
200+
);
201+
expect(
202+
container.modelProviderService.ensureValidDefaultModel,
203+
).toHaveBeenCalledTimes(1);
204+
expect(container.openclawSyncService.syncAll).toHaveBeenCalledTimes(1);
205+
});
206+
207+
it("disconnect does not delete provider when OAuth disconnect fails", async () => {
208+
const container = createTestContainer("/tmp/nexu-provider-oauth-routes");
209+
(
210+
container.openclawAuthService.getProviderOAuthStatus as ReturnType<
211+
typeof vi.fn
212+
>
213+
).mockResolvedValueOnce({ connected: true });
214+
(
215+
container.openclawAuthService.disconnectOAuth as ReturnType<typeof vi.fn>
216+
).mockResolvedValueOnce(false);
217+
const app = createApp(container);
218+
219+
const response = await app.request(
220+
"/api/v1/providers/openai/oauth/disconnect",
221+
{ method: "POST" },
222+
);
223+
224+
expect(response.status).toBe(200);
225+
await expect(response.json()).resolves.toEqual({ ok: false });
226+
expect(
227+
container.modelProviderService.deleteProvider,
228+
).not.toHaveBeenCalled();
229+
});
230+
231+
it("disconnect does not delete provider when no OAuth profile was connected", async () => {
232+
const container = createTestContainer("/tmp/nexu-provider-oauth-routes");
233+
(
234+
container.openclawAuthService.getProviderOAuthStatus as ReturnType<
235+
typeof vi.fn
236+
>
237+
).mockResolvedValueOnce({ connected: false });
238+
(
239+
container.openclawAuthService.disconnectOAuth as ReturnType<typeof vi.fn>
240+
).mockResolvedValueOnce(true);
241+
const app = createApp(container);
242+
243+
const response = await app.request(
244+
"/api/v1/providers/openai/oauth/disconnect",
245+
{ method: "POST" },
246+
);
247+
248+
expect(response.status).toBe(200);
249+
await expect(response.json()).resolves.toEqual({ ok: true });
250+
expect(
251+
container.modelProviderService.deleteProvider,
252+
).not.toHaveBeenCalled();
253+
expect(
254+
container.modelProviderService.ensureValidDefaultModel,
255+
).not.toHaveBeenCalled();
256+
expect(container.openclawSyncService.syncAll).not.toHaveBeenCalled();
257+
});
176258
});

apps/web/src/components/skills/import-skill-modal.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,8 @@ export default function ImportSkillModal({
237237
type="url"
238238
placeholder="https://github.com/user/repo"
239239
className="mt-1.5"
240+
disabled
241+
readOnly
240242
/>
241243
<div className="mt-4 flex items-start gap-1.5">
242244
<Lock size={12} className="text-text-muted shrink-0 mt-0.5" />

apps/web/src/pages/skills.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type { InstalledSkill, MinimalSkill } from "@/types/desktop";
1616
import { Compass, Loader2, Plus, Search, Settings2, Zap } from "lucide-react";
1717
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
1818
import { useTranslation } from "react-i18next";
19-
import { Link } from "react-router-dom";
19+
import { Link, useSearchParams } from "react-router-dom";
2020
import { toast } from "sonner";
2121

2222
type TopTab = "explore" | "yours";
@@ -265,7 +265,12 @@ export function SkillsPage() {
265265

266266
const [searchQuery, setSearchQuery] = useState("");
267267
const debouncedQuery = useDebounce(searchQuery, 150);
268-
const [topTab, setTopTab] = useState<TopTab>("yours");
268+
const [searchParams, setSearchParams] = useSearchParams();
269+
const topTab: TopTab =
270+
searchParams.get("tab") === "explore" ? "explore" : "yours";
271+
const setTopTab = (tab: TopTab) => {
272+
setSearchParams(tab === "yours" ? {} : { tab }, { replace: true });
273+
};
269274
const [yoursSubTab, setYoursSubTab] = useState<YoursSubTab>("all");
270275
const [activeTag, setActiveTag] = useState<string | null>(null);
271276
const [visibleCount, setVisibleCount] = useState(PAGE_SIZE);
@@ -713,7 +718,7 @@ export function SkillsPage() {
713718
className="text-[var(--color-accent)] hover:underline"
714719
>
715720
GitHub Issues
716-
</a>
721+
</a>{" "}
717722
{t("skills.clawhubDisclaimerAfterLink")}
718723
</p>
719724

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import "../../apps/controller/tests/provider-oauth-routes.test.ts";

0 commit comments

Comments
 (0)