nit(mcp): add deeplink button for cursor #6026

centdix · 2025-06-23T08:13:42Z

Important

Adds a deep link button for MCP tokens in TokensTable.svelte.

Feature Addition:
- Adds cursorMcpUrl derived URL in TokensTable.svelte for deep linking MCP tokens.
- Adds a button with an image linking to cursorMcpUrl in the MCP token display section.
UI Changes:
- Changes MCP token display to a flex column layout to accommodate the new button.

^{This description was created by}^{for 5292138. You can customize this summary. It will automatically update as commits are pushed.}

ellipsis-dev

Caution

Changes requested ❌

Reviewed everything up to 84057cd in 1 minute and 57 seconds. Click for details.

Reviewed 34 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/settings/TokensTable.svelte:225

Draft comment:
Verify the layout change from 'flex-row flex-wrap' to 'flex-col' is intentional, as it will stack children vertically and might affect positioning.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. frontend/src/lib/components/settings/TokensTable.svelte:235

Draft comment:
Extract the inline btoa(JSON.stringify(...)) computation into a separate variable to improve readability and maintainability.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_mzjYZWQoCY6hJUbT

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

ellipsis-dev · 2025-06-23T08:16:18Z

frontend/src/lib/components/settings/TokensTable.svelte

 		>
 			<p class="text-sm mb-2">New MCP URL:</p>
 			<ClipboardPanel content={`${mcpBaseUrl}${newMcpToken}`} />
 			<p class="pt-1 text-xs">
 				Make sure to copy this URL now. You won't be able to see it again!
 			</p>
+			<div class="flex flex-row justify-end gap-2 items-center">
+				<a


Consider adding target='_blank' and rel='noopener noreferrer' to the anchor for safe external navigation, unless the current behavior is intended.

cloudflare-workers-and-pages · 2025-06-23T08:20:12Z

Deploying windmill with Cloudflare Pages

Latest commit:	`5292138`
Status:	✅ Deploy successful!
Preview URL:	https://23279fc1.windmill.pages.dev
Branch Preview URL:	https://fg-add-cursor-button.windmill.pages.dev

View logs

ellipsis-dev

Caution

Changes requested ❌

Reviewed everything up to 9cf701f in 2 minutes and 1 seconds. Click for details.

Reviewed 34 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/settings/TokensTable.svelte:223

Draft comment:
Ensure the change from 'flex flex-row flex-wrap' to 'flex flex-col' meets the design layout requirements.
Reason this comment was not posted:
Confidence changes required: 10% <= threshold 50% None

Workflow ID: wflow_CormHItVeUkqUa4E

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

frontend/src/lib/components/settings/TokensTable.svelte

rubenfiszel · 2025-06-23T08:27:06Z

/ai review this PR

rubenfiszel · 2025-06-23T08:28:12Z

/ai review this PR

claude · 2025-06-23T08:29:56Z

Claude finished @rubenfiszel's task —— View job

Code Review for PR: Add Deeplink Button for Cursor

Todo List:

Read and analyze the changed file (TokensTable.svelte)
Check implementation against Svelte best practices
Review for security, performance, and functional issues
Provide comprehensive review feedback

Analysis

The PR adds a deeplink button for MCP tokens that allows users to easily add the Windmill MCP server to Cursor. The implementation is in frontend/src/lib/components/settings/TokensTable.svelte with changes to lines 226 and 233-247.

Review Findings

✅ Positive Aspects

Clean Integration: The button integrates well with the existing MCP token display logic
User Experience: Provides a convenient way for users to add Windmill MCP server to Cursor
Conditional Display: Properly shows only when newMcpToken is present
Good Visual Design: Uses the official Cursor install image

⚠️ Issues to Address

