fix: wire showPostConnectGuide to real onboarding state (BB-112)

[nachocossio]

Summary

• The MCPJam logo + "try asking Excalidraw..." prompt were getting lost during first-run onboarding (BB-112).
• Root cause: showPostConnectGuide was hardcoded to false in PlaygroundCenter.tsx when rendering PlaygroundMain, so the guided post-connect screen (PostConnectGuide) could never render — users always fell through to the generic playground screen instead, even once useOnboarding's state machine correctly reached connected_guided.
• Fix: pass the real state.onboarding.isGuidedPostConnect value instead of the hardcoded false. One-line change; isGuidedPostConnect was already computed correctly and already used one line below for a different purpose.
• This restores behavior that was intentionally built (see PostConnectGuide.tsx and its existing test asserting the "Try asking Excalidraw to draw something." copy) but never wired up after the Playground was rebuilt in feat(inspector): playground tab (IDE-style Chat + App Builder) #2121.

Test plan

• Added PlaygroundCenter.test.tsx: fails on the old hardcoded false (confirmed RED), passes after the fix (confirmed GREEN).
• Ran full related suite (PlaygroundMain, use-onboarding, use-composer-onboarding, playground/__tests__): 181/181 passing, no regressions.
• Confirmed via git blame this is the only production call site rendering PlaygroundMain with onboarding wiring (the Evals live-chat panel intentionally omits the prop).
• Visually verified in a throwaway harness that toggling the flag renders PostConnectGuide (existing, already-styled component) instead of the generic screen.

🤖 Generated with Claude Code

---

Summary by cubic

Restores the guided post-connect screen during first-run onboarding (BB-112) by wiring showPostConnectGuide to the real onboarding state. Old behavior: the prop was hardcoded to false so new users always saw the generic playground; new behavior: when state.onboarding.isGuidedPostConnect is true, PlaygroundMain renders the guide, with no changes to other flows.

• Review notes
  • One-line change in PlaygroundCenter.tsx: pass state.onboarding.isGuidedPostConnect to PlaygroundMain.
  • Adds PlaygroundCenter.test.tsx to cover guided vs non-guided cases; fails pre-fix, passes post-fix.
  • This is the only production call site that wires onboarding into PlaygroundMain; the Evals live-chat panel intentionally omits the prop.
  • No migration or config changes; impact is limited to the guided onboarding phase.

^{Written for commit 8bbbbf3. Summary will update on new commits.}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[chelojimenez]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bc58e434-449a-4c5c-b602-a9b5f3169951

📥 Commits

Reviewing files that changed from the base of the PR and between 48b9d15 and 8bbbbf3.

📒 Files selected for processing (2)

• mcpjam-inspector/client/src/components/playground/PlaygroundCenter.tsx
• mcpjam-inspector/client/src/components/playground/__tests__/PlaygroundCenter.test.tsx

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.

---

Walkthrough

PlaygroundCenter now passes the current onboarding guided-phase state to PlaygroundMain through showPostConnectGuide. Tests mock the playground components and verify guide visibility when onboarding is guided and when it is not.

Merge Risk: ⚪ Minimal · up to 8bbbb

This change restores the intended post-connect onboarding screen by using the real onboarding state, with no actionable merge-blocking risk remaining after normal checks and review.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

No issues found across 2 files

