feat(user-testing): address a scenario by its chatbox id, not its host - #3761
Conversation
`/user-testing/:scenarioId` carried a HOST id, from when every scenario was a client and the two were 1:1. Environment-backed scenarios break that in both directions: several can point at the same host, and the host-keyed query (`getChatboxByHostId`) deliberately refuses to return them, so an environment scenario had no addressable detail view at all. The chatbox id is the identity both kinds share — and the one everything downstream (sessions, clusters, insights, the share section) was already keyed by. Three places were still building links from `namedHostId`, including the session link the Sessions pane hands you to copy. Old links keep working. A `:scenarioId` that matches a host instead of a chatbox redirects onto that host's scenario, preserving query and hash, as does the older `?host=` form. That match ignores environment-backed rows — they carry a `namedHostId` for display only, and several can share one, so matching on it would hand an old link to an unrelated scenario. Same rule the backend's `getHostPublishChatbox` enforces. Deletes the mount-time back-mint and its three pieces of state (a per-host latch, a suppress set for intentional deletes, a 1500ms stuck-timer). It existed because opening a chatbox-less host had to provision one; a scenario is now something that already exists, so a client without one simply isn't one. The mutation survives for the agent's publish tool, where provisioning is the explicit request rather than a side effect of looking at a URL — and that handler now navigates with the id the mutation returns instead of bouncing through the redirect. Detail and list learn the environment fields from mcpjam-backend #889: an environment-backed scenario shows the ENVIRONMENT as its identity, and a row whose environment can't resolve is badged rather than rendered as a confident, empty-looking row. The list keeps showing it — its share link is minted, and the person who can retire it has to be able to see it. The create flow still writes a host (that becomes environment-first in a follow-up); it navigates with the host id and the ladder resolves it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1c032999-8558-40b3-befb-8f0df002f44e) |
✅ 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. |
WalkthroughUser Testing routes now use chatbox IDs as canonical scenario identifiers. Legacy host-based links redirect while preserving query and hash data. Scenario resolution validates chatbox list entries before detail queries and no longer provisions chatboxes during mounting. Agent publish and delete actions use resolved chatboxes. Overview and detail views display environment metadata and resolution errors. Tests cover routing, redirects, missing scenarios, environment states, and agent actions. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@mcpjam-inspector/client/src/components/chatboxes/UserTestingScenarioDetail.tsx`:
- Around line 78-84: Update the scenario identity branching in
UserTestingScenarioDetail to use chatbox.environmentId rather than
environmentName, including the related branch around the alternate referenced
lines. For environment-backed scenarios with no resolvable environmentName,
preserve that identity and fall back to the existing placeholder instead of
displaying namedHostName.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ce9fe2c-049f-4c72-907a-7216a3f1c858
📒 Files selected for processing (12)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/__tests__/ChatboxesRoute.billing.test.tsxmcpjam-inspector/client/src/components/UserTestingTab.tsxmcpjam-inspector/client/src/components/__tests__/UserTestingTab.agent.test.tsxmcpjam-inspector/client/src/components/__tests__/UserTestingTab.journeys.test.tsxmcpjam-inspector/client/src/components/chatboxes/ChatboxUsagePanel.tsxmcpjam-inspector/client/src/components/chatboxes/UserTestingOverviewPanel.tsxmcpjam-inspector/client/src/components/chatboxes/UserTestingScenarioDetail.tsxmcpjam-inspector/client/src/components/chatboxes/__tests__/UserTestingOverviewPanel.test.tsxmcpjam-inspector/client/src/components/chatboxes/__tests__/UserTestingScenarioDetail.test.tsxmcpjam-inspector/client/src/hooks/useChatboxes.tsmcpjam-inspector/client/src/lib/app-navigation.ts
| const environmentName = chatbox.environmentName ?? null; | ||
| // Present only when the environment can't resolve right now (archived, a | ||
| // pinned plugin disabled, its host gone). The scenario still opens: its | ||
| // sessions are history worth reading, and unpublishing it is the action | ||
| // this state calls for. | ||
| const environmentError = chatbox.environmentError ?? null; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Branch identity on environmentId, not environmentName.
An environment-backed scenario whose environment cannot resolve may return environmentId with a null environmentName. This condition then falls into the host-backed branch and presents namedHostName as the scenario's identity. The ChatboxListItem contract states that name is display-only on environment-backed rows, so the header would claim an identity the data does not support — immediately above the banner that says the environment is broken.
Key the branch on environmentId and let the name degrade to a placeholder.
🔧 Proposed fix
+ const isEnvironmentBacked = Boolean(chatbox.environmentId);
const environmentName = chatbox.environmentName ?? null;- {environmentName ? (
+ {isEnvironmentBacked ? (
// Environment-backed: the environment IS the scenario's
// identity. Its client is a detail of the environment, not a
// second name for the thing.
<>
<Layers className="size-4 shrink-0" />
<span className="truncate font-medium text-foreground">
- {environmentName}
+ {environmentName ?? "Unresolved environment"}
</span>
</>
) : (Also applies to: 147-157
🤖 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/chatboxes/UserTestingScenarioDetail.tsx`
around lines 78 - 84, Update the scenario identity branching in
UserTestingScenarioDetail to use chatbox.environmentId rather than
environmentName, including the related branch around the alternate referenced
lines. For environment-backed scenarios with no resolvable environmentName,
preserve that identity and fall back to the existing placeholder instead of
displaying namedHostName.
Why
/user-testing/:scenarioIdcarried a host id, from when every scenario was a client and the two were 1:1. Environment-backed scenarios break that in both directions: several can point at the same host, and the host-keyed query (getChatboxByHostId) deliberately filters them out — so an environment-backed scenario had no addressable detail view at all.The chatbox id is the identity both kinds share, and the one everything downstream — sessions, clusters, insights, the share section — was already keyed by. Three places were still minting links from
namedHostId, including the session link the Sessions pane hands you to copy.Backend groundwork for this shipped in mcpjam-backend #887 (publish contract), #889 (environment-aware projections) and #890 (guest ENV_* statuses).
Old links keep working
A
:scenarioIdthat matches a host instead of a chatbox redirects onto that host's scenario, preserving query and hash; the older?host=form still translates too.That match ignores environment-backed rows. They carry a
namedHostIdfor display only and several can share one, so matching on it would hand an old link to an unrelated scenario — the same rule the backend'sgetHostPublishChatboxenforces. There's a test for exactly this.What's deleted
The mount-time back-mint, and its three pieces of state: a per-host latch, a suppress set for intentional deletes, and a 1500 ms stuck-timer. It existed because opening a chatbox-less host had to provision one on the way in. A scenario is now something that already exists — a client without one simply isn't one, and says so.
The mutation itself survives for one caller: the agent's
ui_publish_chatbox, where provisioning is the explicit request rather than a side effect of looking at a URL. That handler now navigates with the id the mutation returns instead of bouncing through the redirect.What's added
Detail and list read the environment fields from #889:
Not in this PR
The create flow still writes a host (environment-first create + the list filter that drops auto-minted client rows is the next PR). It navigates with the host id and the ladder resolves it. The agent tools stay host-addressed; re-pointing them at scenario identity is its own PR.
Tests
Rewrote
UserTestingTab.journeys.test.tsxaround the resolution ladder — the old file tested back-minting, which no longer exists. It now pins: legacy host-id link redirects; an environment-backed row never absorbs that host's links; unknown id is not-found and never reaches the wire (getChatboxdeclaresv.id('chatboxes'), so an unknown id throws out ofuseQueryand white-screens the app); a Journeys host still gets the Swarms dead-end; nothing is provisioned on mount.Also updated: overview panel (opens by chatbox id, badges an unresolvable row), detail (env identity, ENV_ARCHIVED state, chatbox-id nav), agent bridge, billing route.
Full
client/srcsuite was re-run separately; result reported on the PR.🤖 Generated with Claude Code
Note
Medium Risk
Routing and deep-link identity changed across User Testing with careful legacy handling; behavior is well-tested but affects bookmarks, agent navigation, and scenario resolution for mixed host/environment rows.
Overview
User Testing now treats
/user-testing/:scenarioIdas a chatbox id (not a host id), so environment-backed scenarios get a real detail view. Validation runs against the chatbox list beforegetChatboxruns, avoiding invalid ids crashinguseQuery.Legacy links still work: a param that matches a host (non–environment-backed row only) redirects to that scenario’s chatbox id with query and hash preserved;
?host=is still translated. Environment-backed rows never absorb a host’s old links.Removed the mount-time
ensureChatboxForHostback-mint and all related latch/suppress/stuck-timer state; hosts without a chatbox show not-found. AgentpublishChatboxstill provisions explicitly and navigates with the returnedchatboxId;deleteChatboxtargets the host’s own publish row (!environmentId).UI/types add environment name and
environmentErroron list and detail (warning banner, list badge); navigation and session links usechatboxId. Docs and tests were updated for the resolution ladder and removed back-mint behavior.Reviewed by Cursor Bugbot for commit edec0b0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Switch User Testing routes to address scenarios by chatbox id instead of host id. Legacy links still work via redirect, and environment-backed scenarios now display their environment and warn when it can’t resolve.
New Features
/user-testing/:scenarioIdnow takes a chatbox id. Host-id and?host=links redirect to the chatbox, preserving query and hash.Refactors
ui_publish_chatbox, which now navigates to the returned chatbox id. Deletes no longer re-provision.scenarioHostIdwithscenarioId,useChatboxByHostIdwithuseChatbox, and updated navigation and tests. Overview row clicks and usage panel links now use chatbox ids.Written for commit edec0b0. Summary will update on new commits.