Security & URL Encoding (frontend/src/lib/components/settings/TokensTable.svelte:235)
```
href={`https://cursor.com/install-mcp?name=windmill&config=${btoa(
```
- Issue: The base64 encoded config should be URL-encoded to ensure safe transmission
- Fix: Wrap btoa() result with encodeURIComponent()
- Risk: Potential URL parsing issues if the base64 contains URL-unsafe characters
External Link Security (frontend/src/lib/components/settings/TokensTable.svelte:234)
```
<a href={`https://cursor.com/install-mcp...`} class="pt-2">
```
- Issue: External link opens in same tab without security attributes
- Recommendation: Add target="_blank" and rel="noopener noreferrer" for security and UX
Code Readability (frontend/src/lib/components/settings/TokensTable.svelte:235-239)
```
href={`https://cursor.com/install-mcp?name=windmill&config=${btoa(
    JSON.stringify({
        url: `${mcpBaseUrl}${newMcpToken}`
    })
)}`}
```
- Issue: Complex inline computation reduces readability
- Recommendation: Extract the URL generation to a derived value or computed variable
Layout Change Verification (frontend/src/lib/components/settings/TokensTable.svelte:226)
```
class="... flex flex-col"  // Changed from flex-row flex-wrap
```
- Question: Is this layout change intentional? It will stack elements vertically
- Impact: Changes the visual layout of the MCP token display
Accessibility Improvements (frontend/src/lib/components/settings/TokensTable.svelte:241-245)
```
<img src="https://cursor.com/deeplink/mcp-install-dark.svg" alt="Add windmill MCP server to Cursor" height="32" />
```
- Improvement: Consider adding width attribute for better sizing control
- Improvement: The alt text could be more descriptive for screen readers

Recommendations

High Priority: Fix URL encoding issue

href={`https://cursor.com/install-mcp?name=windmill&config=${encodeURIComponent(btoa(JSON.stringify({ url: `${mcpBaseUrl}${newMcpToken}` })))}`}

Medium Priority: Add security attributes

<a href="..." target="_blank" rel="noopener noreferrer" class="pt-2">

Low Priority: Extract URL generation for better readability

const cursorDeeplinkUrl = $derived(
    `https://cursor.com/install-mcp?name=windmill&config=${encodeURIComponent(btoa(JSON.stringify({ url: `${mcpBaseUrl}${newMcpToken}` })))}`
)

Conclusion

The feature implementation is solid and provides good user value. The main concerns are around security (URL encoding) and external link handling. The layout change should be confirmed as intentional. After addressing the URL encoding issue, this would be ready to merge.

claude · 2025-06-23T08:30:54Z

Claude finished @rubenfiszel's task —— View job

Code Review for PR: Add Deeplink Button for Cursor

Todo List:

Read and analyze the changed file (TokensTable.svelte)
Check implementation against Svelte best practices
Review for security, performance, and functional issues
Provide comprehensive review feedback

Review Summary

The implementation adds a useful deeplink button for Cursor integration, but there are several important issues that should be addressed:

🔴 Critical Issues