_{You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

_{Re-trigger cubic}

[github-actions]

Internal preview

Preview URL: https://mcp-inspector-pr-4055.up.railway.app
Deployed commit: 11c23a3
PR head commit: 8bbbbf3
Backend target: staging fallback.
Health: ✅ Convex reachable
Access is employee-only in non-production environments.

[ZeHuari]

Findigs

I traced the history of the changed line and verified the test locally (RED on main: 1 failed / 1 passed; GREEN with the fix; 359/359 passing across playground/, ui-playground/ and the onboarding hooks). My concern is that the diagnosis in the description doesn't hold up, and the one-liner changes the first-run design rather than repairing a broken wire.

tl;dr: the hardcoded false was deliberate, and flipping it to true satisfies half of BB-112 while regressing the other half — the MCPJam logo stops rendering.

[P1] The fix removes the logo that BB-112 asks to restore

PlaygroundMain.tsx:4308-4331 is a mutually exclusive ternary:

showPostConnectGuide === true → only , which is a line of copy plus a bouncing arrow. No logo, no hero, no starter prompts.
showPostConnectGuide === false (current behavior) → the hero with , "This is your playground for MCP.", and MultiModelStarterPromptsBlock.

BB-112 asks for the logo and the "try Excalidraw" prompt to be visible. Today the logo is rendered on first run; after this change it never is. So the description ("the logo + prompt were getting lost") doesn't match the code — false produces a stable state that always includes the logo. If the goal is both, the change belongs inside PlaygroundMain (render the logo within the guide branch), not at the PlaygroundCenter call site.

[P2] The false was intentional, not a missed wire

showPostConnectGuide={onboarding.isGuidedPostConnect} existed in #1689 and was deliberately changed to false in #1716 ("Remove NUX modal, auto-connect Excalidraw on first run"), in the same hunk that introduced the current NUX (firstRunComposerSeed, initialInputTypewriter, blockSubmitUntilServerConnected, pulseSubmit). #2121 only moved the code from AppBuilderTab to PlaygroundCenter — it inherited the false rather than dropping the wiring. So this PR revives the old NUX and switches off the current one; that's a product decision worth making explicitly.

[P3] It silently disables the typewriter and the send hint

In use-composer-onboarding.ts, every typewriter path is gated on !showPostConnectGuide (L45, L64, L76, L92, L97), and the effect at L123-128 seeds the full prompt in one shot. PlaygroundMain.tsx:4386 also hides HandDrawnSendHint when the prop is true.

There's no grace window either: while the phase is connecting_excalidraw, loadingState is skeleton (use-playground-state.ts:958-1010), so PlaygroundMain mounts already in connected_guided — the prop is true on the very first render and the typewriter never types a character. Net effect for a new user: "the prompt types itself + a hand-drawn arrow pointing at Send" becomes "the prompt appears instantly, no hint".

Also worth noting: firstRunComposerSeed stays true, so initialInputTypewriter, pulseSubmit and blockSubmitUntilServerConnected are still passed but now partly or fully inert — we'd end up with two half-enabled NUX flows.

[P4] It hides the model selector, host selector and "+" menu for the whole first run

minimalMode: showPostConnectGuide (PlaygroundMain.tsx:4199) disables the "+" menu, the ClientSelector and the ModelSelector in the composer (chat-input.tsx:1543, 1840, 1849, 2077), and PlaygroundMain.tsx:4590 hides the entire PlaygroundCenterHeaderBar. A new user can't choose a model or host until they've sent their first message. That was the #1689 design, so it may be intended — but it shouldn't ride along as a side effect of a one-line change.

[P5] The new test is tautological — it can't catch any of the above

PlaygroundCenter.test.tsx mocks PlaygroundMain and asserts the prop arrives, which restates the implementation line rather than the behavior. Nothing user-visible is checked (is "Try asking Excalidraw to draw something." on screen? is the logo? does the typewriter run?), and because PlaygroundMain is mocked, P1, P3 and P4 are invisible to it by construction. The true branch is already covered in PlaygroundMain.test.tsx:1021-1035. The test that would add value is an integration test of PlaygroundCenter + real PlaygroundMain asserting what the first-run screen actually contains.

Minor: buildState() returns an untyped object, so if the usePlaygroundState contract changes the test stays green against a state shape that no longer exists. A satisfies Partial<ReturnType> would pin it.

[P6] A likelier root cause for the reported symptom, untouched here

The NUX is marked as seen the moment it renders: shouldMarkFirstRunNuxShown → markOnboardingShown() (use-playground-state.ts:988-1001), which writes status: "seen" with shownAt; getInitialLocalPhase then returns "dismissed" for that state (use-onboarding.ts:66). So a refresh before sending the first message kills the NUX permanently. Same if the user signs in with WorkOS mid-session (use-onboarding.ts:167 forces phase = "completed").

If the reported symptom is "the logo and prompt show up and then get lost", that early marking fits far better than the hardcoded false, which yields a stable state rather than an intermittent one. Could you share the exact repro steps from the ticket?

[P7] Nit

state.onboarding.isGuidedPostConnect now appears on two consecutive lines — worth hoisting const isGuidedPostConnect = state.onboarding.isGuidedPostConnect above the return.

[ZeHuari]

Review

I traced the history of the changed line and verified the test locally (RED on main: 1 failed / 1 passed; GREEN with the fix; 359/359 passing across playground/, ui-playground/ and the onboarding hooks). My concern is that the diagnosis in the description doesn't hold up, and the one-liner changes the first-run design rather than repairing a broken wire.

tl;dr: the hardcoded false was deliberate, and flipping it to true satisfies half of BB-112 while regressing the other half — the MCPJam logo stops rendering.

[P1] The fix removes the logo that BB-112 asks to restore

PlaygroundMain.tsx:4308-4331 is a mutually exclusive ternary:

• showPostConnectGuide === true → only <PostConnectGuide />, which is a line of copy plus a bouncing arrow. No logo, no hero, no starter prompts.
• showPostConnectGuide === false (current behavior) → the hero with <img src="/mcp_jam_dark.png">, "This is your playground for MCP.", and MultiModelStarterPromptsBlock.

BB-112 asks for the logo and the "try Excalidraw" prompt to be visible. Today the logo is rendered on first run; after this change it never is. So the description ("the logo + prompt were getting lost") doesn't match the code — false produces a stable state that always includes the logo. If the goal is both, the change belongs inside PlaygroundMain (render the logo within the guide branch), not at the PlaygroundCenter call site.

[P2] The false was intentional, not a missed wire

showPostConnectGuide={onboarding.isGuidedPostConnect} existed in #1689 and was deliberately changed to false in #1716 ("Remove NUX modal, auto-connect Excalidraw on first run"), in the same hunk that introduced the current NUX (firstRunComposerSeed, initialInputTypewriter, blockSubmitUntilServerConnected, pulseSubmit). #2121 only moved the code from AppBuilderTab to PlaygroundCenter — it inherited the false rather than dropping the wiring. So this PR revives the old NUX and switches off the current one; that's a product decision worth making explicitly.

[P3] It silently disables the typewriter and the send hint

In use-composer-onboarding.ts, every typewriter path is gated on !showPostConnectGuide (L45, L64, L76, L92, L97), and the effect at L123-128 seeds the full prompt in one shot. PlaygroundMain.tsx:4386 also hides HandDrawnSendHint when the prop is true.

There's no grace window either: while the phase is connecting_excalidraw, loadingState is skeleton (use-playground-state.ts:958-1010), so PlaygroundMain mounts already in connected_guided — the prop is true on the very first render and the typewriter never types a character. Net effect for a new user: "the prompt types itself + a hand-drawn arrow pointing at Send" becomes "the prompt appears instantly, no hint".

Also worth noting: firstRunComposerSeed stays true, so initialInputTypewriter, pulseSubmit and blockSubmitUntilServerConnected are still passed but now partly or fully inert — we'd end up with two half-enabled NUX flows.

[P4] It hides the model selector, host selector and "+" menu for the whole first run

minimalMode: showPostConnectGuide (PlaygroundMain.tsx:4199) disables the "+" menu, the ClientSelector and the ModelSelector in the composer (chat-input.tsx:1543, 1840, 1849, 2077), and PlaygroundMain.tsx:4590 hides the entire PlaygroundCenterHeaderBar. A new user can't choose a model or host until they've sent their first message. That was the #1689 design, so it may be intended — but it shouldn't ride along as a side effect of a one-line change.

[P5] The new test is tautological — it can't catch any of the above

PlaygroundCenter.test.tsx mocks PlaygroundMain and asserts the prop arrives, which restates the implementation line rather than the behavior. Nothing user-visible is checked (is "Try asking Excalidraw to draw something." on screen? is the logo? does the typewriter run?), and because PlaygroundMain is mocked, P1, P3 and P4 are invisible to it by construction. The true branch is already covered in PlaygroundMain.test.tsx:1021-1035. The test that would add value is an integration test of PlaygroundCenter + real PlaygroundMain asserting what the first-run screen actually contains.

Minor: buildState() returns an untyped object, so if the usePlaygroundState contract changes the test stays green against a state shape that no longer exists. A satisfies Partial<ReturnType<typeof usePlaygroundState>> would pin it.

[P6] A likelier root cause for the reported symptom, untouched here

The NUX is marked as seen the moment it renders: shouldMarkFirstRunNuxShown → markOnboardingShown() (use-playground-state.ts:988-1001), which writes status: "seen" with shownAt; getInitialLocalPhase then returns "dismissed" for that state (use-onboarding.ts:66). So a refresh before sending the first message kills the NUX permanently. Same if the user signs in with WorkOS mid-session (use-onboarding.ts:167 forces phase = "completed").

If the reported symptom is "the logo and prompt show up and then get lost", that early marking fits far better than the hardcoded false, which yields a stable state rather than an intermittent one. Could you share the exact repro steps from the ticket?

[P7] Nit

state.onboarding.isGuidedPostConnect now appears on two consecutive lines — worth hoisting const isGuidedPostConnect = state.onboarding.isGuidedPostConnect above the return.

---

Suggestion: let's confirm the repro and get an explicit design call on what first-run should show. If the answer is "logo + Excalidraw prompt" (what the ticket literally asks for), the change should keep the hero and add the guide copy to it inside PlaygroundMain — at which point this call-site flip is either unnecessary or needs to come with cleanup of the now-inert first-run props.

[ZeHuari]

Follow-up: second triage pass

I re-verified each point against the code instead of just describing it. Four new facts, one severity downgrade, and four things I want to explicitly clear so you don't chase ghosts.

New: there is no test coverage of this screen at all — in either direction

• e2e/nux.spec.ts exists and covers the first-run flow, but only the redirect to /playground. It makes zero assertions about what the screen contains.
• The one assertion that does mention the hero (PlaygroundMain.test.tsx:999, "This is your playground for MCP.") renders with default props, i.e. showPostConnectGuide = false. It stays green even though the change removes that hero in production.

So the 359/359 green run isn't evidence either way, and neither is the e2e suite. This is the concrete reason P5 matters: nothing in CI can tell us whether the first-run screen is right before or after this change.

New: the typewriter has a second, worse failure mode (refines P3)

I said the typewriter never runs. That's true on the happy path, but there's a degraded path worth knowing about. If the first-run skeleton hits its 12 s timeout (FIRST_RUN_SKELETON_TIMEOUT_MS, use-playground-state.ts:82) while Excalidraw is still connecting, PlaygroundMain mounts with the flag still false — the typewriter starts typing — and then the flag flips to true once the connection lands, and the effect at use-composer-onboarding.ts:123-128 snaps the input to the full prompt mid-word.

Net: happy path = no typewriter at all; slow-connection path = typewriter that visibly jumps to the end. Neither is the designed behavior.

Downgraded: P4's blast radius is one session, not indefinite

I traced the full lifecycle: the NUX is marked as seen the moment it paints (shouldMarkFirstRunNuxShown → markOnboardingShown()), and getInitialLocalPhase returns "dismissed" for status: "seen" with a shownAt (use-onboarding.ts:66) — which the existing test "does not resume guided mode after the NUX was shown" already pins. So minimal mode (no model selector, no host selector, no header bar) lasts a single session and ends at the first refresh. Still a product call, but the exposure window is much smaller than I first implied.

Sharpened: why P6 should gate this PR, not just accompany it

This is the argument I'd want answered before merge: the hardcoded false produces a stable state — the hero renders every time, deterministically. The ticket describes something intermittent ("disappearing or getting lost"). A deterministic state cannot produce an intermittent symptom.

The early "mark as seen" behavior can: paint the NUX, refresh before sending the first message, and it's gone forever. Same if the user signs into WorkOS mid-flow (use-onboarding.ts:167 forces phase = "completed"). Could you share the repro steps from BB-112? If the reporter's screen recording shows the hero appearing and then vanishing, this PR is fixing the wrong site.

Cleared — not issues, don't spend time on them

Things I checked and want to explicitly rule out:

• ErrorBox is preserved in the guide branch (PlaygroundMain.tsx:4310-4324). No error-handling regression.
• The evals live-chat panel is unaffected — it passes hideWelcomeHero + hideCenterHeaderChrome and never this prop. Your claim about the second call site is correct.
• No compare/multi-model conflict: showMultiModelTraceEmptyPanel is already gated on !showPostConnectGuide (PlaygroundMain.tsx:1911), so that interaction was anticipated, and a first-run user isn't in compare mode anyway.
• Prettier on the new test file is noise — neighboring files fail the same root-config check and deploy-staging runs prettier-fix. Ignore it.

Where this leaves the review

P1 and P6 are the two blockers, and neither is a coding problem: what should first-run show (the ticket says logo and prompt, which is the one thing neither branch does today), and what are the repro steps. P3 and P4 become recorded design decisions once the first question is answered. P2 is a description rewrite, P5 is a test worth adding either way, P7 is a nit.