fix(macos): review batch 2 — MathCAT gate, redaction, .command/stdin/chord - #935
Merged
Conversation
…chord Five more no-Mac-hardware-safe fixes from the macOS platform review (docs/planning/review.md), worked lowest-to-highest priority: - #46: MathCAT is no longer offered on macOS — like DECtalk, its only backend is a Windows .dll (libmathcat_c.dll); gated Windows-only in the optional-components catalog alongside DECtalk (pure platform-branch relocation; no new public surface). - #60/#73: the macOS `security` CLI passes the keychain secret as `-w <secret>` in separate argv, and a short/non-hex secret slipped past the per-arg redaction into the diagnostics log; format_args_for_log now redacts the value following `-w` explicitly before the generic per-arg pass. - #38 (tester-pending): persona_launcher now writes a Finder-launchable .command shell script (shebang + exec + chmod 0o755) on darwin instead of a useless .bat (Finder runs .command files in Terminal). - #51 (tester-pending): the Simple File Open dialog's "toggle hidden files" chord is now Cmd+Shift+. on macOS (Finder convention) instead of Ctrl+H (the system Hide shortcut), via a platform-aware helper shared by the path-field and list key handlers. - #64/#77 (tester-pending): eSpeak synthesis pipes very long input (>8000 chars) via --stdin instead of a trailing argv element that can overflow the OS command-line length (Windows ~32,767) and truncate/abort a very long Read Aloud span with no clear error. #46 and #60/#73 are fully closed; #38/#51/#64/#77 are code-complete and unit-tested here but only show their effect on real macOS hardware, so they're closed pending tester validation (reopenable via Help > Report a Bug). GATE-11 rebaselined: optional_components.py 850->852 (+2), simple_open_dialog.py 500->503 (+3). Docs (PRD/release notes/userguide/ CHANGELOG) updated; HTML/EPUB artifacts regenerated. Co-Authored-By: Claude <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
|
|
||
| def test_toggle_hidden_chord_is_ctrl_h_on_windows(monkeypatch): | ||
| import quill.ui.simple_open_dialog as sod |
| def test_toggle_hidden_chord_is_cmd_shift_dot_on_macos(monkeypatch): | ||
| """#51: macOS reuses the Finder convention (Cmd+Shift+.) because Cmd+H is | ||
| the system Hide-window shortcut and would hide QUILL instead.""" | ||
| import quill.ui.simple_open_dialog as sod |
| (with a shell shebang and the executable bit), not a Windows .bat.""" | ||
| import os | ||
|
|
||
| import quill.core.persona_launcher as pl |
There was a problem hiding this comment.
Pull request overview
This PR applies a second batch of macOS-focused fixes across optional components, logging redaction, persona launchers, keyboard shortcuts, and Read Aloud process invocation, with accompanying unit tests and regenerated documentation artifacts.
Changes:
- Gate Windows-only speech components (MathCAT/DECtalk) so they aren’t offered on macOS.
- Harden diagnostics logging redaction for macOS Keychain
security -w <secret>invocations. - Improve macOS UX: Finder-launchable persona shortcuts (
.command), platform-appropriate hidden-files toggle chord, and piping long eSpeak input via--stdin.
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/ui/test_main_frame_simple_open.py | Adds unit tests for platform-aware hidden-files toggle chord detection. |
| tests/unit/core/test_read_aloud.py | Adds tests for eSpeak long-text stdin piping vs short-text argv behavior. |
| tests/unit/core/test_persona_launcher.py | Adds test coverage for macOS .command persona launcher generation and chmod. |
| tests/unit/core/test_optional_components.py | Updates/extends tests to assert MathCAT/DECtalk visibility by platform. |
| tests/stability/test_stability.py | Adds regression tests ensuring security -w <secret> is redacted in logs. |
| quill/ui/simple_open_dialog.py | Implements platform-aware hidden-files toggle chord helper used in key handlers. |
| quill/tools/module_size_budgets.json | Rebaselines module size budgets to account for small growth in touched modules. |
| quill/stability/redaction.py | Explicitly redacts the argv element following -w for the macOS security CLI. |
| quill/core/read_aloud.py | Pipes long eSpeak synthesis text via stdin to avoid command-line length overflow. |
| quill/core/persona_launcher.py | Adds macOS .command launcher generation path for personas. |
| quill/core/optional_components.py | Moves MathCAT into the existing Windows-only gate alongside DECtalk. |
| docs/user guide/userguide.md | Documents updated macOS hidden-files chord and macOS persona shortcut behavior. |
| docs/user guide/userguide.html | Regenerated HTML reflecting user guide updates. |
| docs/release notes/release0.9.0-beta2.md | Adds “macOS review batch 2” release notes section for the fixes. |
| docs/release notes/release0.9.0-beta2.html | Regenerated HTML reflecting release notes updates. |
| docs/Product Requirement Documents and Specifications/QUILL-PRD.md | Records “macOS review batch 2” changes in the PRD history section. |
| docs/Product Requirement Documents and Specifications/QUILL-PRD.html | Regenerated HTML reflecting PRD updates. |
| CHANGELOG.md | Adds a changelog entry summarizing this batch of macOS fixes. |
Comment on lines
+46
to
+52
| import os | ||
|
|
||
| argv = build_launch_argv(persona_name) | ||
| quoted = " ".join(f'"{part}"' if " " in part else part for part in argv) | ||
| safe_name = "".join(c if c.isalnum() or c in " -_" else "_" for c in persona_name).strip() | ||
| path = target_dir / f"QUILL - {safe_name}.command" | ||
| path.write_text(f"#!/bin/sh\nexec {quoted}\n", encoding="utf-8") |
| - A **Filter** dropdown with the file types the dialog will show. The default, **Supported files**, includes plain text, Markdown, HTML, and Rich Text. Switch to **Plain text**, **Markdown**, **HTML**, or **Rich Text** to narrow further, or to **All files** to see everything. | ||
| - A **Files** list of folders and files in the current directory. Folders are prefixed with `[dir]`. Use the **Up** button (or press Backspace in the list) to go to the parent folder. | ||
| - A **Hidden** toggle to show or hide files whose names start with a dot or whose Windows hidden attribute is set. **Ctrl+H** toggles this from the path field or the file list. | ||
| - A **Hidden** toggle to show or hide files whose names start with a dot or whose Windows hidden attribute is set. **Ctrl+H** toggles this from the path field or the file list (on macOS, use **Cmd+Shift+.** — the Finder convention — since Ctrl+H is the system Hide shortcut). |
Comment on lines
+615
to
+616
| list (on macOS, use <strong>Cmd+Shift+.</strong> — the Finder convention | ||
| — since Ctrl+H is the system Hide shortcut).</li> |
Comment on lines
+355
to
+356
| dialog, the hidden-files toggle was bound to Ctrl+H — which is macOS's | ||
| system Hide-window shortcut, so it hid QUILL instead. It now uses the |
| (#38)</li> | ||
| <li><strong>"Toggle hidden files" is Cmd+Shift+. on macOS.</strong> In | ||
| the Simple File Open dialog, the hidden-files toggle was bound to Ctrl+H | ||
| — which is macOS's system Hide-window shortcut, so it hid QUILL instead. |
| - **A full-codebase macOS platform review landed ~24 fixes.** Read Aloud was silent on macOS for every WAV-based engine (Piper, Kokoro, ElevenLabs, SAPI5, DECtalk) — the live playback path gated on Windows' `winsound` and silently deleted each synthesized WAV; it now plays via `afplay` on macOS. The earcon volume slider was a no-op on macOS (`NSSound` had no volume control wired); it now applies `NSSound.setVolume_()` per sound. `total_ram_gb()` returned a flat 8.0 GB on macOS, so every Mac got wrong Whisper model recommendations; it now queries `sysctl hw.memsize`. DECtalk was offered as an installable component on macOS with no working backend (its only backend is `DECtalk.dll`); it's now Windows-only, and the "Dictation (offline speech)" description no longer claims SAPI 5 works on macOS (it doesn't). Three macOS keymap collisions — `edit.replace` (Ctrl+H → Cmd+H = Hide), `edit.pop_mark` (Ctrl+M → Cmd+M = Minimize), `edit.select_chunk` (Ctrl+Space → Cmd+Space = Spotlight) — now have darwin alternates (`Cmd+Alt+F`, `Cmd+Alt+M`, `Cmd+Alt+Space`); Find Next/Previous now default to the macOS-standard `Cmd+G` / `Cmd+Shift+G`. The duplicate "About Quill" Help-menu entry is hidden on macOS (the Application menu already shows it), and the redundant `Cmd+F4` close accelerator is Windows-only. VoiceOver announcements now cap at 4096 chars (so a runaway status string isn't an unreadable wall of text) and carry an interrupt flag (routine status no longer talks over what you're hearing), and the AppKit announce path marshals off-main calls onto the main thread. Document saves and autosave snapshots are now atomic (temp + fsync + rename) so a crash mid-write can't corrupt your real document or make a truncated snapshot the recovery source. The `[macos]` packaging extra no longer pulls macOS-only build tools (`py2app`, pinned `setuptools`) on Windows/Linux. The macOS dictation "microphone unavailable" message no longer says "Windows microphone permissions" verbatim. The macOS release CI now runs the Python test suite (`pytest -m "not slow"`), so the `skipif(sys.platform != "darwin")` tests finally execute. New dependency tests guard the macOS packaging markers, and `test_high_contrast.py` now actually tests true/false behavior instead of just `isinstance(result, bool)`. | ||
| - **Fixed "Failed to get data from the clipboard (OpenClipboard Failed)" errors during cut/paste.** Reading the clipboard made exactly one attempt, so a screen reader or clipboard-history manager briefly holding it produced this error immediately. Clipboard reads now retry briefly (up to ~200ms) before giving up. | ||
| - **macOS low-hanging-fruit pass (5 fixes from the platform review):** tray minimize/restore status messages now say "menu bar" on macOS instead of "system tray" (the feature renders as a menu-bar status item there); the Settings default-folder hint shows a `/Users/...` example on macOS instead of a Windows `C:\Users\...` path; the AI Hub Engines tab's install-complete callback now guards all four post-install widget calls against a destroyed panel (an earlier fix guarded only the button re-enable, so closing the Hub mid-install could still crash on a later widget); bundled-tool relative paths use forward slashes so the bundled binary is actually found on macOS (a backslash is a literal filename character there, not a separator); and the zero-caller Windows HTML/RTF email-clipboard module is flagged as dead code with a note that a revival needs a macOS `NSPasteboard` counterpart (it degrades to plain text on macOS today). | ||
| - **macOS review batch 2 (5 more fixes):** MathCAT is no longer offered as a download on macOS (#46) — like DECtalk, its only backend is a Windows `.dll` (`libmathcat_c.dll`) that can never load on a Mac, so it's now gated Windows-only alongside DECtalk in the optional-components catalog. Short keychain secrets no longer leak into logs (#60/#73): the macOS `security` CLI takes the secret as `-w <secret>` in separate argv, and a short/non-hex secret slipped past the per-arg redaction that guards the diagnostics log; the value following `-w` is now redacted explicitly before logging. Three fixes are code-complete and unit-tested here but only show their real effect on a Mac, so they await tester results (reopenable via Help > Report a Bug): Work Persona launchers now write a Finder-launchable `.command` shell script (shebang + `exec` + chmod 0o755) on macOS instead of a useless Windows `.bat` (#38); the Simple File Open dialog's "toggle hidden files" chord is now `Cmd+Shift+.` on macOS instead of `Ctrl+H` (which is the system Hide-window shortcut) (#51); and eSpeak synthesis pipes very long input (over 8,000 chars) via `--stdin` instead of passing it as a trailing argv element that can overflow the OS command-line length (Windows ~32,767) and truncate/abort a very long Read Aloud span with no clear error (#64/#77). |
| - **Cross-platform messaging.** The dictation "microphone unavailable" message and the "Dictation (offline speech)" component description now branch for macOS instead of citing Windows-only SAPI 5 / permission paths; `total_ram_gb()` reads real RAM via `sysctl` on macOS. | ||
| - **Packaging and tests.** The `[macos]` extra's `py2app`/`setuptools<83` deps carry `sys_platform == 'darwin'` markers; a `test` job in `macos-release.yml` runs `pytest -m "not slow"` on `macos-26`; dependency tests assert macOS packaging deps are darwin-marked and Windows-only deps never appear in the `[macos]` extra; `test_high_contrast.py` now tests true/false/missing-CLI behavior rather than a bare `isinstance(bool)`. | ||
| - **macOS low-hanging-fruit pass (2026-07-09).** Five no-Mac-hardware-safe fixes from the platform review backlog: (1) tray minimize/restore status messages now say "menu bar" on macOS instead of "system tray" (the surface renders as a menu-bar status item there), via a module-level `_TRAY_NOUN` constant read once at import and applied at six `_set_status` call sites; (2) the Settings default-folder hint shows a `/Users/...` example on macOS instead of a Windows `C:\Users\...` path (the only `C:\Users` hint in the tree); (3) the AI Hub Engines tab's install-complete callback (`_after_install`) now guards all four post-install widget calls against a destroyed-panel `RuntimeError`, not just the Set Up button re-enable, so closing the Hub before a background pack install finishes is a clean no-op rather than a crash on whichever widget happened to tear down last; (4) bundled-tool relative paths (`external_tools.bundled_subpath`) use forward slashes so the bundled binary is actually found on macOS, where a backslash is a literal filename character rather than a separator; (5) the zero-caller Windows HTML/RTF email-clipboard module is flagged as dead code with a note that a revival needs a macOS `NSPasteboard` counterpart (it degrades to plain text on macOS today). #11, #12, #55, and #59 are closed; #37 is partially closed (the path-hint instance, with a broader "dialog terminology" sweep remaining); #38 and #51 are deferred pending real Mac hardware; #40's premise was false and is closed (the `Ctrl+Alt+Shift+` chords and `Alt+Shift+D` are actively routed to the AI/compare command class and `view.toggle_dark_mode` respectively). | ||
| - **macOS review batch 2 (2026-07-09).** Five more fixes from the platform review backlog, worked lowest-to-highest priority, all verifiable on this Windows dev box via unit tests: (1) **#46 closed** — MathCAT is no longer offered as a download on macOS; like DECtalk, its only backend is a Windows `.dll` (`libmathcat_c.dll`) that can never load on a Mac, so the optional-component catalog now gates it Windows-only inside the existing `if sys.platform.startswith("win"):` block alongside DECtalk (a pure platform-branch relocation; no new public surface). (2) **#60/#73 closed** — the macOS `security` CLI takes a keychain secret as `-w <secret>` in separate argv elements, and a short or non-hex secret slipped past `redact_command_arg`'s token regexes into the diagnostics log; `format_args_for_log` now redacts the value that follows `-w` explicitly before the generic per-arg pass, so a key passed to Keychain can never appear in a submitted report. Three fixes are code-complete and unit-tested here but only show their real effect on real macOS hardware, so they're closed *pending tester validation* (reopenable via Help > Report a Bug if a symptom persists): (3) **#38** — `persona_launcher.write_launch_shortcut` now writes a Finder-launchable `.command` shell script (`#!/bin/sh` + `exec` + `os.chmod 0o755`) on darwin instead of a useless Windows `.bat` (Finder runs `.command` files in Terminal, unlike `.sh`, which opens in a text editor); (4) **#51** — the Simple File Open dialog's "toggle hidden files" chord is now `Cmd+Shift+.` on macOS (the Finder convention) instead of `Ctrl+H` (which is macOS's system Hide-window shortcut and would hide QUILL), via a small platform-aware `_toggle_hidden_key_pressed` helper shared by the path-field and list key handlers; (5) **#64/#77** — `synthesize_with_espeak` pipes very long input (over 8,000 characters) to eSpeak via `--stdin` instead of passing it as a trailing argv element that can overflow the OS command-line length (Windows ~32,767) and truncate or abort a very long Read Aloud span with no clear error. The live backlog is `docs/planning/review.md`, which now carries the three tester-pending items in a dedicated reopenable section. |
Comment on lines
+14927
to
+14928
| convention) instead of <code>Ctrl+H</code> (which is macOS's system | ||
| Hide-window shortcut and would hide QUILL), via a small platform-aware |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased replacement for #934 (which auto-closed when its stacked base branch
fix/macos-low-hanging-fruitwas deleted on #933's merge). Single commit onmain; content is identical to #934. Admin-merge requested.Five no-Mac-hardware-safe fixes from the macOS platform review (
docs/planning/review.md), worked lowest-to-highest priority.Closed (fully resolvable here, unit-tested)
.dll(libmathcat_c.dll) that can never load on a Mac. Gated Windows-only inoptional_components.pyalongside DECtalk (pure platform-branch relocation; no new public surface).securityCLI passes the secret as-w <secret>in separate argv, and a short/non-hex secret slipped pastredact_command_arg's token regexes into the diagnostics log.format_args_for_lognow redacts the value following-wexplicitly before the generic per-arg pass.Closed pending tester validation (code-complete + unit-tested here; real effect only on a Mac — reopen via Help > Report a Bug if a symptom persists)
persona_launcher.write_launch_shortcutnow writes a Finder-launchable.commandshell script (#!/bin/sh+exec+os.chmod 0o755) on darwin instead of a useless.bat.Cmd+Shift+.on macOS. In the Simple File Open dialog the chord wasCtrl+H— macOS's system Hide-window shortcut. NowCmd+Shift+.(Finder convention) on darwin via a small platform-aware_toggle_hidden_key_pressedhelper; staysCtrl+Hon Windows.synthesize_with_espeakpipes input over 8,000 chars via--stdininstead of a trailing argv element that can overflow the OS command-line length (Windows ~32,767).Notes
optional_components.py850->852 (+2),simple_open_dialog.py500->503 (+3).origin/main..HEAD).review.md(local, untracked) trimmed: feat(io): implement Apple Pages IWA import with heading structure #46/build(deps): update regex requirement from >=2024.0.0 to >=2026.5.9 #60/Standardize all dialogs: native (wx.MessageDialog) or web (existing accessible WebView) — kill hand-rolled custom dialogs; fixes onboarding crash + #72 #73 removed from the open list; Import Apple Pages (.pages) documents with headings/structure (IWA parse or LibreOffice fallback) #38/build(deps): bump actions/upload-artifact from 4 to 7 #51/build(deps-dev): update mypy requirement from >=1.11 to >=2.1.0 #64/Make OpenAI-compatible endpoints a first-class model provider (base URL + key + model) #77 in a "Closed pending tester validation" section.🤖 Generated with Claude Code