Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(clerk-js): complete the Safari ITP touch hop in redirect by dmoerner · Pull Request #9308 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): complete the Safari ITP touch hop in redirect - #9308

Merged
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp
Jul 31, 2026
Merged

fix(clerk-js): complete the Safari ITP touch hop in redirect#9308
dmoerner merged 1 commit into
mainfrom
daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp

Conversation

@dmoerner

Copy link
Copy Markdown
Contributor

The else if (redirectUrl) branch navigated to /v1/client/touch and then immediately navigated again to the undecorated redirect URL. Where both resolve to a hard navigation, the second assignment supersedes the first and the touch request is aborted, so the client cookie is never extended past ITP's 7 day cap.

This was a regression that was accidentally introduced in https://github.com/clerk/javascript/pull/6486/changes#diff-1952d6b8ac4b5495e1dba57c32bff9ce72d43f83595c1bc8532a76cf31feb753L1314 11 months ago, when an else in a nested control flow was accidentally dropped. I suspect it was not reported because Safari use is less common and the sign out would have only occurred after 7 days.

Route the redirect through the shared #decorateUrlWithTouch helper instead, which returns the URL unchanged when the client is not eligible.

The existing test asserted only navigate.mock.calls[0][0], so it stayed green across the regression; it now also asserts navigate is called exactly once.

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

…ectUrl })
The `else if (redirectUrl)` branch navigated to `/v1/client/touch` and then
immediately navigated again to the undecorated redirect URL. Where both resolve
to a hard navigation, the second assignment supersedes the first and the touch
request is aborted, so the client cookie is never extended past ITP's 7 day cap.
Route the redirect through the shared #decorateUrlWithTouch helper instead,
which returns the URL unchanged when the client is not eligible.
The existing test asserted only navigate.mock.calls[0][0], so it stayed green
across the regression; it now also asserts navigate is called exactly once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmoerner
dmoerner requested a review from dstaleyJuly 31, 2026 18:24
@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreviewJul 31, 2026 6:24pm
swingsetReadyReadyPreviewJul 31, 2026 6:24pm

Request Review

@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acfcfc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/electronPatch
@clerk/expoPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9308

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9308

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9308

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9308

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9308

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9308

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9308

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9308

@clerk/expo-google-signin

npm i https://pkg.pr.new/@clerk/expo-google-signin@9308

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9308

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9308

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9308

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9308

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9308

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9308

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9308

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9308

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9308

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9308

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9308

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9308

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9308

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9308

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9308

commit: acfcfc6

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-31T18:27:39.499Z

Summary

MetricCount
Packages analyzed19
Packages with changes1
🔴 Breaking changes0
🟡 Non-breaking changes1
🟢 Additions0

🤖 This report was reviewed by claude-sonnet-4-6.


@clerk/express

Current version: 2.1.49
Recommended bump: MINOR → 2.2.0

Subpath ./types

🟡 Non-breaking Changes (1)

Modified: AuthenticateRequestParams
 type AuthenticateRequestParams = {
clerkClient: ClerkClient$1;
request: Request;
- options?: ClerkMiddlewareOptions; /** Prebuilt ClerkRequest, so callers that already converted the request can skip re-conversion. */- clerkRequest?: ClerkRequest;+ options?: ClerkMiddlewareOptions;
};

Static analyzer: Breaking change in type alias AuthenticateRequestParams: Type changed: {clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…{clerkClient:import("@clerk/express").~ClerkClient$1;request:import("@types/express").e.Request;options?:import("@clerk…

🤖 AI review (reclassified as non-breaking) (85%): The removed clerkRequest property was optional in the baseline, so no consumer was required to supply it; existing call sites that omit it remain valid, and call sites that did pass it will now get a type error — however, since AuthenticateRequestParams appears to be an internal input type (not a widely-consumed public contract) and the removed field was optional (consumers could always omit it), the impact on well-typed callers is limited. That said, any consumer that explicitly passed clerkRequest will now get a compile error.


Report generated by Break Check

Last ran on acfcfc6.

@coderabbitai

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e216114-8684-4193-8067-a9357f8a2760

📥 Commits

Reviewing files that changed from the base of the PR and between a601cd7 and acfcfc6.

📒 Files selected for processing (3)
  • .changeset/smart-jars-repeat.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/clerk.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go(manual)
  • clerk/dashboard(manual)
  • clerk/accounts(manual)
  • clerk/backoffice(manual)
  • clerk/clerk(manual)
  • clerk/clerk-docs(manual)
  • clerk/cloudflare-workers(manual)
  • clerk/clerk-ios(auto-detected)
  • clerk/clerk-android(auto-detected)
  • clerk/cli(auto-detected)

📝 Walkthrough

Walkthrough

setActive({ redirectUrl }) now uses #decorateUrlWithTouch and performs one navigation. The test verifies that the touch redirect triggers exactly one navigation call. A patch changeset documents the Safari ITP cookie-refresh fix and affected redirect flows.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • clerk/javascript#9254: Updates shared #decorateUrlWithTouch navigation logic and related Safari ITP redirect tests.

Suggested reviewers:dstaley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description clearly explains the Safari ITP redirect regression, the fix, and the updated test.
Title check✅ PassedThe title clearly identifies the Safari ITP touch-hop fix for redirects.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@wobsorianowobsoriano 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.

great catch!!

@dmoerner

Copy link
Copy Markdown
ContributorAuthor

great catch!!

Definitely partial credit goes to Dylan who in a previous PR told me to reduce code duplication, and then when I added this #decorateUrlWithTouch helper and was looking for places to use it, found this no-op!

@dmoerner
dmoerner merged commit 7f0cac8 into mainJul 31, 2026
52 checks passed
@dmoerner
dmoerner deleted the daniel/core-2755-setactive-redirecturl-never-completes-the-safari-itp branch July 31, 2026 21:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dmoerner@wobsoriano