Skip to content

Conversation

@thomaswhyyou
Copy link
Contributor

@thomaswhyyou thomaswhyyou commented Jan 27, 2026

Description

Add a search field to GuideActivationUrlPatternData, and include it when initializing a new URLPattern, which is a new match field added in https://github.com/knocklabs/control/pull/7160.

@vercel
Copy link

vercel bot commented Jan 27, 2026

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

Project Deployment Review Updated (UTC)
javascript-ms-teams-connect-example Ready Ready Preview, Comment Jan 28, 2026 9:57pm
javascript-nextjs-example Ready Ready Preview, Comment Jan 28, 2026 9:57pm
javascript-slack-connect-example Ready Ready Preview, Comment Jan 28, 2026 9:57pm
javascript-slack-kit-example Ready Ready Preview, Comment Jan 28, 2026 9:57pm

Request Review

@linear
Copy link

linear bot commented Jan 27, 2026

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2026

🦋 Changeset detected

Latest commit: 1eeac18

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

This PR includes changesets to release 13 packages
Name Type
@knocklabs/client Patch
client-example Patch
guide-example Patch
@knocklabs/expo Patch
@knocklabs/react-core Patch
@knocklabs/react-native Patch
@knocklabs/react Patch
@knocklabs/expo-example Patch
ms-teams-connect-example Patch
nextjs-app-dir-example Patch
nextjs-example Patch
slack-connect-example Patch
slack-kit-example Patch

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

Copy link
Contributor Author

thomaswhyyou commented Jan 27, 2026

@thomaswhyyou thomaswhyyou force-pushed the thomas-kno-10246-control-support-exclusion-rules-for-activation-rules-3 branch from ac02df9 to 6729fc9 Compare January 27, 2026 20:36
@thomaswhyyou thomaswhyyou force-pushed the thomas-kno-11298-sdk-add-support-for-targeting-query-params branch from 630fc0e to 7493be5 Compare January 27, 2026 20:36
@thomaswhyyou thomaswhyyou changed the title support search param in activation url pattern feat: support search param in activation url pattern Jan 27, 2026
@thomaswhyyou
Copy link
Contributor Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@thomaswhyyou thomaswhyyou marked this pull request as ready for review January 27, 2026 22:35
@thomaswhyyou thomaswhyyou requested review from a team, connorlindsey and emisilvacab January 27, 2026 22:35
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

pattern: new URLPattern({
pathname: rule.pathname ?? undefined,
search: rule.search ?? undefined,
}),
Copy link

Choose a reason for hiding this comment

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

URLPattern matches all URLs when both fields undefined

Medium Severity

When both pathname and search are undefined or null in a GuideActivationUrlPatternData, the URLPattern constructor receives { pathname: undefined, search: undefined }, which defaults all components to wildcard patterns and matches every URL. The type's comment states "At least one part should be present" but this invariant isn't enforced in code. If the server sends malformed data, an "allow" directive would show guides on all pages, or a "block" directive would hide guides everywhere.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fine, it is validated by the control backend to have at least one part.

Base automatically changed from thomas-kno-10246-control-support-exclusion-rules-for-activation-rules-3 to main January 28, 2026 21:50
@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.45%. Comparing base (0ceb165) to head (1eeac18).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/client/src/clients/guide/client.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
- Coverage   68.47%   68.45%   -0.03%     
==========================================
  Files         193      193              
  Lines        8058     8061       +3     
  Branches     1065     1065              
==========================================
  Hits         5518     5518              
- Misses       2515     2518       +3     
  Partials       25       25              
Files with missing lines Coverage Δ
packages/client/src/clients/guide/types.ts 100.00% <ø> (ø)
packages/client/src/clients/guide/client.ts 88.41% <0.00%> (-0.32%) ⬇️

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.

3 participants