Skip to content

fix: allow PTT hotkey Tauri commands#3797

Closed
NgoQuocViet2001 wants to merge 3 commits into
tinyhumansai:mainfrom
NgoQuocViet2001:fix/ptt-hotkey-acl
Closed

fix: allow PTT hotkey Tauri commands#3797
NgoQuocViet2001 wants to merge 3 commits into
tinyhumansai:mainfrom
NgoQuocViet2001:fix/ptt-hotkey-acl

Conversation

@NgoQuocViet2001

@NgoQuocViet2001 NgoQuocViet2001 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a narrow Tauri permission for the desktop push-to-talk hotkey flow.
  • Grants that permission to the default desktop capability used by the main/overlay windows.
  • Allows the full default PTT path: register hotkey, unregister hotkey, and show/hide the PTT overlay while held.
  • Tightens the regression test so the permission's command allow-list must match the expected commands exactly.

Problem

The PTT frontend invokes register_ptt_hotkey and unregister_ptt_hotkey from the Tauri webview. Those commands are registered in the Rust invoke handler, but the default desktop capability did not grant them, so Tauri rejected calls with Command unregister_ptt_hotkey not allowed by ACL (#3724).

Follow-up review also caught that the default showOverlay setting calls show_ptt_overlay before audio capture starts. Without that command in the same permission, the hotkey could register but the default PTT flow would still abort on the overlay invoke.

Solution

  • Introduce allow-ptt-hotkey-control, scoped to the three PTT lifecycle commands:
    • register_ptt_hotkey
    • unregister_ptt_hotkey
    • show_ptt_overlay
  • Reference that permission from app/src-tauri/capabilities/default.json.
  • Cover the wiring with app/test/tauri-ptt-acl.test.ts, which parses the default capability and permission TOML contract.
  • Make the test enforce exact command equality, preventing accidental ACL broadening.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy.
  • Diff coverage >= 80% - changed ACL contract is covered by tauri-ptt-acl.test.ts; CI will enforce formal coverage.
  • Coverage matrix updated - N/A: no feature row was added, removed, or renamed.
  • All affected feature IDs from the matrix are listed in ## Related.
  • No new external network dependencies introduced.
  • Manual smoke checklist updated if this touches release-cut surfaces - N/A: desktop ACL config only, no release checklist step changed.
  • Linked issue closed via Closes #NNN in ## Related.

Impact

  • Desktop only.
  • Restores PTT hotkey registration/unregistration and the default overlay display path from the Tauri webview.
  • Security impact is intentionally narrow: the permission allows only the PTT hotkey/overlay lifecycle commands, not broader window/process access.
  • No migration or external dependency changes.

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/ptt-hotkey-acl
  • Commit SHA: 34dc3c03dd1565d69e7406f4897c1312939319d7

Validation Run

  • pnpm --filter openhuman-app exec vitest run --config test/vitest.config.ts test/tauri-ptt-acl.test.ts --pool=threads - 1 file passed, 2 tests passed.
  • pnpm --filter openhuman-app exec prettier --check test/tauri-ptt-acl.test.ts src-tauri/permissions/allow-ptt-hotkey-control.toml
  • git diff --check -- app/src-tauri/permissions/allow-ptt-hotkey-control.toml app/test/tauri-ptt-acl.test.ts
  • Duplicate check: no open PR found for register_ptt_hotkey / unregister_ptt_hotkey before opening this PR.
  • pnpm --filter openhuman-app format:check - N/A: changed-file Prettier check passed instead.
  • pnpm typecheck - N/A: focused Vitest compiled the test file and no app TypeScript source changed.
  • Focused tests: app/test/tauri-ptt-acl.test.ts passed, 2 tests.
  • Rust fmt/check (if changed): N/A, no Rust source changed.
  • Tauri fmt/check (if changed): N/A, Tauri source code unchanged.

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: PTT webview calls can register/unregister the global PTT hotkey and show/hide the default overlay instead of failing Tauri ACL.
  • User-visible effect: enabling, holding, and clearing the push-to-talk shortcut no longer trips ACL denial in the default PTT flow.

Parity Contract

  • Legacy behavior preserved: existing default desktop capability remains unchanged except for the new narrow PTT permission.
  • Guard/fallback/dispatch parity checks: regression test asserts the capability reference and exact command allow-list.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none found in open PR search for register_ptt_hotkey / unregister_ptt_hotkey.
  • Canonical PR: this PR.
  • Resolution (closed/superseded/updated): N/A

@NgoQuocViet2001 NgoQuocViet2001 requested a review from a team June 19, 2026 08:48
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new Tauri permission definition allow-ppt-hotkey-control that grants access to register_ppt_hotkey, unregister_ppt_hotkey, and show_ppt_overlay commands. Registers this permission (along with allow-loopback-oauth) in the default desktop capability. Introduces a Vitest test suite that validates both config files from disk.

Changes

PTT Hotkey ACL Fix

Layer / File(s) Summary
Permission definition and capability wiring
app/src-tauri/permissions/allow-ppt-hotkey-control.toml, app/src-tauri/capabilities/default.json
New allow-ppt-hotkey-control TOML permission grants register_ppt_hotkey, unregister_ppt_hotkey, and show_ppt_overlay with an empty deny list. Both allow-loopback-oauth and allow-ppt-hotkey-control are appended to the permissions array in the default capability.
Vitest ACL validation tests
app/test/tauri-ppt-acl.test.ts
Test harness defines paths and constants, a regex-based helper extracts quoted command strings from the TOML allow list, and a describe suite reads the capability JSON and permission TOML from disk to assert both files are consistent and all PTT commands are present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hippity-hop, the ACL gate swung wide,
No more "command not allowed" to hide!
register_ppt_hotkey skips right through,
unregister and show_ppt_overlay crew.
The TOML is planted, the tests all pass,
Push-to-talk blooms through the security glass! 🎙️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding Tauri ACL permissions for PTT hotkey commands, which is the core fix in the PR.
Linked Issues check ✅ Passed The PR adds register_ptt_hotkey and unregister_ptt_hotkey to the Tauri capability config and includes a regression test, fulfilling the primary coding requirements from issue #3724.
Out of Scope Changes check ✅ Passed All changes are scoped to the Tauri ACL configuration and a related regression test, with no unrelated modifications detected.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29c0a49b9b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread app/src-tauri/permissions/allow-ptt-hotkey-control.toml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/test/tauri-ptt-acl.test.ts (1)

24-26: ⚡ Quick win

Use an interface for the parsed capability shape.

The inline object shape assertion should be replaced with a named interface for TS consistency.

Suggested change
+interface DefaultCapability {
+  permissions?: unknown[];
+}
+
 describe('desktop Tauri ACL for push-to-talk hotkeys', () => {
-  const defaultCapability = JSON.parse(readFileSync(DEFAULT_CAPABILITY_PATH, 'utf8')) as {
-    permissions?: unknown[];
-  };
+  const defaultCapability = JSON.parse(readFileSync(DEFAULT_CAPABILITY_PATH, 'utf8')) as DefaultCapability;

As per coding guidelines, **/*.{ts,tsx}: Always use interface for defining object shapes in TypeScript.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/test/tauri-ptt-acl.test.ts` around lines 24 - 26, The type assertion for
the `defaultCapability` variable uses an inline object shape instead of a named
interface, which violates TypeScript consistency guidelines. Create a named
interface that defines the object shape with the `permissions?: unknown[]`
property, then replace the inline `as { permissions?: unknown[] }` assertion
with the new interface name. This will align with the coding guidelines that
require named interfaces for defining object shapes in TypeScript.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/test/tauri-ptt-acl.test.ts`:
- Around line 33-35: The test in the it.each block only verifies that each
PTT_COMMANDS entry is contained within the extractAllowedCommands result, but
does not enforce that these are the only allowed commands. Change the test to
verify exact equality between the extracted allowed commands and the
PTT_COMMANDS array, rather than using containment checks. This ensures that
adding extra commands to the allow-ptt-hotkey-control permission would cause the
test to fail, preventing unintended ACL scope widening.

---

Nitpick comments:
In `@app/test/tauri-ptt-acl.test.ts`:
- Around line 24-26: The type assertion for the `defaultCapability` variable
uses an inline object shape instead of a named interface, which violates
TypeScript consistency guidelines. Create a named interface that defines the
object shape with the `permissions?: unknown[]` property, then replace the
inline `as { permissions?: unknown[] }` assertion with the new interface name.
This will align with the coding guidelines that require named interfaces for
defining object shapes in TypeScript.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f7d5d58d-bc24-4cb5-9e38-bab84627e7aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2da20d2 and 29c0a49.

📒 Files selected for processing (3)
  • app/src-tauri/capabilities/default.json
  • app/src-tauri/permissions/allow-ptt-hotkey-control.toml
  • app/test/tauri-ptt-acl.test.ts

Comment thread app/test/tauri-ptt-acl.test.ts Outdated
@oxoxDev

oxoxDev commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #3842 (#3724) — identical correctly-scoped allowlist, green/mergeable. Closing in favor of #3842 — thanks for the fix and the exact-match regression test!

@oxoxDev oxoxDev closed this Jun 22, 2026
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.

[Bug] Always-on listening and push-to-talk both broken — ACL error on hotkey registration

3 participants