Put the agent connection first: /connect as a page, and a first run that pairs it with the invite - #153
Conversation
…hat pairs it with the invite The setup that makes this product work from an agent lived at /settings/mcp, which is the last place a new account looks, and the invitation waited behind a first saved skill. Two changes, one intent. /connect is now a top level page, public and indexable, linked from the product nav, the account menu, the library header, and the command palette. /settings/mcp redirects to it permanently, in both spellings. The page keeps its own copy, its testids, and every event it already emitted; the team scoped copy events stay team scoped and simply report nothing for a reader who has not signed in. A team created during onboarding lands on /start instead of an empty library: connect your agent, add your first skill, and invite your team, side by side rather than in sequence. The connect step carries the real install commands and the real endpoint, and the invite step carries the real invitation form.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8f4f4cdbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <InviteMemberForm | ||
| className="" | ||
| idPrefix="onboarding-invite" | ||
| layout="stack" |
There was a problem hiding this comment.
Attribute successful invites to the onboarding surface
When an admin submits this form on /start, the reused InviteMemberForm posts only the email and role, and createInvitationLink consequently captures team_member_invited with only role and email_sent. Unlike the separately configured link-copy event, successful invitations therefore have no surface: "onboarding" property and cannot be distinguished from invitations sent through existing forms, making the new onboarding invitation metric described by this experiment unrecoverable. Thread the surface through the form and server action into team_member_invited.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 34fb450.
The surface now travels with the request, because the invitation is created on the server and a client-side event on the form can only describe the copied link. InviteMemberForm takes a required surface prop and posts it as a hidden field, createInvitationLink parses it with a bounded enum, and team_member_invited carries it alongside role and email_sent. The values are the same three team_invite_link_copied already uses (onboarding, first_skill_invite_step, organization_settings), so the emailed invitation and the copied link read against each other per surface. This step passes surface="onboarding"; the other two render sites name their own, and the prop is required so a fourth one cannot forget.
tests/agent-first-connect.test.mjs gains a test that walks the surface from the form through the action into the event definition.
| const hasSessionCookie = Boolean(getSessionCookie(await headers())) | ||
|
|
||
| if (hasSessionCookie) return <ProtectedAppShell>{children}</ProtectedAppShell> |
There was a problem hiding this comment.
Validate the session before selecting protected chrome
When a visitor has a present but expired, revoked, or otherwise invalid session cookie, this branch wraps the public /connect page in ProtectedAppShell; its AuthenticatedHeader calls getAppContext, whose session check redirects to sign-in. Thus precisely the stale-cookie case already handled elsewhere in proxy.ts cannot read this newly public acquisition page at all. Use validated session state for the shell choice, or make the protected chrome tolerate a failed session and fall back to ResourceShell.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 34fb450, by removing the branch rather than validating it.
The layout no longer chooses a shell. /connect is public and nothing else: app/connect/layout.tsx is a ResourceShell and nothing else, and no file under app/connect reads a session, a cookie, or a header. A stale or revoked cookie therefore cannot route a public acquisition page into ProtectedAppShell, and the same removal fixes the prerender failure this branch was failing the build on (Route /connect: Next.js encountered uncached or runtime data during prerendering) and the objection to a page being public and private at the same time.
The page is now synchronous and hands out absoluteUrl("/api/mcp") instead of deriving the host from the request, so every visitor copies the same instructions. lib/connect-viewer.ts is deleted. The personalized, team scoped setup stays on /start, behind the session, where the team is already known.
A structural test now reads every file under app/connect and fails on getSessionCookie, getSession, requireSession, getAppContext, auth(, cookies(), or headers(), so the branch cannot come back by accident.
…nvitation
The page shipped as two pages behind one URL: the layout read the session
cookie and chose between the product chrome and the public shell. That read is
per-request state on a route that has to prerender, which is what failed the
build ("uncached or runtime data during prerendering"), and it is also what let
a stale but present cookie hand a public acquisition page to a shell that
redirects to sign in.
Both problems, and the objection to a page that is public and private at once,
have the same answer: /connect is public and nothing else. The layout is a
ResourceShell and nothing else, the page reads no session, no cookie and no
header, and the endpoint it hands out is the canonical production one rather
than the request host, so every visitor copies the same instructions. This is
the rule the other acquisition pages already follow, spelled out in
resource-chrome: the invitation is the same for every reader, and a signed-in
one who takes it lands in their library regardless. lib/connect-viewer.ts, which
existed only to resolve the viewer's team on this page, is gone with it.
The MCP setup funnel follows the page out of the session. mcp_setup_viewed and
mcp_config_copied are no longer team scoped, because most views of a public page
have no team behind them and asking for one would put the session back in front
of the first byte. The client property keeps the surfaces apart: generic is the
endpoint copied from the first run on /start, the rest come from the guide. The
team scoped half of the first run stays where the team is known, in
onboarding_steps_viewed.
Separately, an invitation sent from /start now says so. The invitation is
created on the server, so a client-side event on the form can describe the
copied link but not the invitation that was actually sent; the surface travels
as a hidden field through createInvitationLink into team_member_invited, with
the same values team_invite_link_copied already uses, so the emailed invitation
and the copied link read against each other per surface.
The e2e guards move with the page: /connect is asserted in the public spec as a
shell with nothing deferred behind it, and the two account menu navigations
start from a page that still carries the account menu.
What changed, and whyThe build failure. Vercel failed with The restructure.
This is the rule the other acquisition pages already follow, and it is written down in This also answers the objection to the dual nature. A page that is public and private at the same time was the thing being asked about, and it is gone. There is one Analytics follow the page out of the session. Surface propagation (the P1). An invitation sent from Tests. Checks. Commit: 34fb450 |
Moving the MCP setup funnel out of the session took the team with it. That is right for /connect, which is public and prerendered and has no team to name, but /start is authenticated and knows exactly whose first run it is, so the copy on that screen stopped being readable per team for no reason. team_id is optional on mcp_setup_viewed and mcp_config_copied rather than required: the public page sends none, the first run sends the real one, and nothing under app/connect goes near a session or a cookie. Making it optional means an event can now have properties that are all optional, so the argument and field helpers ask which keys a caller has to fill in rather than whether there are any keys at all, and an all optional event stays callable with no properties.
|
Follow-up in 593f858: team attribution is back on the authenticated first-run surface, with |
Tommy's feedback: MCP setup should move out of settings, but /connect should stay private, not become a public indexable page. - Move app/connect into the (app) route group as app/(app)/connect, so it inherits the same session check, sign-in redirect, protected app shell, and noindex metadata as /start, /library, and /settings. - Drop /connect from the sitemap and public/llms.txt: it is no longer a public acquisition surface. - Restore personalization: the connect guide now reads getAppContext() and derives the MCP endpoint from the request host, same as /start and the old settings/mcp page, instead of a hardcoded production URL. The plugin install commands stay canonical since the plugin itself is not team scoped. - mcp_setup_viewed and mcp_config_copied now carry team_id from /connect as well as from /start, since both surfaces are authenticated. The event shapes are unchanged; team_id stays optional on the type. - Flip the structural test that forbade session reads under app/connect into one that asserts the page is authenticated, personalized, and out of the sitemap/llms.txt. - Update the instant-nav e2e specs: /connect moves from the public spec to the authenticated one, with mcp-content as its streamed marker. The permanent /settings/mcp -> /connect redirect is unchanged.
|
Reworked this PR per Tommy's feedback (2026-08-23, Italian): connect should be a private page only, not public, since the goal was to move MCP setup out of settings, not to make it a public marketing page. What changed on top of 593f858:
Verification:
New head: 755bda1 |
…blocking prerender An empty new team is owed the first-run screen, the MCP URL comes from the deployment env instead of request headers, and both pages keep a static shell so instant navigation holds. Co-authored-by: Cursor <cursoragent@cursor.com>
Hypothesis
Agent-first onboarding. 3 of the 9 people who signed up went straight to
/settings/mcpright after creating their account: the first thing they wanted was to reach the product from their agent. That page is a settings page, which is the last place a new account looks, so the promise "use it from your agent" is buried behind a menu. The invitation to a teammate has the same problem in the other direction: it only appears after a first skill is saved, and Skills Board is worth something only when more than one person is in the library.So: put the agent connection first in the first run, put the invitation beside it rather than behind a skill, and give the connection a page anyone can reach and read, including someone still deciding whether to sign up.
What changed
/connectis a top level page. The MCP setup page moved out of settings. It is public and indexable, so a reader can check the endpoint, the auth model, and the plugin install commands before creating an account, and an agent can read it too. Signed in, the same URL renders inside the product chrome; signed out, it renders in the marketing chrome./settings/mcpredirects to it permanently, in both spellings, so bookmarks and any open tab still work.Every entry point now names the new path: the product nav, the mobile nav, the account menu, the library header, and the command palette.
A new team lands on
/start, not on an empty library. Three steps, side by side, in any order:Creating a team from inside the app still lands in the library: that is a change of context, not a first run.
Analytics. No event was renamed. The moved page keeps
mcp_setup_viewed,mcp_config_copied,plugin_install_copied, andmcp_entry_clickedexactly as they were, and/settings/mcpstays in themcp_entry_clickeddestination union so the series before and after the move stay readable together. New values are additive:location: "onboarding",destination: "/connect",surface: "onboarding". Two events are new, for the two first-run steps that had none of their own:onboarding_steps_viewedandonboarding_step_clickedwithstep: "first_skill" | "invite_team". Connecting an agent is deliberately not one of them, because it is already counted three ways and counting it again would inflate that step./connectis in the sitemap and inllms.txt, one line each.Metric it should move
mcp_config_copiedandplugin_install_copiedper new signup. All time today: 6 and 0. The point is the rate per new account, not the total.team_member_invitedandteam_invite_link_copiedwithsurface: "onboarding"are the ones to watch.onboarding_steps_viewedas the denominator for both, so the first-run screen can be read as a funnel rather than a page view.How we will know
New-signup behavior over the next 2 to 4 weeks, counted per account rather than in totals: of the accounts created after this ships, how many copy a config or an install command, and how many send an invitation. Traffic is small enough that this is a read of behavior, not a test result. If the connect step is copied but the invitation still never is, the pairing is not the constraint and the invite copy is.
Notes
mcp_config_copiedis team scoped so it can be counted againstmcp_setup_viewedfor the same team, and a null-team bucket would cost more than it tells. Their interest is still visible throughplugin_install_copied, which is not team scoped.pnpm typecheckandpnpm test:unitpass: 466 unit tests, 9 of them new for this change.