Security: URL Encoding Missing (TokensTable.svelte:235)
```
- href={`https://cursor.com/install-mcp?name=windmill&config=${btoa(
+ href={`https://cursor.com/install-mcp?name=windmill&config=${encodeURIComponent(btoa(
```
The btoa() result should be URL-encoded to ensure special characters are properly escaped in the query parameter.
Security: External Link Safety (TokensTable.svelte:234)
```
- <a
+ <a target="_blank" rel="noopener noreferrer"
```
External links should include security attributes to prevent potential security issues.

🟡 Code Quality Issues

Readability: Complex Inline Expression (TokensTable.svelte:235-239)
The inline btoa(JSON.stringify(...)) computation makes the href attribute hard to read. Consider extracting it:

<script>
const cursorConfig = $derived(btoa(JSON.stringify({
  url: `${mcpBaseUrl}${newMcpToken}`
})));
</script>

<a href={`https://cursor.com/install-mcp?name=windmill&config=${encodeURIComponent(cursorConfig)}`}>

Layout Change Impact (TokensTable.svelte:226)
The change from flex flex-row flex-wrap to flex flex-col significantly alters the layout behavior. Ensure this vertical stacking is intentional and doesn't break existing designs.

✅ Positive Aspects

Conditional Rendering: Properly shows the button only when newMcpToken is present
Clear Intent: The deeplink functionality is straightforward and well-targeted
Image Alt Text: Good accessibility with descriptive alt text
Integration Pattern: Follows a reasonable pattern for MCP server integration

🔧 Recommendations

Apply security fixes for URL encoding and external link attributes
Extract the config computation to improve readability
Verify layout changes don't break existing UI in different screen sizes
Consider error handling for cases where the Cursor deeplink might fail

Svelte 5 Best Practices Compliance

The code generally follows Svelte 5 patterns well:

✅ Uses $derived for reactive computations (mcpBaseUrl)
✅ Uses $state for component state management
✅ Proper conditional rendering with {#if}
⚠️ Could benefit from extracting the config computation to a $derived value

The ellipsis-dev review comments align with these findings, particularly around URL encoding and external link security.

ellipsis-dev

Important

Looks good to me! 👍

Reviewed 5292138 in 1 minute and 41 seconds. Click for details.

Reviewed 31 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/lib/components/settings/TokensTable.svelte:68

Draft comment:
Ensure newMcpToken is defined before concatenation. If undefined, the encoded URL may be malformed and btoa can throw on non-ASCII values.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The cursorMcpUrl is only used in the template when newMcpToken exists (line 229-248). The URL is never constructed when newMcpToken is undefined. The component handles this case by not showing the Cursor button at all when there's no token. This is good defensive programming - the potentially dangerous operation only happens when we know we have a valid token. I could be missing some edge case where the reactive URL gets computed before newMcpToken is set. The $derived computation might run at unexpected times. Even if the URL is computed early, it won't be used until newMcpToken exists. The component's UI guards against misuse. Adding ?? '' would hide a potential bug rather than fix it. The comment should be deleted. The component already safely handles undefined tokens by not showing the URL until a token exists.

2. frontend/src/lib/components/settings/TokensTable.svelte:239

Draft comment:
When using cursorMcpUrl as the href, ensure that its computed value is always a valid URL. Consider conditionally rendering the link if dependencies are undefined.
Reason this comment was not posted:
Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_iqTLl7hlBHmbhyMU

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

centdix marked this pull request as ready for review June 23, 2025 08:14

centdix requested review from rubenfiszel, hugocasa and alpetric as code owners June 23, 2025 08:14

ellipsis-dev bot reviewed Jun 23, 2025

View reviewed changes

add deeplink button for cursor

9cf701f

centdix force-pushed the fg/add-cursor-button branch from 84057cd to 9cf701f Compare June 23, 2025 08:17

centdix marked this pull request as draft June 23, 2025 08:18

centdix marked this pull request as ready for review June 23, 2025 08:18

ellipsis-dev bot reviewed Jun 23, 2025

View reviewed changes

frontend/src/lib/components/settings/TokensTable.svelte Outdated Show resolved Hide resolved

frontend/src/lib/components/settings/TokensTable.svelte Outdated Show resolved Hide resolved

fix

5292138

ellipsis-dev bot reviewed Jun 23, 2025

View reviewed changes

rubenfiszel force-pushed the main branch 3 times, most recently from d745b80 to 42e06e7 Compare August 7, 2025 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

nit(mcp): add deeplink button for cursor #6026

nit(mcp): add deeplink button for cursor #6026

Uh oh!

centdix commented Jun 23, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot Jun 23, 2025

Uh oh!

cloudflare-workers-and-pages bot commented Jun 23, 2025 •

edited

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Uh oh!

Uh oh!

rubenfiszel commented Jun 23, 2025

Uh oh!

rubenfiszel commented Jun 23, 2025

Uh oh!

claude bot commented Jun 23, 2025 •

edited

Loading

Uh oh!

claude bot commented Jun 23, 2025 •

edited

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Uh oh!

nit(mcp): add deeplink button for cursor #6026

Are you sure you want to change the base?

nit(mcp): add deeplink button for cursor #6026

Uh oh!

Conversation

centdix commented Jun 23, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

cloudflare-workers-and-pages bot commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying windmill with Cloudflare Pages

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rubenfiszel commented Jun 23, 2025

Uh oh!

rubenfiszel commented Jun 23, 2025

Uh oh!

claude bot commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review for PR: Add Deeplink Button for Cursor

Analysis

Review Findings

✅ Positive Aspects

⚠️ Issues to Address

Recommendations

Conclusion

Uh oh!

claude bot commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review for PR: Add Deeplink Button for Cursor

Review Summary

🔴 Critical Issues

🟡 Code Quality Issues

✅ Positive Aspects

🔧 Recommendations

Svelte 5 Best Practices Compliance

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

centdix commented Jun 23, 2025 •

edited by ellipsis-dev bot

Loading

cloudflare-workers-and-pages bot commented Jun 23, 2025 •

edited

Loading

claude bot commented Jun 23, 2025 •

edited

Loading

claude bot commented Jun 23, 2025 •

edited

Loading