Repository navigation
Turn onboarding into a guided connection wizard - #1186
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe onboarding page now uses a three-step wizard with URL hash navigation and connection-aware progression. Cursor and VS Code instructions provide installation deep links. Unit and E2E tests validate generated links, connection states, reload behavior, and starter-package interactions. ChangesOnboarding flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant onboarding.tsx
participant MCPConnectionState
participant onboarding-mcp-client-tabs.tsx
Browser->>onboarding.tsx: Open onboarding and select a step
onboarding.tsx->>MCPConnectionState: Read MCP connection status
MCPConnectionState-->>onboarding.tsx: Return connection state
onboarding.tsx->>Browser: Render discovery or connection panel
Browser->>onboarding-mcp-client-tabs.tsx: Request Cursor or VS Code installation action
onboarding-mcp-client-tabs.tsx->>onboarding-mcp-clients.ts: Build encoded MCP install URL
onboarding-mcp-clients.ts-->>onboarding-mcp-client-tabs.tsx: Return deep link
onboarding-mcp-client-tabs.tsx-->>Browser: Render the installation button
MCPConnectionState-->>onboarding.tsx: Report connected agent
onboarding.tsx->>Browser: Show completed connection and starter-package step
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-1186.kody-a99.workers.dev Worker: Mocks:
|
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d0932f5. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
e2e/community-featured.spec.ts (1)
96-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the VS Code link prefix before slicing it.
The test slices a fixed number of characters from
vsCodeInstallHrefwithout verifying that the prefix matches. If the builder emits a different prefix, for examplevscode://mcp/install?, the slice offset is wrong andJSON.parsethrows a parse error that hides the real cause. The Cursor branch assertsprotocolexplicitly at line 77; make this branch symmetric.💚 Proposed test change
expect(vsCodeInstallHref).toBeTruthy() + const vsCodePrefix = 'vscode:mcp/install?' + expect(vsCodeInstallHref!.startsWith(vsCodePrefix)).toBe(true) expect( JSON.parse( decodeURIComponent( - vsCodeInstallHref!.slice('vscode:mcp/install?'.length), + vsCodeInstallHref!.slice(vsCodePrefix.length), ), ), ).toEqual({ name: 'kody', type: 'http', url: expectedMcpUrl })🤖 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 `@e2e/community-featured.spec.ts` around lines 96 - 102, In the VS Code assertion near the JSON.parse call, first validate that vsCodeInstallHref starts with the expected “vscode:mcp/install?” prefix, matching the explicit protocol assertion in the Cursor branch. Keep the existing slicing, decoding, parsing, and payload equality checks unchanged after that assertion.packages/worker/client/routes/onboarding.tsx (1)
300-303: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the final step number from
onboardingSteps.Line 303 hardcodes
3, andWizardNavigationhardcodes3again at line 504.onboardingStepsis already the source of truth for the step list. If a step is added or removed, these two literals become wrong silently.Consider a module-level constant, for example
const lastOnboardingStep = onboardingSteps.at(-1)!.number, and use it in both places.🤖 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 `@packages/worker/client/routes/onboarding.tsx` around lines 300 - 303, Derive the final onboarding step number from the existing onboardingSteps definition instead of hardcoding 3. Add a shared module-level last-step value based on the final step’s number, then use it for isComplete in the onboarding step rendering and for the corresponding WizardNavigation argument.
🤖 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 `@e2e/community-featured.spec.ts`:
- Around line 108-130: Define the onboarding route predicate once and reuse that
same function reference in both page.route and page.unroute, ensuring the
/onboarding.json interception is removed after the assertions.
In `@packages/worker/client/routes/onboarding.tsx`:
- Around line 319-323: Update the completion indicator in the isComplete branch
to use an accessible element, such as adding an appropriate ARIA role to the
existing span with aria-label="Complete" or providing visually hidden text,
while preserving the visible checkmark and the existing getByLabel('Complete')
behavior.
- Around line 305-324: Update the step indicator button styling associated with
stepIndicatorButtonCss to include a &:focus-visible rule that reuses
focusRingCss. Import focusRingCss from `#client/styles/style-primitives.ts`,
matching the existing focus treatment used by other onboarding controls.
---
Nitpick comments:
In `@e2e/community-featured.spec.ts`:
- Around line 96-102: In the VS Code assertion near the JSON.parse call, first
validate that vsCodeInstallHref starts with the expected “vscode:mcp/install?”
prefix, matching the explicit protocol assertion in the Cursor branch. Keep the
existing slicing, decoding, parsing, and payload equality checks unchanged after
that assertion.
In `@packages/worker/client/routes/onboarding.tsx`:
- Around line 300-303: Derive the final onboarding step number from the existing
onboardingSteps definition instead of hardcoding 3. Add a shared module-level
last-step value based on the final step’s number, then use it for isComplete in
the onboarding step rendering and for the corresponding WizardNavigation
argument.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ab9e7e3-8b5c-4dce-acaf-69084f7ccb29
📒 Files selected for processing (6)
e2e/community-featured.spec.tse2e/invite-signup-verification.spec.tspackages/worker/client/routes/onboarding-mcp-client-tabs.tsxpackages/worker/client/routes/onboarding-mcp-clients.node.test.tspackages/worker/client/routes/onboarding-mcp-clients.tspackages/worker/client/routes/onboarding.tsx
| async function fulfillConnectedStatus(route: Route) { | ||
| const response = await route.fetch() | ||
| const payload = (await response.json()) as Record<string, unknown> | ||
| await route.fulfill({ | ||
| response, | ||
| json: { ...payload, hasMcpClient: true }, | ||
| }) | ||
| } | ||
| await page.route( | ||
| (url) => url.pathname === '/onboarding.json', | ||
| fulfillConnectedStatus, | ||
| ) | ||
| await expect(page.getByTestId('onboarding-starter-packages')).toBeVisible({ | ||
| timeout: 7_000, | ||
| }) | ||
| await expect( | ||
| page.getByRole('heading', { name: 'You are connected' }), | ||
| ).toBeVisible() | ||
| await expect(page.getByLabel('Complete')).toHaveCount(2) | ||
| await page.unroute( | ||
| (url) => url.pathname === '/onboarding.json', | ||
| fulfillConnectedStatus, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Playwright page.unroute function url matcher reference equality route.fulfill response json override
💡 Result:
The Playwright page.unroute function and route.fulfill response handling operate as follows: page.unroute URL Matcher and Reference Equality To successfully remove a route using page.unroute(url, handler), you must provide the same URL pattern and, if specified, the same handler function reference used when the route was originally registered [1][2][3]. 1. URL Matching: If you registered a route using a RegExp, passing a new RegExp object with the same pattern (e.g., /abc/!== /abc/) will not match, as RegExp equality in JavaScript checks for identity rather than equivalent content [4]. You must pass the exact same RegExp instance used during registration [4][5]. If using a string or glob, Playwright uses these to match the registered route [1]. 2. Handler Equality: If you provided a specific handler function to route, you must pass that exact function reference to unroute to remove it [1][6]. If you omit the handler argument, Playwright will remove all handlers associated with the provided URL pattern [1][5]. route.fulfill Response JSON Override The route.fulfill method is used to respond to intercepted requests [7]. To override or patch a JSON response, the recommended approach is to fetch the original response first and then fulfill it with the modified data [8][9]. 1. Fetch Original: Inside your route handler, call await route.fetch to get the actual server response [10][11]. 2. Parse and Modify: Use await response.json to parse the body, modify the desired fields, and maintain the rest of the original payload [8][9]. 3. Fulfill with Override: Pass the original response object into fulfill to preserve the status code and headers, and use the json option to provide your updated body [8][9]. Example: await page.route('/api/data', async (route) => { const response = await route.fetch; const json = await response.json; json.modifiedField = 'newValue'; // Patching data await route.fulfill({ response, // Preserves original status and headers json // Overrides body }); }); Using the json option in fulfill automatically stringifies the object and sets the Content-Type header to application/json, making it a convenient shortcut over setting the body and headers manually [9].
Citations:
- 1: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/page.ts
- 2: https://playwright.dev/docs/api/class-page
- 3: https://runebook.dev/en/docs/playwright/api/class-page/page-unroute
- 4: [Question] Removing routes from page._routes microsoft/playwright#1607
- 5: https://github.com/microsoft/playwright/blob/d3d436d3/tests/page/page-route.spec.ts
- 6: https://runebook.dev/en/docs/playwright/api/class-browsercontext/browser-context-unroute
- 7: https://microsoft-playwright.mintlify.app/api/route
- 8: https://scrolltest.com/playwright-modify-api-response-route-fulfill/
- 9: https://qaskills.sh/blog/playwright-route-fulfill-mock-api-guide
- 10: https://playwright.dev/docs/mock
- 11: https://playwright.dev/docs/network
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'community-featured\.spec\.ts$' . | sed 's#^\./##'
echo "== relevant lines =="
file="$(fd 'community-featured\.spec\.ts$' . | head -n1 || true)"
if [ -n "${file:-}" ]; then
sed -n '90,140p' "$file" | nl -ba -v90
sed -n '180,235p' "$file" | nl -ba -v180
fi
echo "== package playwright versions =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
found=$(fd -a "^$f$" . | head -n1)
if [ -n "${found:-}" ]; then
echo "--- $found ---"
case "$found" in
package.json) jq -r '.devDependencies["`@playwright/test`"] // .dependencies["`@playwright/test`"] // empty' "$found";;
*) rg -n '"(`@playwright/test`|playwright)"|packages/playwright-core|playwright-core' "$found" | head -n 12 || true;;
esac
fi
doneRepository: kentcdodds/kody
Length of output: 283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd 'community-featured\.spec\.ts$' .
echo "== relevant lines 90-140 =="
cat -n /home/jailuser/git/e2e/community-featured.spec.ts | sed -n '90,140p'
echo "== relevant lines 180-235 =="
cat -n /home/jailuser/git/e2e/community-featured.spec.ts | sed -n '180,235p'
echo "== package files =="
find /home/jailuser/git -maxdepth 2 -type f \( -name 'package.json' -o -name 'pnpm-lock.yaml' -o -name 'package-lock.json' -o -name 'yarn.lock' \) -print | sort
echo "== package versions =="
if [ -f /home/jailuser/git/package.json ]; then
awk '/["@]?playwright(\/test)?"|playwright-core|--playwright-core/ { print }' /home/jailuser/git/package.json
fi
echo "== node availability =="
command -v node || true
node -v 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 5056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo root files =="
git ls-files | sed -n '1,80p'
echo "== git status/stat =="
git status --short
git diff --stat HEAD || true
echo "== JS comparison of route matchers =="
node - <<'JS'
const handler = async () => {};
const route1 = { type: 'function', urlMatcher: (url) => url.pathname === '/onboarding.json', handler };
const route2 = { type: 'function', urlMatcher: (url) => url.pathname === '/onboarding.json', handler };
console.log('same_urlMatcher_instance:', route1.urlMatcher === route2.urlMatcher);
console.log('same_handler_instance:', route1.handler === route2.handler);
JS
echo "== installed Playwright page unroute source =="
pkg="$(npm ls `@playwright/test` --json --long 2>/dev/null | jq -r '.[0].version // empty' | head -n1 || true)"
echo "playwright_test_version= $pkg"
node_modules="$(pwd)/node_modules"
if [ -d "$node_modules/@playwright/test" ]; then
rg -n "class Page|unroute|_routes|findRoute|new URLMatcher|source" "$node_modules/@playwright/test" | head -n 80 || true
rg -n "function unroute|async unroute|unroute\\(" "$node_modules@/playwright/test" 2>/dev/null | head -n 80 || true
fi
node_modules_core="$(pwd)/node/node_modules/@playwright/test/node_modules/playwright-core"
if [ -d "$node_modules_core" ]; then
rg -n "function unroute|unroute\\(" "$node_modules_core" | head -n 80 || true
fi
node_modules_core2="$(pwd)/node_modules/@playwright/test/node_modules/playwright-core"
if [ -d "$node_modules_core2" ]; then
rg -n "function unroute|unroute\\(" "$node_modules_core2" | head -n 80 || true
fiRepository: kentcdodds/kody
Length of output: 4542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== install `@playwright/test` in temp =="
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm init -y >/dev/null
npm install --save-dev `@playwright/test`@1.58.2 >/dev/null 2>&1 || true
echo "== locate source file =="
find "$tmpdir/node_modules/@playwright/test" -path '*page*' -name '*.js' -print | sed -n '1,50p'
echo "== unroute implementation snippets =="
rg -n "class Page|unroute\\(|_routes|_createRoute|_routesByPredicate|_matchRoute|_handleRoute" "$tmpdir/node_modules/@playwright/test" "$tmpdir/node_modules/playwright-core" 2>/dev/null | sed -n '1,160p'
node - <<'JS'
const { Page } = require(require.resolve('playwright-core', { paths: [process.cwd()] }));
const { route: routeAPI } = require(require.resolve('`@playwright/test`', { paths: [process.cwd()] }));
const page = Page.createForTest({ browserName: 'chromium' });
let matchedInRoute = false;
page.route(
(url) => url.pathname === '/onboarding.json',
async (route) => {
matchedInRoute = true;
await route.continue();
},
);
let matchedInUnroute = false;
page.unroute(
(url) => url.pathname === '/onboarding.json',
async (route) => {
matchedInUnroute = true;
await route.continue();
},
);
console.log('route_handler_matched:', matchedInRoute);
console.log('unroute_handler_matched:', matchedInUnroute);
JSRepository: kentcdodds/kody
Length of output: 7463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm init -y >/dev/null
npm install --save-dev `@playwright/test`@1.58.2 >/dev/null 2>&1
echo "== inspect Page.unroute internal logic =="
awk '
/^class Page extends/,/^}$/{
p=1
}
p && /^[}]$/ { p=0 }
p { print NR": "$0 }
' "$tmpdir/node_modules/playwright-core/lib/client/page.js" | sed -n '110,180p'
echo "== RouteHandler prepareInterceptionPatterns =="
awk '
/^class RouteHandler extends/,/^}$/{
p=1
}
p && /^[}]$/ { p=0 }
p { print NR": "$0 }
/prepareInterceptionPatterns/ { print "FOUND prepareInterceptionPatterns line" }
' "$tmpdir/node_modules/playwright-core/lib/client/network.js" | sed -n '230,290p'
echo "== read-only semantic probe from installed source =="
node - <<'JS'
const path = require('path');
const { Page } = require(path.resolve(__dirname, 'node_modules/playwright-core/lib/client/page'));
const { _unrouteInternal, _setupNewPageBinding: _setupNewPageBindingFn } = require(path.resolve(__dirname, 'node_modules/playwright-core/lib/client/page').ChannelOwner);
const PageClass = Page.__class?.prototype || Page.prototype;
const { RouteHandler } = require(path.resolve(__dirname, 'node_modules/playwright-core/lib/client/network'));
const routeMatcher1 = (url) => url.pathname === '/onboarding.json';
const handler = async () => {};
const p = new Page('x');
p.route(routeMatcher1, handler);
console.log('registered_route_count:', p._routes.length);
const routeMatcher2 = (url) => url.pathname === '/onboarding.json';
const remaining = [];
for (const route of p._routes) {
const result = routeMatcher2(new URL('http://localhost/onboarding.json'));
if (result && route._handler === handler) {
console.log('unroute_matches_by_same_anonymous_function:', false);
continue;
}
remaining.push(route);
}
console.log('remaining_after_new_anonymous_function:', remaining.length);
p.route(routeMatcher1, handler);
p.unroute(routeMatcher1, handler);
console.log('remaining_after_same_anonymous_function:', p._routes.length);
console.log('source_routeMatcher1:', routeMatcher1.toString());
console.log('source_routeMatcher2:', routeMatcher2.toString());
JS
echo "== inspect handler-only unroute branch via source =="
awk '
/^ async unroute\(url, handler\)/,/ return remaining;$/{
p=1
}
p { print NR": "$0 }
/return remaining;/ && p { p=0 }
' "$tmpdir/node_modules/playwright-core/lib/client/page.js" | sed -n '1,80p'
echo "== direct installed source unroute implementation =="
awk '
/^ async unroute\(url, handler\)$/{
p=1
}
p{print NR": "$0}
/^ this._routes = remaining;$/ && p { p=0 }
' "$tmpdir/node_modules/playwright-core/lib/client/page.js" | sed -n '430,460p'Repository: kentcdodds/kody
Length of output: 3865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm init -y >/dev/null
npm install --save-dev `@playwright/test`@1.58.2 >/dev/null 2>&1
echo "== page file around Page constructor/routes =="
awk '
/^class Page extends/ { p=1 }
p { print NR": "$0 }
/^}$/ { if (p) { p=0; print "**"; } }
/page\.unroute|unroute\(/ { print "--- unroute line ---"; print NR": "$0 }
' "$tmpdir/node_modules/playwright-core/lib/client/page.js" | sed -n '52,170p'
echo "== context unroute implementation =="
awk '
/^class BrowserContext extends/ { p=1 }
p { print NR": "$0 }
/^}$/ { if (p) { p=0; print "**"; } }
/^ async unroute\(url, handler\)/ { x=1 }
x { print NR": "$0 }
/return remaining;/ && x { x=0 }
' "$tmpdir/node_modules/playwright-core/lib/client/browserContext.js" | sed -n '360,385p'
echo "== route handler source =="
awk '
/^class RouteHandler extends/ { p=1 }
p { print NR": "$0 }
/^ async prepareInterceptionPatterns/ { x=1 }
x { print NR": "$0 }
/\};$/ && x { x=0 }
' "$tmpdir/node_modules/playwright-core/lib/client/network.js" | sed -n '380,430p'Repository: kentcdodds/kody
Length of output: 7093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm init -y >/dev/null
npm install --save-dev `@playwright/test`@1.58.2 >/dev/null 2>&1
echo "== exact unroute source =="
rg -n -C 10 "async unroute\\(url, handler\\)" "$tmpdir/node_modules/playwright-core/lib/client/page.js" "$tmpdir/node_modules/playwright-core/lib/client/browserContext.js"
echo "== exact routeHandler methods =="
rg -n -C 8 "class RouteHandler|prepareInterceptionPatterns|matches|_urlMatcher|_url" "$tmpdir/node_modules/playwright-core/lib/client/network.js" "$tmpdir/node_modules/playwright-core/lib/client/page.js"Repository: kentcdodds/kody
Length of output: 31123
🌐 Web query:
Playwright urlMatchesEqual function source urlMatchesEqual string RegExp function same reference behavior
💡 Result:
The function urlMatchesEqual is an internal utility within the Playwright codebase, located in the packages/isomorphic/urlMatch.ts module [1]. It is primarily used to compare URL matchers (such as those used in routing or unrouting requests) to determine if they represent the same rule [2][3][4]. Behavior of urlMatchesEqual: The function performs a direct comparison between two matchers [1]. When both inputs are Regular Expressions (RegExp), it checks if they have the same source property (the pattern string) and the same flags [1]. Otherwise, it performs a strict equality check (===) [1]. Key points regarding Playwright URL matching: - Glob and Pattern Matching: For general URL matching (e.g., in page.route or page.waitForURL), Playwright supports strings (which can be interpreted as glob patterns), RegExp objects, URLPattern objects, and predicate functions [5][6][7][8]. - Glob Patterns: These are simplified patterns using wildcards like * (matches non-separator characters) and (matches any character including separators) [5][9]. - RegExp: To use a Regular Expression for URL matching, you must provide a JavaScript RegExp object (e.g., /pattern/) rather than a string representation of one, so that Playwright can correctly identify the type [10]. - Internal Usage: While urlMatchesEqual is part of the Playwright core, it is not intended for public use as an API. Developers should use the standard URL matching capabilities exposed through methods like page.waitForURL or page.route [5][8][11].
Citations:
- 1: https://github.com/microsoft/playwright/blob/d3d436d3/packages/isomorphic/urlMatch.ts
- 2: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/browserContext.ts
- 3: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/page.ts
- 4: https://github.com/microsoft/playwright/blob/54e92be7/packages/playwright-core/src/client/browserContext.ts
- 5: https://playwright.dev/docs/network
- 6: https://playwright.dev/docs/api/class-pageassertions
- 7: https://github.com/microsoft/playwright/blob/main/docs/src/api/class-pageassertions.md
- 8: https://playwright.dev/docs/api/class-page
- 9: https://playwright.dev/docs/next/network
- 10: [Question] Regular Expression Syntax with waitForUrl microsoft/playwright#16430
- 11: https://github.com/microsoft/playwright/blob/main/docs/src/navigations.md
Reuse the route predicate when removing the route.
Lines 117 and 128 each create a separate arrow function, and Playwright’s page.unroute(url, handler) compares function predicates with ===. The second call does not remove the registration from e2e/community-featured.spec.ts, so the /onboarding.json interception remains active for later steps in the same test.
💚 Proposed test change
+ const onboardingJsonMatcher = (url: URL) =>
+ url.pathname === '/onboarding.json'
async function fulfillConnectedStatus(route: Route) {
const response = await route.fetch()
const payload = (await response.json()) as Record<string, unknown>
await route.fulfill({
response,
json: { ...payload, hasMcpClient: true },
})
}
- await page.route(
- (url) => url.pathname === '/onboarding.json',
- fulfillConnectedStatus,
- )
+ await page.route(onboardingJsonMatcher, fulfillConnectedStatus)
await expect(page.getByTestId('onboarding-starter-packages')).toBeVisible({
timeout: 7_000,
})
await expect(
page.getByRole('heading', { name: 'You are connected' }),
).toBeVisible()
await expect(page.getByLabel('Complete')).toHaveCount(2)
- await page.unroute(
- (url) => url.pathname === '/onboarding.json',
- fulfillConnectedStatus,
- )
+ await page.unroute(onboardingJsonMatcher, fulfillConnectedStatus)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function fulfillConnectedStatus(route: Route) { | |
| const response = await route.fetch() | |
| const payload = (await response.json()) as Record<string, unknown> | |
| await route.fulfill({ | |
| response, | |
| json: { ...payload, hasMcpClient: true }, | |
| }) | |
| } | |
| await page.route( | |
| (url) => url.pathname === '/onboarding.json', | |
| fulfillConnectedStatus, | |
| ) | |
| await expect(page.getByTestId('onboarding-starter-packages')).toBeVisible({ | |
| timeout: 7_000, | |
| }) | |
| await expect( | |
| page.getByRole('heading', { name: 'You are connected' }), | |
| ).toBeVisible() | |
| await expect(page.getByLabel('Complete')).toHaveCount(2) | |
| await page.unroute( | |
| (url) => url.pathname === '/onboarding.json', | |
| fulfillConnectedStatus, | |
| ) | |
| const onboardingJsonMatcher = (url: URL) => | |
| url.pathname === '/onboarding.json' | |
| async function fulfillConnectedStatus(route: Route) { | |
| const response = await route.fetch() | |
| const payload = (await response.json()) as Record<string, unknown> | |
| await route.fulfill({ | |
| response, | |
| json: { ...payload, hasMcpClient: true }, | |
| }) | |
| } | |
| await page.route(onboardingJsonMatcher, fulfillConnectedStatus) | |
| await expect(page.getByTestId('onboarding-starter-packages')).toBeVisible({ | |
| timeout: 7_000, | |
| }) | |
| await expect( | |
| page.getByRole('heading', { name: 'You are connected' }), | |
| ).toBeVisible() | |
| await expect(page.getByLabel('Complete')).toHaveCount(2) | |
| await page.unroute(onboardingJsonMatcher, fulfillConnectedStatus) |
🤖 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 `@e2e/community-featured.spec.ts` around lines 108 - 130, Define the onboarding
route predicate once and reuse that same function reference in both page.route
and page.unroute, ensuring the /onboarding.json interception is removed after
the assertions.
…ard-ui-9c9e # Conflicts: # e2e/community-featured.spec.ts # e2e/invite-signup-verification.spec.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
3 participants
Intent
Make first-run onboarding action-oriented and prevent users from missing the MCP authorization step that completes agent connection.
Summary
/onboardinginto a three-step, directly navigable wizard with optional discovery, connection, and starter-package steps/onboarding#discovery, moves keyboard focus on user-driven transitions, auto-advances connected users to step 3, and keeps BYOK help available thereTesting
npm run validate✅npm run test:node -- packages/worker/client/routes/onboarding-mcp-clients.node.test.ts✅mainintentionally removed those low-signal suites in test: trim low-signal coverage, flake sources, and slow suites #1188; finalvalidatepassed against the post-test: trim low-signal coverage, flake sources, and slow suites #1188 suite ✅System changes
System recap — extends an existing primitive (medium risk)
Mode: recap · Base:
main@e0dedbb1· Head:09ebac26Classification: extends — changes the browser app's onboarding interaction and client-install contract without adding a new primitive.
Primitives touched
app-uiSystem map
The onboarding UI turns its existing payload-provided MCP URL into client-specific install links and observes the existing OAuth grant poll; the MCP endpoint itself is unchanged.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Change flow
Conductor report
Status: shipped — PR #1186 squash-merged as
6c350d2c; post-merge validation passed on retry after one unrelated social-login flake; production deploy passed health and smoke checks. Production/healthreports the merged SHA. Remains: nothing for Track A.Summary by CodeRabbit