Feat/host compare default codex claude code#2997
Conversation
…on Windows npx resolves to the npx.cmd shim, which execFile can't invoke without a shell on Windows (spawn EINVAL), and routing through a shell instead breaks because the PEM values we pass as argv get split on their embedded newlines. Invoke the Convex CLI's bin entry via node directly to sidestep both.
Host Compare (including the caniuse.dev landing) previously fell back to "all live hosts" when nothing else picked a selection, which is empty for anyone who hasn't created a host yet. Default to the Codex and Claude Code presets instead so a fresh visit shows a meaningful comparison immediately, with live hosts as the fallback if those presets are ever unavailable.
dosubot
Bot
added
the
size:M
|
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. |
Internal previewPreview URL: https://mcp-inspector-pr-2997.up.railway.app |
WalkthroughThis PR changes two unrelated areas. In the client, a new Related PRsNo related PRs were provided in the given context. Suggested labels
Suggested reviewersNo reviewer information was provided in the given context. Poem Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/components/hosts/comparison/__tests__/host-compare-selection.test.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/components/hosts/comparison/host-compare-selection.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcpjam-inspector/client/src/components/hosts/comparison/host-compare-selection.ts">
<violation number="1" location="mcpjam-inspector/client/src/components/hosts/comparison/host-compare-selection.ts:116">
P3: A fresh Host Compare visit can default to a single preset if only one of Codex/Claude Code is present, producing a partial one-column default instead of the documented live-host fallback. Since the default is intended as the Codex + Claude Code pair, consider requiring both default ids before returning it.</violation>
</file>
<file name="mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx:137">
P3: The fallback default path can still dirty clean compare URLs: when the Codex/Claude Code preset ids are unavailable, the resolver returns the live-host fallback but this mirror check only suppresses the exact preset pair. Consider deriving the suppressed default from the same reconciliation/fallback logic used by resolveInitialHostCompareSelection.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| DEFAULT_COMPARE_HOST_IDS, | ||
| known, | ||
| ); | ||
| if (fromDefaultPresets.length > 0) return fromDefaultPresets; |
There was a problem hiding this comment.
P3: A fresh Host Compare visit can default to a single preset if only one of Codex/Claude Code is present, producing a partial one-column default instead of the documented live-host fallback. Since the default is intended as the Codex + Claude Code pair, consider requiring both default ids before returning it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/hosts/comparison/host-compare-selection.ts, line 116:
<comment>A fresh Host Compare visit can default to a single preset if only one of Codex/Claude Code is present, producing a partial one-column default instead of the documented live-host fallback. Since the default is intended as the Codex + Claude Code pair, consider requiring both default ids before returning it.</comment>
<file context>
@@ -91,5 +106,14 @@ export function resolveInitialHostCompareSelection(args: {
+ DEFAULT_COMPARE_HOST_IDS,
+ known,
+ );
+ if (fromDefaultPresets.length > 0) return fromDefaultPresets;
+
return [...args.liveHostIds];
</file context>
| if (fromDefaultPresets.length > 0) return fromDefaultPresets; | |
| if (fromDefaultPresets.length === DEFAULT_COMPARE_HOST_IDS.length) return fromDefaultPresets; |
| const isDefault = | ||
| selectedHostIds.length === liveHostIds.length && | ||
| selectedHostIds.every((id, i) => id === liveHostIds[i]); | ||
| selectedHostIds.length === DEFAULT_COMPARE_HOST_IDS.length && |
There was a problem hiding this comment.
P3: The fallback default path can still dirty clean compare URLs: when the Codex/Claude Code preset ids are unavailable, the resolver returns the live-host fallback but this mirror check only suppresses the exact preset pair. Consider deriving the suppressed default from the same reconciliation/fallback logic used by resolveInitialHostCompareSelection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx, line 137:
<comment>The fallback default path can still dirty clean compare URLs: when the Codex/Claude Code preset ids are unavailable, the resolver returns the live-host fallback but this mirror check only suppresses the exact preset pair. Consider deriving the suppressed default from the same reconciliation/fallback logic used by resolveInitialHostCompareSelection.</comment>
<file context>
@@ -126,12 +129,13 @@ export function HostConfigCompareView({
const isDefault =
- selectedHostIds.length === liveHostIds.length &&
- selectedHostIds.every((id, i) => id === liveHostIds[i]);
+ selectedHostIds.length === DEFAULT_COMPARE_HOST_IDS.length &&
+ selectedHostIds.every((id, i) => id === DEFAULT_COMPARE_HOST_IDS[i]);
const current = searchParams.get(HOSTS_QUERY_PARAM);
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx (1)
121-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOrder-sensitive
isDefaultcheck can spuriously show?hosts=for the default set.
isDefaultrequires exact positional equality againstDEFAULT_COMPARE_HOST_IDS(Line 136-138). ButtoggleHostCompareSelectionappends newly-added ids to the end of the array (per the test:toggleHostCompareSelection(["a"], "b")→["a", "b"]). If a user toggles off Codex and re-toggles it back on while Claude Code remains selected, the resulting order becomes["claude-code", "codex"]— the same set as the default, but positionally different — soisDefaultevaluatesfalseand the URL is populated with?hosts=...even though the selection is functionally the default. This undercuts the stated goal that "shared links stay clean" (Line 122) for the common toggle-off/toggle-on interaction.Consider comparing as an order-independent set instead of index-by-index equality.
🔧 Suggested fix
- const isDefault = - selectedHostIds.length === DEFAULT_COMPARE_HOST_IDS.length && - selectedHostIds.every((id, i) => id === DEFAULT_COMPARE_HOST_IDS[i]); + const defaultIdSet = new Set(DEFAULT_COMPARE_HOST_IDS); + const isDefault = + selectedHostIds.length === DEFAULT_COMPARE_HOST_IDS.length && + selectedHostIds.every((id) => defaultIdSet.has(id));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx` around lines 121 - 152, The default-selection check in HostConfigCompareView is order-sensitive, so the URL can incorrectly keep ?hosts= even when the selected hosts match DEFAULT_COMPARE_HOST_IDS in a different order. Update the isDefault logic inside the useEffect to compare selectedHostIds as an order-independent set (or normalized order) before deciding whether to delete HOSTS_QUERY_PARAM. Keep the existing urlConsumedRef and toggleHostCompareSelection behavior, but make the default detection robust to re-adding hosts in a different sequence.
🧹 Nitpick comments (1)
mcpjam-inspector/server/utils/convex-guest-auth-sync.ts (1)
27-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRead the Convex CLI entry from
package.json#bin. The package currently declaresbin/main.js, so resolving it from metadata keeps this aligned if Convex changes its entrypoint later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcpjam-inspector/server/utils/convex-guest-auth-sync.ts` around lines 27 - 28, The Convex CLI entry is being hardcoded from the package directory instead of using the package metadata. Update the logic in convex-guest-auth-sync.ts around cachedConvexCliEntry and require.resolve("convex/package.json") to read the CLI entry from package.json#bin, then resolve that path relative to the package root so it stays aligned if Convex changes its entrypoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsx`:
- Around line 121-152: The default-selection check in HostConfigCompareView is
order-sensitive, so the URL can incorrectly keep ?hosts= even when the selected
hosts match DEFAULT_COMPARE_HOST_IDS in a different order. Update the isDefault
logic inside the useEffect to compare selectedHostIds as an order-independent
set (or normalized order) before deciding whether to delete HOSTS_QUERY_PARAM.
Keep the existing urlConsumedRef and toggleHostCompareSelection behavior, but
make the default detection robust to re-adding hosts in a different sequence.
---
Nitpick comments:
In `@mcpjam-inspector/server/utils/convex-guest-auth-sync.ts`:
- Around line 27-28: The Convex CLI entry is being hardcoded from the package
directory instead of using the package metadata. Update the logic in
convex-guest-auth-sync.ts around cachedConvexCliEntry and
require.resolve("convex/package.json") to read the CLI entry from
package.json#bin, then resolve that path relative to the package root so it
stays aligned if Convex changes its entrypoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44f80126-8e93-454a-a491-8249438a8700
📒 Files selected for processing (4)
mcpjam-inspector/client/src/components/hosts/comparison/HostConfigCompareView.tsxmcpjam-inspector/client/src/components/hosts/comparison/__tests__/host-compare-selection.test.tsmcpjam-inspector/client/src/components/hosts/comparison/host-compare-selection.tsmcpjam-inspector/server/utils/convex-guest-auth-sync.ts
2 participants
fix(server) + feat(hosts): Windows guest-auth fix, default Host Compare to Codex + Claude Code
Branch:
feat/host-compare-default-codex-claude-code(2 commits on top ofmain)Summary
node./clients/compareand thecaniuse.devembed) to Codex + Claude Code instead of an empty "all live hosts" fallback.Why
Changes
convex-guest-auth-sync.ts— resolve/invoke Convex CLI vianode, notnpx.host-compare-selection.ts— newDEFAULT_COMPARE_HOST_IDSfallback.HostConfigCompareView.tsx— URL-default check updated to match.host-compare-selection.test.ts— updated/added tests (13/13 passing).Test plan
Summary by cubic
Fixes Windows guest-auth provisioning by invoking the
convexCLI via Node, and defaults Host Compare to Codex + Claude Code so fresh visits and embeds show a useful comparison.New Features
/clients/compareand the caniuse.dev embed) to Codex + Claude Code; falls back to live hosts if presets are unavailable.?hosts=when the selection matches the default order.Bug Fixes
node <convex CLI entry>instead ofnpxto avoidspawn EINVALand newline-arg splitting, unblocking local dev.Written for commit 06c2d3c. Summary will update on new commits.