Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

apps/web/src/pages/skills.tsx (1)

268-273: Consider using functional update to preserve other search params.

The current implementation replaces all search params when switching tabs. If other params are added later (e.g., filters, pagination), they would be lost when toggling tabs.

♻️ Suggested safer pattern

 const [searchParams, setSearchParams] = useSearchParams();
 const topTab: TopTab =
   searchParams.get("tab") === "explore" ? "explore" : "yours";
 const setTopTab = (tab: TopTab) => {
-  setSearchParams(tab === "yours" ? {} : { tab }, { replace: true });
+  setSearchParams((prev) => {
+    const next = new URLSearchParams(prev);
+    if (tab === "yours") {
+      next.delete("tab");
+    } else {
+      next.set("tab", tab);
+    }
+    return next;
+  }, { replace: true });
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/pages/skills.tsx` around lines 268 - 273, The setTopTab
implementation currently calls setSearchParams with a new object and clobbers
any existing query parameters; update it to use the functional form of
setSearchParams so you can read and mutate the existing URLSearchParams instead
of replacing them wholesale: inside setTopTab (and keep topTab and
useSearchParams unchanged) call setSearchParams(prev => { const params = new
URLSearchParams(prev); if (tab === "yours") params.delete("tab"); else
params.set("tab", "explore"); return params; }, { replace: true }) so other
params (filters, page, etc.) are preserved when toggling tabs.

apps/controller/tests/provider-oauth-routes.test.ts (1)

202-219: Assert the rest of the cleanup path stays untouched on ok: false.

This failure-path test only guards deleteProvider(). If the handler regresses and still calls ensureValidDefaultModel() or syncAll(), this case stays green.

✅ Tighten the failure-path assertions

     await expect(response.json()).resolves.toEqual({ ok: false });
     expect(
       container.modelProviderService.deleteProvider,
     ).not.toHaveBeenCalled();
+    expect(
+      container.modelProviderService.ensureValidDefaultModel,
+    ).not.toHaveBeenCalled();
+    expect(container.openclawSyncService.syncAll).not.toHaveBeenCalled();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/controller/tests/provider-oauth-routes.test.ts` around lines 202 - 219,
Tighten the failure-path test so it also asserts other cleanup routines are not
invoked when disconnectOAuth resolves to false: after the existing expects add
assertions that container.modelProviderService.ensureValidDefaultModel and the
sync method (container.providerSyncService.syncAll) were not called; this
ensures the handler doesn't call ensureValidDefaultModel() or syncAll() on an
OAuth disconnect failure.

apps/controller/src/services/model-provider-service.ts (1)

259-260: Avoid a third hardcoded OAuth provider registry.

"openai" is already encoded in apps/controller/src/runtime/openclaw-auth-profiles-store.ts:16-18 and apps/controller/src/lib/openclaw-config-compiler.ts:37-39, while this file also has separate MiniMax OAuth handling. Pull this from a shared source—or rename it to the narrower concept it actually represents—so provider capability data does not drift across files.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/controller/src/services/model-provider-service.ts` around lines 259 -
260, Replace the hardcoded OAUTH_PROVIDER_IDS set with the canonical/shared
source or narrow its meaning: either import the central OAuth-capability
registry used elsewhere and replace the local constant OAUTH_PROVIDER_IDS with
that shared symbol, or if this set only reflects MiniMax-specific behavior,
rename OAUTH_PROVIDER_IDS to a narrower identifier (e.g.,
MINIMAX_OAUTH_PROVIDER_IDS) and update usages in the model-provider-service
methods to match; ensure you remove the duplicated literal array and reference
the single source of truth so provider capability data no longer drifts across
files.