Skip to content

fix(routing): keep query params when redirecting to the default mentor#335

Open
sonegillis1 wants to merge 1 commit into
mainfrom
fix/embed-preserve-query-params
Open

fix(routing): keep query params when redirecting to the default mentor#335
sonegillis1 wants to merge 1 commit into
mainfrom
fix/embed-preserve-query-params

Conversation

@sonegillis1

Copy link
Copy Markdown
Contributor

Problem

Visiting http://localhost:3001/?embed=true&mode=anonymous&extra-body-classes=iframed-externally&component=chat produced:

  1. /?embed=…/platform/<tenant>/<mentorId>?embed=… (query preserved ✓)
  2. /platform/<tenant>/<mentorId> (query stripped ✗)

Expected: stay at step 1's URL with the params intact.

Root cause

redirectToMentor in providers/index.tsx fires twice. It only preserved the query when pathname === '/'. The SDK MentorProvider re-invokes it after the first redirect settles — by then pathname is /platform/<tenant>/<mentorId>, so queryParams is '' and it re-pushes the bare URL, stripping embed/mode/component/etc.

Fix

Guard redirectToMentor to bail out when already on the target mentor path, so the redundant self-redirect doesn't drop the query string. All existing navigation behavior is otherwise untouched.

function redirectToMentor(tenantKey, mentorId) {
  const targetPath = `/platform/${tenantKey}/${mentorId}`;
  if (window.location.pathname === targetPath) return; // already here — don't strip
  let queryParams = '';
  if (window.location.pathname === '/') queryParams = window.location.search;
  router.push(`${targetPath}${queryParams}`);
}

Tests

Added a provider unit test asserting redirectToMentor is a no-op (no re-push) when already on the target mentor. Full provider suite green (117 passed); full pre-push unit suite passed (10,564).

Note

Pushed with --no-verify (author-authorized): the pre-push suite passed but git push hit the repeated SIGPIPE-after-pass transport flake. Worth verifying in the browser that the embed URL now stays put; if a params-stripping second redirect persists, the SDK MentorProvider may need a look too.

🤖 Generated with Claude Code

Visiting `/?embed=true&mode=anonymous&...` redirected to the resolved mentor
with the query intact, then the SDK MentorProvider re-invoked redirectToMentor
after landing — and since `pathname` was no longer `/`, it re-pushed a bare
URL and stripped the embed/mode/component params.

Guard redirectToMentor to bail out when already on the target mentor path, so
the redundant re-redirect no longer drops the query string.

Adds a provider unit test covering the no-op-when-already-on-target case.
@sonegillis1
sonegillis1 requested a review from mamigot as a code owner July 14, 2026 04:12
@mamigot

mamigot commented Jul 14, 2026

Copy link
Copy Markdown
Member

Claude finished @sonegillis1's task —— View job


E2E Coverage Review

  • Dismiss stale REQUEST_CHANGES reviews (none found)
  • Identify changed files — no new page.tsx routes
  • Run mechanical coverage check — passed (545 checkpoints, 25/25 files mapped)
  • Applied decision logic: CASE E — approved

✅ E2E Coverage — No Issues

No new app routes were introduced (providers/index.tsx, providers/__tests__/index.test.tsx, CHANGELOG.md only). All existing coverage is intact.

Coverage check: 545 checkpoints unchanged · 25/25 app pages mapped (100%)

@mamigot mamigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ E2E Coverage — No Issues

No new app routes were introduced. All existing coverage is intact.

Coverage check result: 545 checkpoints unchanged, 25/25 app pages mapped (100%).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants