fix(client): resolve hostContext.theme stuck on "light" when dark mode toggled#1983
fix(client): resolve hostContext.theme stuck on "light" when dark mode toggled#1983mdodell wants to merge 2 commits intoMCPJam:mainfrom
Conversation
…dark mode is toggled Theme had two competing sources of truth — `themeMode` in the preferences store (set by the Settings toggle) and `draftHostContext.theme` baked into the default host context by `buildDefaultWorkspaceHostContext`. When the global theme changed, `loadWorkspaceHostContext` silently skipped the update whenever the store was dirty or a saved config existed, so the stale "light" value always won the resolution chain. Remove `theme` from the default workspace host context so it is resolved at render time from the global preference instead. The playground theme toggle now updates the global preference directly. Closes MCPJam#1982 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughTheme is removed from workspace host-context and client-config builders; 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
|
Nice fix. I think there is still one saved-config path worth covering before merge: if a workspace already has a saved/draft host context with A focused regression would be: set |
… global prefs The theme toggle in HostContextHeader was calling updateThemeMode/setThemeMode which changed the global app shell preference. The intent is for the app-builder theme to be independent — persisting with the workspace host context (like locale and timezone) so MCP apps receive the correct theme via hostContext.theme. MCPAppsRenderer no longer reads configuredHostTheme from draftHostContext directly; theme reaches it through ChatboxHostThemeProvider -> chatboxHostTheme, which PlaygroundMain feeds from extractHostTheme(draftHostContext) ?? globalThemeMode. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/__tests__/mcp-apps-renderer.test.tsx (1)
384-413: ⚡ Quick winStrengthen this regression test with dark-token verification.
Lines 399 and 409 assert
theme: "dark", but the bug contract also requires dark SEP variables. Please asserthostContext.styles.variablesreflects dark-mode tokens in both initialize and update paths.Proposed test augmentation
+import { getHostStyleOrDefault } from "@/lib/host-styles"; ... it("resolves theme from host context provider, ignoring draftHostContext.theme", async () => { Object.assign(mockPlaygroundStoreState, { isPlaygroundActive: true, }); mockPreferencesState.themeMode = "dark"; + const expectedDarkVariables = + getHostStyleOrDefault(mockPreferencesState.hostStyle).resolveStyleVariables( + "dark", + ); mockHostContextStoreState.draftHostContext = { theme: "light", }; ... expect(appBridgeArgsRef.current?.options?.hostContext?.theme).toBe("dark"); + expect( + appBridgeArgsRef.current?.options?.hostContext?.styles?.variables, + ).toMatchObject(expectedDarkVariables); ... expect(mockBridge.setHostContext).toHaveBeenLastCalledWith( expect.objectContaining({ theme: "dark", + styles: expect.objectContaining({ + variables: expect.objectContaining(expectedDarkVariables), + }), }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/__tests__/mcp-apps-renderer.test.tsx` around lines 384 - 413, The test currently asserts the theme is "dark" but must also verify dark-mode SEP variables are present; update the MCPAppsRenderer test to additionally assert that appBridgeArgsRef.current?.options?.hostContext?.styles?.variables contains the expected dark-mode tokens right after render/initialization, and also that mockBridge.setHostContext was last called with an object whose hostContext.styles.variables reflect the same dark tokens after triggerReady(); reference the MCPAppsRenderer component, appBridgeArgsRef, mockBridge.connect, mockBridge.setHostContext, and triggerReady to locate where to add these two assertions (one for the initial appBridgeArgsRef snapshot and one inside the expect(mockBridge.setHostContext).toHaveBeenLastCalledWith(...)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/__tests__/mcp-apps-renderer.test.tsx`:
- Around line 384-413: The test currently asserts the theme is "dark" but must
also verify dark-mode SEP variables are present; update the MCPAppsRenderer test
to additionally assert that
appBridgeArgsRef.current?.options?.hostContext?.styles?.variables contains the
expected dark-mode tokens right after render/initialization, and also that
mockBridge.setHostContext was last called with an object whose
hostContext.styles.variables reflect the same dark tokens after triggerReady();
reference the MCPAppsRenderer component, appBridgeArgsRef, mockBridge.connect,
mockBridge.setHostContext, and triggerReady to locate where to add these two
assertions (one for the initial appBridgeArgsRef snapshot and one inside the
expect(mockBridge.setHostContext).toHaveBeenLastCalledWith(...)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3911cc3f-f32a-45ac-aba8-465514c7b7e9
📒 Files selected for processing (5)
mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/__tests__/mcp-apps-renderer.test.tsxmcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/mcp-apps-renderer.tsxmcpjam-inspector/client/src/components/shared/HostContextHeader.tsxmcpjam-inspector/client/src/components/shared/__tests__/HostContextHeader.test.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx
✅ Files skipped from review due to trivial changes (3)
- mcpjam-inspector/client/src/components/shared/tests/HostContextHeader.test.tsx
- mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx
- mcpjam-inspector/client/src/components/shared/HostContextHeader.tsx
Fixes #1982
hostContext.themeand style variables always resolved to"light"even after toggling dark mode in Settings. The globalthemeModepreference was baked intobuildDefaultWorkspaceHostContext, butloadWorkspaceHostContextsilently skipped updates when the store was dirty or had a saved config — so the stale value always won.Removes
themefrom the default workspace host context so the resolution chain falls through tothemeModedirectly. The playground theme toggle now updates the global preference instead of patchingdraftHostContext.