feat: add deeplink support and Raycast extension (#1540)#1728
feat: add deeplink support and Raycast extension (#1540)#1728bcornish1797 wants to merge 4 commits into
Conversation
| if (!url.startsWith("cap-desktop://")) { | ||
| throw new Error("Cap deeplink must use cap-desktop://"); | ||
| } |
There was a problem hiding this comment.
buildCapUrl always constructs the URL with the cap-desktop:// prefix via the template literal, so url.startsWith("cap-desktop://") is unconditionally true and the branch can never be taken. The guard can be removed without changing behavior.
| if (!url.startsWith("cap-desktop://")) { | |
| throw new Error("Cap deeplink must use cap-desktop://"); | |
| } | |
| const url = buildCapUrl(path, params); | |
| try { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/raycast/src/deeplink.ts
Line: 41-43
Comment:
**Dead safety guard**
`buildCapUrl` always constructs the URL with the `cap-desktop://` prefix via the template literal, so `url.startsWith("cap-desktop://")` is unconditionally `true` and the branch can never be taken. The guard can be removed without changing behavior.
```suggestion
const url = buildCapUrl(path, params);
try {
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
已处理:这个 dead guard 已在后续提交里移除。
| - `capture_type`: `screen` or `window` (required) | ||
| - `target`: screen/window name exactly as shown in Cap (required) | ||
| - `capture_system_audio`: `true` / `false` (optional) | ||
| - `mic_label`: microphone label exactly as shown in Cap (optional) | ||
| - omitting `mic_label`, `device_id`, `model_id`, and `off` keeps the current Cap inputs unchanged | ||
| - camera: | ||
| - `device_id=<id>` or `model_id=<VID:PID>` | ||
| - `off=true` to disable camera |
There was a problem hiding this comment.
mic_off parameter not documented
The Rust parser for record/start calls parse_microphone_from_query(&query, "mic_label", "mic_off"), meaning mic_off=true is a valid way to explicitly disable the microphone when starting a recording. This parameter is never mentioned in the docs — users relying on the docs will not know it exists.
Consider adding a bullet like:
- `mic_off=true` to disable microphone input (cannot be combined with `mic_label`)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/DEEPLINKS.md
Line: 15-22
Comment:
**`mic_off` parameter not documented**
The Rust parser for `record/start` calls `parse_microphone_from_query(&query, "mic_label", "mic_off")`, meaning `mic_off=true` is a valid way to explicitly disable the microphone when starting a recording. This parameter is never mentioned in the docs — users relying on the docs will not know it exists.
Consider adding a bullet like:
```
- `mic_off=true` to disable microphone input (cannot be combined with `mic_label`)
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
已处理:文档里已补充 mic_off=true,且注明不能与 mic_label 同用。
|
Hi, it looks like the CI workflow is waiting for approval to run (first-time contributor from a fork). Could you please approve the workflow run? Thanks! |
|
@greptile-apps both review suggestions (dead safety guard in apps/raycast/src/deeplink.ts and mic_off parameter documentation in apps/desktop/src-tauri/DEEPLINKS.md) are already addressed in commit 1d8f6cf. Please re-review. |
|
Bumping this: greptile suggestions already addressed in commit 1d8f6cf (dead safety guard removed, mic_off documented). CI is green aside from the Vercel 'Authorization required to deploy' gate that's a fork-workflow permission issue, not code. Would appreciate a re-review @greptile-apps and a look from maintainers when time allows. Happy to add anything else if blocking. |
| let (mic_label, _) = parse_microphone_from_query(&query, "label", "off")?; | ||
| Ok(Self::SwitchMicrophone { mic_label }) |
There was a problem hiding this comment.
No-params call silently disables microphone
parse_microphone_from_query returns (None, false) when neither label nor off is present — the false signals "no change requested." By discarding it with _, this arm creates SwitchMicrophone { mic_label: None }, which calls set_mic_input(state, None). Confirmed from lib.rs:565-572: set_mic_input(None) compares against app.selected_mic_label and, when the mic is currently active, proceeds to disable it. A bare cap-desktop://device/microphone (no query params) will therefore silently disable an active microphone instead of being a no-op or returning an error.
Either require at least one of label or off and return a ParseFailed error when both are absent, or mirror the StartRecordingPath pattern and only call set_mic_input when apply_mic is true:
("device", "microphone") => {
let (mic_label, apply_mic) = parse_microphone_from_query(&query, "label", "off")?;
if !apply_mic {
return Err(ActionParseFromUrlError::ParseFailed(
"device/microphone requires 'label' or 'off=true'".to_string(),
));
}
Ok(Self::SwitchMicrophone { mic_label })
}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 281-282
Comment:
**No-params call silently disables microphone**
`parse_microphone_from_query` returns `(None, false)` when neither `label` nor `off` is present — the `false` signals "no change requested." By discarding it with `_`, this arm creates `SwitchMicrophone { mic_label: None }`, which calls `set_mic_input(state, None)`. Confirmed from `lib.rs:565-572`: `set_mic_input(None)` compares against `app.selected_mic_label` and, when the mic is currently active, proceeds to disable it. A bare `cap-desktop://device/microphone` (no query params) will therefore silently disable an active microphone instead of being a no-op or returning an error.
Either require at least one of `label` or `off` and return a `ParseFailed` error when both are absent, or mirror the `StartRecordingPath` pattern and only call `set_mic_input` when `apply_mic` is `true`:
```rust
("device", "microphone") => {
let (mic_label, apply_mic) = parse_microphone_from_query(&query, "label", "off")?;
if !apply_mic {
return Err(ActionParseFromUrlError::ParseFailed(
"device/microphone requires 'label' or 'off=true'".to_string(),
));
}
Ok(Self::SwitchMicrophone { mic_label })
}
```
How can I resolve this? If you propose a fix, please make it concise.|
Hi, could a maintainer please approve the CI for this fork PR? All checks are ready to run. Thank you! |
|
Hi team — quick follow-up on this one. Looking at the CI state, the only remaining blocker is the Vercel deployment check, which shows Would any team member with Vercel team access mind approving the deployment when you get a chance? That's the single step between this PR and mergeability. Thanks! |
|
Quick update — pushed Fix: Tests added (in the same commit):
The test module now has 14 cases (up from 7) and the confirmed P1 is closed. @greptile-apps please re-review when you have a moment. @richiemcilroy @Brendonovich — current status at a glance:
Happy to add anything else if it helps the merge. |
| ("device", "camera") => Ok(Self::SwitchCamera { | ||
| camera: parse_camera_from_query(&query)?, | ||
| }), |
There was a problem hiding this comment.
Bare
device/camera URL silently disables camera
parse_camera_from_query returns Ok(None) when no params are present (all inputs None, off=false), so cap-desktop://device/camera with no query string produces SwitchCamera { camera: None } and calls set_camera_input(None) — which, confirmed from lib.rs:731-742, sets camera_in_use = false, clears selected_camera_id, and calls RemoveInput. This is exactly the same silent-disable bug that was fixed for device/microphone (which now requires label or off=true). The camera handler needs the same guard:
("device", "camera") => {
let camera = parse_camera_from_query(&query)?;
if !query.contains_key("device_id")
&& !query.contains_key("model_id")
&& !query.contains_key("off")
{
return Err(ActionParseFromUrlError::ParseFailed(
"device/camera requires 'device_id', 'model_id', or 'off=true'".to_string(),
));
}
Ok(Self::SwitchCamera { camera })
}A corresponding rejects_camera_with_no_params test should also be added to mirror rejects_microphone_with_no_params.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 290-292
Comment:
**Bare `device/camera` URL silently disables camera**
`parse_camera_from_query` returns `Ok(None)` when no params are present (all inputs `None`, `off=false`), so `cap-desktop://device/camera` with no query string produces `SwitchCamera { camera: None }` and calls `set_camera_input(None)` — which, confirmed from `lib.rs:731-742`, sets `camera_in_use = false`, clears `selected_camera_id`, and calls `RemoveInput`. This is exactly the same silent-disable bug that was fixed for `device/microphone` (which now requires `label` or `off=true`). The camera handler needs the same guard:
```rust
("device", "camera") => {
let camera = parse_camera_from_query(&query)?;
if !query.contains_key("device_id")
&& !query.contains_key("model_id")
&& !query.contains_key("off")
{
return Err(ActionParseFromUrlError::ParseFailed(
"device/camera requires 'device_id', 'model_id', or 'off=true'".to_string(),
));
}
Ok(Self::SwitchCamera { camera })
}
```
A corresponding `rejects_camera_with_no_params` test should also be added to mirror `rejects_microphone_with_no_params`.
How can I resolve this? If you propose a fix, please make it concise.Address the remaining Greptile P1: a bare `cap-desktop://device/microphone`
URL (no `label` or `off=true`) previously constructed
`SwitchMicrophone { mic_label: None }`, which in turn called
`set_mic_input(None)` and silently disabled an active microphone. The
Raycast extension never sends this shape, but the Rust handler is a public
API surface and must not treat 'no intent' as 'disable'. The arm now
returns `ParseFailed` when neither `label` nor `off` is supplied,
mirroring the existing `StartRecordingPath` `apply_mic` pattern.
Also expands the deeplink test module to cover gaps highlighted during
review:
- regression: `rejects_microphone_with_no_params` locks in the fix above
- `parses_switch_microphone_label` (previously only `off=true` was tested)
- `parses_stop_pause_resume_restart_toggle` covers the five record/* unit
actions that had no direct tests
- `parses_settings_open_with_page` / `parses_settings_open_without_page`
- `rejects_unknown_path_domain` for the fallthrough arm
- `rejects_start_recording_invalid_capture_type` for capture_type guard
Mirror of the recent device/microphone fix. A bare
cap-desktop://device/camera URL would previously produce
SwitchCamera { camera: None }, which silently disables an active
camera. The arm now requires at least one of device_id, model_id, or
off=true and returns ParseFailed otherwise, matching the microphone
discipline.
Adds two tests: rejects_camera_with_no_params locks in the fix, and
parses_switch_camera_off covers the explicit disable path so the
rejection does not regress it.
4f68f5c to
089b762
Compare
|
@richiemcilroy I rebased this onto the latest main, so it is conflict free again. The deeplink P1 Greptile flagged (a bare device/microphone or device/camera URL silently turning off the input) is already fixed; those return a parse error now. The only failing check is the Vercel preview deploy, which needs a Cap team member to authorize it for a fork PR. Could you take a look when you have time? |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
| let apply_camera = query.contains_key("device_id") | ||
| || query.contains_key("model_id") | ||
| || query.contains_key("off"); | ||
| let (mic_label, apply_mic) = | ||
| parse_microphone_from_query(&query, "mic_label", "mic_off")?; | ||
|
|
||
| Ok(Self::StartRecordingPath { | ||
| capture_mode, | ||
| camera: parse_camera_from_query(&query)?, | ||
| apply_camera, | ||
| mic_label, | ||
| apply_mic, | ||
| capture_system_audio: parse_bool( | ||
| query.get("capture_system_audio"), | ||
| false, | ||
| )?, | ||
| mode: parse_mode(query.get("mode"))?, | ||
| }) |
There was a problem hiding this comment.
apply_camera treating off as “present” (vs off=true) can accidentally disable the camera for URLs like ...&off=false or device_id=. Suggest basing the flag on parsed values instead of contains_key.
| let apply_camera = query.contains_key("device_id") | |
| || query.contains_key("model_id") | |
| || query.contains_key("off"); | |
| let (mic_label, apply_mic) = | |
| parse_microphone_from_query(&query, "mic_label", "mic_off")?; | |
| Ok(Self::StartRecordingPath { | |
| capture_mode, | |
| camera: parse_camera_from_query(&query)?, | |
| apply_camera, | |
| mic_label, | |
| apply_mic, | |
| capture_system_audio: parse_bool( | |
| query.get("capture_system_audio"), | |
| false, | |
| )?, | |
| mode: parse_mode(query.get("mode"))?, | |
| }) | |
| let camera = parse_camera_from_query(&query)?; | |
| let off = parse_bool(query.get("off"), false)?; | |
| let apply_camera = camera.is_some() || off; | |
| let (mic_label, apply_mic) = | |
| parse_microphone_from_query(&query, "mic_label", "mic_off")?; | |
| Ok(Self::StartRecordingPath { | |
| capture_mode, | |
| camera, | |
| apply_camera, | |
| mic_label, | |
| apply_mic, | |
| capture_system_audio: parse_bool( | |
| query.get("capture_system_audio"), | |
| false, | |
| )?, | |
| mode: parse_mode(query.get("mode"))?, | |
| }) |
| ("device", "camera") => { | ||
| let has_camera_param = query.contains_key("device_id") | ||
| || query.contains_key("model_id") | ||
| || query.contains_key("off"); | ||
| if !has_camera_param { | ||
| return Err(ActionParseFromUrlError::ParseFailed( | ||
| "device/camera requires 'device_id', 'model_id', or 'off=true'" | ||
| .to_string(), | ||
| )); | ||
| } | ||
| Ok(Self::SwitchCamera { | ||
| camera: parse_camera_from_query(&query)?, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Similar issue here: has_camera_param will be true for off=false or empty device_id=, and parse_camera_from_query can yield None in those cases, which looks like a camera-disable.
| ("device", "camera") => { | |
| let has_camera_param = query.contains_key("device_id") | |
| || query.contains_key("model_id") | |
| || query.contains_key("off"); | |
| if !has_camera_param { | |
| return Err(ActionParseFromUrlError::ParseFailed( | |
| "device/camera requires 'device_id', 'model_id', or 'off=true'" | |
| .to_string(), | |
| )); | |
| } | |
| Ok(Self::SwitchCamera { | |
| camera: parse_camera_from_query(&query)?, | |
| }) | |
| } | |
| ("device", "camera") => { | |
| let off = parse_bool(query.get("off"), false)?; | |
| let camera = parse_camera_from_query(&query)?; | |
| if camera.is_none() && !off { | |
| return Err(ActionParseFromUrlError::ParseFailed( | |
| "device/camera requires 'device_id', 'model_id', or 'off=true'" | |
| .to_string(), | |
| )); | |
| } | |
| Ok(Self::SwitchCamera { camera }) | |
| } |
| let label_present = query.contains_key(label_key); | ||
| let label = parse_optional_string(query.get(label_key)); | ||
| let off = parse_bool(query.get(off_key), false)?; | ||
|
|
||
| if off && label_present { | ||
| return Err(ActionParseFromUrlError::ParseFailed(format!( | ||
| "microphone deep link cannot combine '{off_key}=true' with '{label_key}'" | ||
| ))); | ||
| } | ||
|
|
||
| if off { | ||
| return Ok((None, true)); | ||
| } | ||
|
|
||
| if label_present { | ||
| return Ok((label, true)); | ||
| } | ||
|
|
||
| Ok((None, false)) |
There was a problem hiding this comment.
label_present + an empty/whitespace label currently becomes (None, true), which is indistinguishable from an explicit off=true downstream. Feels safer to reject empty labels when the key is present.
| let label_present = query.contains_key(label_key); | |
| let label = parse_optional_string(query.get(label_key)); | |
| let off = parse_bool(query.get(off_key), false)?; | |
| if off && label_present { | |
| return Err(ActionParseFromUrlError::ParseFailed(format!( | |
| "microphone deep link cannot combine '{off_key}=true' with '{label_key}'" | |
| ))); | |
| } | |
| if off { | |
| return Ok((None, true)); | |
| } | |
| if label_present { | |
| return Ok((label, true)); | |
| } | |
| Ok((None, false)) | |
| if label_present && label.is_none() { | |
| return Err(ActionParseFromUrlError::ParseFailed(format!( | |
| "microphone deep link requires non-empty '{label_key}'" | |
| ))); | |
| } | |
| if label_present { | |
| return Ok((label, true)); | |
| } |
Adds path-based cap-desktop:// deeplinks for start/stop/pause/resume/toggle/restart plus microphone, camera, and settings. Keeps the existing cap-desktop://action?value=... JSON format working. Includes a small Raycast extension that uses the new deeplinks.
/claim #1540
Demo video at apps/raycast/demo/cap-raycast-demo.mp4. Raycast typecheck passes. I couldn't run cargo tests in my environment, and ray lint still trips on this monorepo's Raycast validation wiring, but the extension source typechecks cleanly.