-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: Bounty: Deeplinks support + Raycast Extension #1721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
6aa6e79
78554f4
2abdd32
4d92066
3ba9521
2b3b755
f0fa459
d72a569
caf19b5
4c90d91
075fe55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { | ||
| parseDeeplink, | ||
| createDeeplink, | ||
| DeeplinkBuilder, | ||
| DeeplinkActions, | ||
| DEEPLINK_PREFIX, | ||
| } from '../deeplinks'; | ||
|
|
||
| describe('parseDeeplink', () => { | ||
| describe('valid deeplinks', () => { | ||
| it('should parse simple action deeplink', () => { | ||
| const result = parseDeeplink('cap://record'); | ||
| expect(result).toEqual({ action: 'record' }); | ||
| }); | ||
|
|
||
| it('should parse deeplink with query parameters', () => { | ||
| const result = parseDeeplink('cap://switch-microphone?deviceId=mic-123'); | ||
| expect(result).toEqual({ | ||
| action: 'switch-microphone', | ||
| deviceId: 'mic-123', | ||
| }); | ||
| }); | ||
|
|
||
| it('should parse deeplink with multiple parameters', () => { | ||
| const result = parseDeeplink('cap://switch-camera?deviceId=cam-456&format=1080p'); | ||
| expect(result).toEqual({ | ||
| action: 'switch-camera', | ||
| deviceId: 'cam-456', | ||
| format: '1080p', | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle URL-encoded parameters', () => { | ||
| const result = parseDeeplink('cap://record?name=My%20Recording'); | ||
| expect(result).toEqual({ | ||
| action: 'record', | ||
| name: 'My Recording', | ||
| }); | ||
| }); | ||
|
|
||
| it('should ignore empty query values', () => { | ||
| const result = parseDeeplink('cap://record?empty='); | ||
| expect(result).toEqual({ action: 'record' }); | ||
| }); | ||
|
|
||
| it('should trim whitespace from URL', () => { | ||
| const result = parseDeeplink(' cap://pause '); | ||
| expect(result).toEqual({ action: 'pause' }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('invalid deeplinks', () => { | ||
| it('should return null for wrong prefix', () => { | ||
| expect(parseDeeplink('http://example.com')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return null for empty string', () => { | ||
| expect(parseDeeplink('')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return null for null/undefined', () => { | ||
| expect(parseDeeplink(null as unknown as string)).toBeNull(); | ||
| expect(parseDeeplink(undefined as unknown as string)).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return null for invalid action', () => { | ||
| expect(parseDeeplink('cap://invalid-action')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return null for malformed URL', () => { | ||
| expect(parseDeeplink('cap://')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should handle malformed query string gracefully', () => { | ||
| // Invalid percent encoding should not throw | ||
| const result = parseDeeplink('cap://record?name=%ZZ'); | ||
| expect(result?.action).toBe('record'); | ||
| }); | ||
|
|
||
| it('should return null for non-string input', () => { | ||
| expect(parseDeeplink(123 as unknown as string)).toBeNull(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('createDeeplink', () => { | ||
| it('should create simple deeplink', () => { | ||
| expect(createDeeplink('record')).toBe('cap://record'); | ||
| }); | ||
|
|
||
| it('should create deeplink with parameters', () => { | ||
| expect(createDeeplink('switch-microphone', { deviceId: 'mic-123' })) | ||
| .toBe('cap://switch-microphone?deviceId=mic-123'); | ||
| }); | ||
|
|
||
| it('should filter out undefined parameters', () => { | ||
| const result = createDeeplink('switch-camera', { | ||
| deviceId: 'cam-456', | ||
| unused: undefined, | ||
| }); | ||
| expect(result).toBe('cap://switch-camera?deviceId=cam-456'); | ||
| }); | ||
|
|
||
| it('should filter out empty string parameters', () => { | ||
| const result = createDeeplink('record', { name: '' }); | ||
| expect(result).toBe('cap://record'); | ||
| }); | ||
|
|
||
| it('should URL-encode special characters', () => { | ||
| const result = createDeeplink('record', { name: 'My Recording' }); | ||
| expect(result).toBe('cap://record?name=My+Recording'); | ||
| }); | ||
|
|
||
| it('should handle no parameters', () => { | ||
| expect(createDeeplink('stop')).toBe('cap://stop'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('DeeplinkBuilder', () => { | ||
| it('should build simple deeplink', () => { | ||
| const result = new DeeplinkBuilder('record').build(); | ||
| expect(result).toBe('cap://record'); | ||
| }); | ||
|
|
||
| it('should build deeplink with parameters', () => { | ||
| const result = new DeeplinkBuilder('switch-microphone') | ||
| .withDeviceId('mic-789') | ||
| .build(); | ||
| expect(result).toBe('cap://switch-microphone?deviceId=mic-789'); | ||
| }); | ||
|
|
||
| it('should chain multiple parameters', () => { | ||
| const result = new DeeplinkBuilder('record') | ||
| .withParam('name', 'Test') | ||
| .withParam('format', 'mp4') | ||
| .build(); | ||
| expect(result).toContain('cap://record?'); | ||
| expect(result).toContain('name=Test'); | ||
| expect(result).toContain('format=mp4'); | ||
| }); | ||
|
|
||
| it('should ignore empty parameters', () => { | ||
| const result = new DeeplinkBuilder('record') | ||
| .withParam('empty', '') | ||
| .build(); | ||
| expect(result).toBe('cap://record'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('DeeplinkActions', () => { | ||
| it('should create startRecording deeplink', () => { | ||
| expect(DeeplinkActions.startRecording()).toBe('cap://record'); | ||
| }); | ||
|
|
||
| it('should create stopRecording deeplink', () => { | ||
| expect(DeeplinkActions.stopRecording()).toBe('cap://stop'); | ||
| }); | ||
|
|
||
| it('should create pauseRecording deeplink', () => { | ||
| expect(DeeplinkActions.pauseRecording()).toBe('cap://pause'); | ||
| }); | ||
|
|
||
| it('should create resumeRecording deeplink', () => { | ||
| expect(DeeplinkActions.resumeRecording()).toBe('cap://resume'); | ||
| }); | ||
|
|
||
| it('should create switchMicrophone deeplink with deviceId', () => { | ||
| expect(DeeplinkActions.switchMicrophone('mic-123')) | ||
| .toBe('cap://switch-microphone?deviceId=mic-123'); | ||
| }); | ||
|
|
||
| it('should throw error for switchMicrophone without deviceId', () => { | ||
| expect(() => DeeplinkActions.switchMicrophone('')).toThrow(); | ||
| }); | ||
|
|
||
| it('should create switchCamera deeplink with deviceId', () => { | ||
| expect(DeeplinkActions.switchCamera('cam-456')) | ||
| .toBe('cap://switch-camera?deviceId=cam-456'); | ||
| }); | ||
|
|
||
| it('should throw error for switchCamera without deviceId', () => { | ||
| expect(() => DeeplinkActions.switchCamera('')).toThrow(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||||||||||||||||||||||
| export type DeeplinkAction = | ||||||||||||||||||||||||
| | 'record' | ||||||||||||||||||||||||
| | 'stop' | ||||||||||||||||||||||||
| | 'pause' | ||||||||||||||||||||||||
| | 'resume' | ||||||||||||||||||||||||
| | 'switch-microphone' | ||||||||||||||||||||||||
| | 'switch-camera'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export interface DeeplinkParams { | ||||||||||||||||||||||||
| action: DeeplinkAction; | ||||||||||||||||||||||||
| deviceId?: string; | ||||||||||||||||||||||||
| [key: string]: string | undefined; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export const DEEPLINK_PREFIX = 'cap://'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Validate action against known types | ||||||||||||||||||||||||
| function isValidAction(action: string): action is DeeplinkAction { | ||||||||||||||||||||||||
| const validActions: DeeplinkAction[] = [ | ||||||||||||||||||||||||
| 'record', | ||||||||||||||||||||||||
| 'stop', | ||||||||||||||||||||||||
| 'pause', | ||||||||||||||||||||||||
| 'resume', | ||||||||||||||||||||||||
| 'switch-microphone', | ||||||||||||||||||||||||
| 'switch-camera', | ||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||
| return validActions.includes(action as DeeplinkAction); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export function parseDeeplink(url: string): DeeplinkParams | null { | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| if (!url || typeof url !== 'string') { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!url.startsWith(DEEPLINK_PREFIX)) { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const urlPart = url.slice(DEEPLINK_PREFIX.length).trim(); | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The URL is checked with
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/utils/src/deeplinks.ts
Line: 36-40
Comment:
**Trim applied after prefix check, breaking whitespace test**
The URL is checked with `startsWith` before it is trimmed. `' cap://pause '.startsWith('cap://')` evaluates to `false`, so the function returns `null`. The `trim()` on `urlPart` only runs on the slice that follows the prefix — it never gets there for padded inputs. The accompanying test `'should trim whitespace from URL'` will always fail.
```suggestion
const trimmedUrl = url.trim();
if (!trimmedUrl.startsWith(DEEPLINK_PREFIX)) {
return null;
}
const urlPart = trimmedUrl.slice(DEEPLINK_PREFIX.length);
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!urlPart) { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const [pathSegment, queryString] = urlPart.split('?'); | ||||||||||||||||||||||||
| const segments = pathSegment.split('/').filter(Boolean); | ||||||||||||||||||||||||
| const action = segments[0]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!action || !isValidAction(action)) { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const queryParams: Record<string, string> = {}; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (queryString) { | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| new URLSearchParams(queryString).forEach((value, key) => { | ||||||||||||||||||||||||
| if (value) { | ||||||||||||||||||||||||
| queryParams[key] = value; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||
| // Invalid query string, continue with parsed params | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||
| action, | ||||||||||||||||||||||||
| ...queryParams, | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export function createDeeplink( | ||||||||||||||||||||||||
| action: DeeplinkAction, | ||||||||||||||||||||||||
| params?: Record<string, string | undefined>, | ||||||||||||||||||||||||
| ): string { | ||||||||||||||||||||||||
| let url = `${DEEPLINK_PREFIX}${action}`; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Filter out undefined/empty values | ||||||||||||||||||||||||
| const validParams = Object.entries(params || {}) | ||||||||||||||||||||||||
| .filter(([, value]) => value !== undefined && value !== '') | ||||||||||||||||||||||||
| .reduce((acc, [key, value]) => { | ||||||||||||||||||||||||
| acc[key] = value as string; | ||||||||||||||||||||||||
| return acc; | ||||||||||||||||||||||||
| }, {} as Record<string, string>); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (Object.keys(validParams).length > 0) { | ||||||||||||||||||||||||
| const searchParams = new URLSearchParams(validParams); | ||||||||||||||||||||||||
| url += `?${searchParams.toString()}`; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return url; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Builder pattern for fluent API | ||||||||||||||||||||||||
| export class DeeplinkBuilder { | ||||||||||||||||||||||||
| private action: DeeplinkAction; | ||||||||||||||||||||||||
| private params: Record<string, string> = {}; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| constructor(action: DeeplinkAction) { | ||||||||||||||||||||||||
| this.action = action; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| withParam(key: string, value: string): this { | ||||||||||||||||||||||||
| if (key && value) { | ||||||||||||||||||||||||
| this.params[key] = value; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| return this; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| withDeviceId(deviceId: string): this { | ||||||||||||||||||||||||
| return this.withParam('deviceId', deviceId); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| build(): string { | ||||||||||||||||||||||||
| return createDeeplink(this.action, this.params); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export const DeeplinkActions = { | ||||||||||||||||||||||||
| startRecording: (): string => createDeeplink('record'), | ||||||||||||||||||||||||
| stopRecording: (): string => createDeeplink('stop'), | ||||||||||||||||||||||||
| pauseRecording: (): string => createDeeplink('pause'), | ||||||||||||||||||||||||
| resumeRecording: (): string => createDeeplink('resume'), | ||||||||||||||||||||||||
| switchMicrophone: (deviceId: string): string => { | ||||||||||||||||||||||||
| if (!deviceId) throw new Error('deviceId is required for switchMicrophone'); | ||||||||||||||||||||||||
| return createDeeplink('switch-microphone', { deviceId }); | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| switchCamera: (deviceId: string): string => { | ||||||||||||||||||||||||
| if (!deviceId) throw new Error('deviceId is required for switchCamera'); | ||||||||||||||||||||||||
| return createDeeplink('switch-camera', { deviceId }); | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { DeeplinkHandler, DeeplinkHandlerError } from '../deeplink-handler'; | ||
|
|
||
| describe('DeeplinkHandler', () => { | ||
| describe('initialization', () => { | ||
| it('should throw if context is not provided', () => { | ||
| expect(() => new DeeplinkHandler(null as any)).toThrow(); | ||
| }); | ||
|
|
||
| it('should accept empty context object', () => { | ||
| const handler = new DeeplinkHandler({}); | ||
| expect(handler).toBeDefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('handle valid actions', () => { | ||
| it('should handle record action', async () => { | ||
| const onStartRecording = vi.fn(); | ||
| const handler = new DeeplinkHandler({ onStartRecording }); | ||
|
|
||
| const result = await handler.handle('cap://record'); | ||
|
|
||
| expect(result).toBe(true); | ||
| expect(onStartRecording).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('should handle stop action', async () => { | ||
| const onStopRecording = vi.fn(); | ||
| const handler = new DeeplinkHandler({ onStopRecording }); | ||
|
|
||
| const result = await handler.handle('cap://stop'); | ||
|
|
||
| expect(result).toBe(true); | ||
| expect(onStopRecording).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('should handle pause action', async () => { | ||
| const onPauseRecording = vi.fn(); | ||
| const handler = new DeeplinkHandler({ onPauseRecording }); | ||
|
|
||
| const result = await handler.handle('cap:// | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The file ends abruptly at Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/web-api-contract-effect/__tests__/deeplink-handler.test.ts
Line: 40-41
Comment:
**Test file is truncated — file ends mid-string**
The file ends abruptly at `handler.handle('cap://` with no closing quote, parenthesis, or block. The test for the `pause` action and any tests that follow are completely missing. The file also lacks a final newline. This will cause a parse error when the test runner loads it.
How can I resolve this? If you propose a fix, please make it concise. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project prohibits all code comments (see CLAUDE.md: "CRITICAL: NO CODE COMMENTS"). This file contains three inline comments (
// Validate action against known types,// Filter out undefined/empty values,// Builder pattern for fluent API) as well as a// Invalid query string, continue with parsed paramscomment inside a catch block. All of these must be removed; the code should be self-explanatory through naming alone.Context Used: CLAUDE.md (source)
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!