spec: add Wayland global hotkey toggle plan#10487
spec: add Wayland global hotkey toggle plan#10487oz-for-oss[bot] wants to merge 1 commit intomasterfrom
Conversation
Co-Authored-By: L0GYKAL.eth <32228897+L0GYKAL@users.noreply.github.com> Co-Authored-By: Oz <oz-agent@warp.dev>
|
@oz-for-oss[bot] I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
The product and tech specs define a coherent Wayland-compatible toggle path via compositor-owned shortcuts, channel-specific launcher helpers, single-instance DBus forwarding, and guarded Wayland fallback behavior.
Concerns
- The CLI conflict policy for
--togglewith URL arguments should choose one behavior so implementation and tests do not diverge.
Security
- Activation
platform_dataintroduces a transient token path; the spec should state that these values are not logged, persisted, or emitted in telemetry.
Verdict
Found: 0 critical, 0 important, 2 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| 2. Forward the toggle action through the existing Linux DBus service. | ||
| - In `app/src/app_services/linux/mod.rs`, update `pass_startup_args_to_existing_instance` so `args.toggle_visibility` calls `ExistingApplicationProxy::activate_action` instead of `open`. | ||
| - Use a stable action name such as `toggle-visibility`. Keep it channel-scoped by relying on `DBusServiceHost::well_known_name()` and `ChannelState::app_id()`, as the current `Open` path already does. | ||
| - Continue to pass normal URLs to `Open`; `--toggle` and URL opening should be mutually exclusive at the clap layer or resolved deterministically by prioritizing explicit URLs and returning a clear CLI error for invalid combinations. |
There was a problem hiding this comment.
💡 [SUGGESTION] Choose one policy for --toggle combined with URL args (reject, ignore toggle, or prioritize URLs) and make validation/tests assert that exact behavior.
| - In `app/src/app_services/linux/mod.rs`, update `pass_startup_args_to_existing_instance` so `args.toggle_visibility` calls `ExistingApplicationProxy::activate_action` instead of `open`. | ||
| - Use a stable action name such as `toggle-visibility`. Keep it channel-scoped by relying on `DBusServiceHost::well_known_name()` and `ChannelState::app_id()`, as the current `Open` path already does. | ||
| - Continue to pass normal URLs to `Open`; `--toggle` and URL opening should be mutually exclusive at the clap layer or resolved deterministically by prioritizing explicit URLs and returning a clear CLI error for invalid combinations. | ||
| - Capture and forward freedesktop `platform_data` if the launching environment provides activation metadata such as an activation token. Initially this can be a small helper that reads known environment variables into the DBus platform-data map; if winit cannot consume the token yet, retain it in the action event type for a follow-up rather than discarding it at the DBus boundary. |
There was a problem hiding this comment.
💡 [SUGGESTION] [SECURITY] Treat activation platform_data as transient untrusted input: document that tokens are forwarded only to platform activation code and are not logged, persisted, or emitted in telemetry.
1 participant
Summary
wmctrl, X11-only tooling, or DE-specific integrations as the supported path.Validation
git diff --checkfor the new spec files.Related issue: #4800