Group useChatSession props into ExecutionConfig and HostedRuntimeContext#1925
Group useChatSession props into ExecutionConfig and HostedRuntimeContext#1925chelojimenez merged 1 commit intomainfrom
Conversation
Reduces prop sprawl across all useChatSession call sites by grouping related props into two typed objects: - ExecutionConfig: modelId, systemPrompt, temperature, requireToolApproval - HostedRuntimeContext: workspaceId, selectedServerIds, oauthTokens, shareToken, chatboxToken, chatboxSurface Also fixes EvalChatHandoff missing requireToolApproval — continuing an eval in chat now preserves the tool approval setting. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
dosubot
Bot
added
the
size:L
✅ 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. |
Internal previewPreview URL: https://mcp-inspector-pr-1925.up.railway.app |
WalkthroughThis pull request consolidates fragmented configuration props across chat components and hooks into two structured objects: 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
mcpjam-inspector/client/src/components/hosted/SharedServerChatPage.tsx (1)
486-491: Consolidate override props intohostedContextfor uniform structure.
hostedWorkspaceIdOverride,hostedSelectedServerIdsOverride, andhostedOAuthTokensOverrideare computed into "effective" values and merged intohostedContextbefore every downstream pass (lines 381–385, 2173–2176). Passing three as flat props whileshareTokenrides inhostedContextcreates an unnecessary asymmetry—all six fieldsHostedRuntimeContextwas designed to hold belong together.In
SharedServerChatPage, this becomes a partial migration: three workspace/token slots passed as*Overridewhile onlyshareTokenentershostedContext.Suggested consolidation:
♻️ Fold all four into hostedContext
- hostedWorkspaceIdOverride={session.payload.workspaceId} - hostedSelectedServerIdsOverride={[session.payload.serverId]} - hostedOAuthTokensOverride={oauthTokensForChat} - hostedContext={{ shareToken: session.token }} + hostedContext={{ + workspaceId: session.payload.workspaceId, + selectedServerIds: [session.payload.serverId], + oauthTokens: oauthTokensForChat, + shareToken: session.token, + }}Then remove the now-unused
*Overrideprops fromChatTabProps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/hosted/SharedServerChatPage.tsx` around lines 486 - 491, Consolidate the three override props into the hostedContext object so all HostedRuntimeContext fields travel together: in SharedServerChatPage replace hostedWorkspaceIdOverride, hostedSelectedServerIdsOverride and hostedOAuthTokensOverride by merging their computed "effective" values into the existing hostedContext (which currently contains shareToken) before passing to the child ChatTab component; update the ChatTabProps usage to accept the expanded hostedContext (HostedRuntimeContext) and remove the now-unused hostedWorkspaceIdOverride/hostedSelectedServerIdsOverride/hostedOAuthTokensOverride props, keeping onOAuthRequired/handleOAuthRequired as-is and preserving existing names for clarity.mcpjam-inspector/client/src/components/ChatTabV2.tsx (1)
2167-2177: Forward-compat nit: rebuild the childexecutionConfigfrom the prop, not just from local state.Today the three live state values (
systemPrompt,temperature,requireToolApproval) are equivalent to whatever the parent supplied viaexecutionConfig, sinceuseChatSessionsyncs them in effects. Functionally correct. The brittleness is structural: the dayExecutionConfiggrows another field, this site silently drops it for multi-model cards while the single-model path keeps working. Spreading the prop and letting state win on the three already-tracked fields is a one-line hedge.♻️ Suggested forward-compatible shape
- executionConfig={{ - systemPrompt, - temperature, - requireToolApproval, - }} + executionConfig={{ + ...executionConfig, + systemPrompt, + temperature, + requireToolApproval, + }}Note:
modelIdis intentionally not propagated here —MultiModelChatCardoverrides it per card withString(model.id), so an inheritedexecutionConfig.modelIdwould be harmlessly overwritten downstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/ChatTabV2.tsx` around lines 2167 - 2177, The current inline executionConfig object only uses local state (systemPrompt, temperature, requireToolApproval) and drops any other fields from the incoming prop; change the JSX to build executionConfig by spreading the incoming prop executionConfig first and then overriding the three tracked fields so other future fields are preserved, e.g. use {...executionConfig, systemPrompt, temperature, requireToolApproval}; keep in mind MultiModelChatCard still overrides modelId downstream.
🤖 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/ChatTabV2.tsx`:
- Around line 2167-2177: The current inline executionConfig object only uses
local state (systemPrompt, temperature, requireToolApproval) and drops any other
fields from the incoming prop; change the JSX to build executionConfig by
spreading the incoming prop executionConfig first and then overriding the three
tracked fields so other future fields are preserved, e.g. use
{...executionConfig, systemPrompt, temperature, requireToolApproval}; keep in
mind MultiModelChatCard still overrides modelId downstream.
In `@mcpjam-inspector/client/src/components/hosted/SharedServerChatPage.tsx`:
- Around line 486-491: Consolidate the three override props into the
hostedContext object so all HostedRuntimeContext fields travel together: in
SharedServerChatPage replace hostedWorkspaceIdOverride,
hostedSelectedServerIdsOverride and hostedOAuthTokensOverride by merging their
computed "effective" values into the existing hostedContext (which currently
contains shareToken) before passing to the child ChatTab component; update the
ChatTabProps usage to accept the expanded hostedContext (HostedRuntimeContext)
and remove the now-unused
hostedWorkspaceIdOverride/hostedSelectedServerIdsOverride/hostedOAuthTokensOverride
props, keeping onOAuthRequired/handleOAuthRequired as-is and preserving existing
names for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52753a68-993f-4cdb-b0dc-11421fecd7d3
📒 Files selected for processing (20)
mcpjam-inspector/client/src/components/ChatTabV2.tsxmcpjam-inspector/client/src/components/chat-v2/__tests__/multi-model-chat-card.test.tsxmcpjam-inspector/client/src/components/chat-v2/multi-model-chat-card.tsxmcpjam-inspector/client/src/components/chatboxes/ChatboxEditor.tsxmcpjam-inspector/client/src/components/chatboxes/__tests__/ChatboxEditor.test.tsxmcpjam-inspector/client/src/components/chatboxes/builder/ChatboxBuilderView.tsxmcpjam-inspector/client/src/components/evals/test-template-editor.tsxmcpjam-inspector/client/src/components/hosted/ChatboxChatPage.tsxmcpjam-inspector/client/src/components/hosted/SharedServerChatPage.tsxmcpjam-inspector/client/src/components/hosted/__tests__/ChatboxChatPage.test.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsxmcpjam-inspector/client/src/components/ui-playground/__tests__/multi-model-playground-card.test.tsxmcpjam-inspector/client/src/components/ui-playground/multi-model-playground-card.tsxmcpjam-inspector/client/src/hooks/__tests__/use-chat-session.fork.test.tsxmcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsxmcpjam-inspector/client/src/hooks/__tests__/use-chat-session.minimal-mode.test.tsxmcpjam-inspector/client/src/hooks/use-chat-session.tsmcpjam-inspector/client/src/lib/chat-execution-config.tsmcpjam-inspector/client/src/lib/eval-chat-handoff.tsmcpjam-inspector/client/src/lib/hosted-runtime-context.ts
1 participant
Summary
EvalChatHandoffmissingrequireToolApproval— continuing an eval in chat now preserves tool approval state. The field will beundefineduntil evals gain tool approval support, but the plumbing is ready.initial*props intoexecutionConfig: ExecutionConfig—modelId,systemPrompt,temperature,requireToolApprovaltravel as one object throughuseChatSession,MultiModelChatCard,MultiModelPlaygroundCard, and all chatbox/hosted surfaces.hosted*props intohostedContext: HostedRuntimeContext—workspaceId,selectedServerIds,oauthTokens,shareToken,chatboxToken,chatboxSurfacetravel as one object through the same surfaces.Prep work for a future Host abstraction —
ExecutionConfigbecomes the execution slice,HostedRuntimeContextbecomes the runtime slice.Test plan
🤖 Generated with Claude Code
Note
Medium Risk
Broad prop-shape refactor across chat, hosted, chatbox, and playground surfaces; risk is mainly regressions from missing/incorrectly forwarded fields (model/prompt/temperature/tool approval or hosted tokens) rather than logic changes.
Overview
Refactors chat session wiring by replacing scattered
initial*andhosted*props with two typed objects:executionConfig(model/system prompt/temperature/tool approval) andhostedContext(workspace/server IDs/OAuth + share/chatbox scope) acrossChatTabV2,useChatSession, multi-model cards, playground, and hosted/chatbox entrypoints.Fixes eval handoff completeness by adding
requireToolApprovaltoEvalChatHandoffand applying it when continuing an eval in chat.Adds new shared types
ExecutionConfigandHostedRuntimeContext, and updates affected unit tests to use the new prop shapes.Reviewed by Cursor Bugbot for commit f8944a7. Bugbot is set up for automated code reviews on this repo. Configure here.