diff --git a/.claude/skills/project-guardrails/SKILL.md b/.claude/skills/project-guardrails/SKILL.md index 0f9656e..80ac99a 100644 --- a/.claude/skills/project-guardrails/SKILL.md +++ b/.claude/skills/project-guardrails/SKILL.md @@ -29,7 +29,7 @@ the fast "don't" list.) - **No local models / model downloads.** Transcription is a remote AssemblyAI call. No on-device ASR/LLM, no model cache, no download UI. - Don't reintroduce a "remove filler words (um, uh, like)" directive in the - prompt — `u3-sync-pro` ignores it; it was deliberately dropped. + prompt — `universal-3-5-pro` ignores it; it was deliberately dropped. ## App shape diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 2c8095d..98e50fe 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -58,7 +58,14 @@ jobs: if [ -z "$range" ]; then echo "code=true" >>"$GITHUB_OUTPUT"; exit 0 fi - changed="$(git diff --name-only "$range")" + # `|| true` is the safety property, not tidiness: this step runs under + # `bash -e`, so a failing git diff (an unreachable base sha after a + # force-push, a fork edge case) fails the step, fails this job, and makes + # the `check` job SKIP — which GitHub reports to branch protection as a + # passing required check. The entire macOS gate would be bypassed on a + # green-looking PR. An empty `changed` falls through to code=true below, + # so failing open runs the full check instead. + changed="$(git diff --name-only "$range" || true)" echo "Changed files in $range:"; echo "$changed" # Capture non-docs lines and test emptiness (robust across grep impls). nondocs="$(printf '%s\n' "$changed" | grep -vE '^docs/' || true)" @@ -93,3 +100,41 @@ jobs: - name: Run check.sh working-directory: blurt run: scripts/check.sh + + # Always-runs summary job, so a *skipped* `check` can't be mistaken for a pass. + # GitHub reports a skipped required check to branch protection as successful, so + # anything that makes `check` skip — the `changes` filter erroring, a `needs` + # failure — silently bypasses the entire macOS gate on a green-looking PR. + # This job runs `if: always()` and fails unless `check` either succeeded or was + # skipped *by the docs-only design* (changes.outputs.code == 'false'). + # + # NOTE: this only protects the repo once branch protection requires `gate` + # instead of (or as well as) `check` — that's a repo settings change. + gate: + needs: [changes, check] + if: always() + runs-on: ubuntu-latest + steps: + - name: Assert the macOS gate ran or was intentionally skipped + env: + CHECK_RESULT: ${{ needs.check.result }} + CHANGES_RESULT: ${{ needs.changes.result }} + CODE_CHANGED: ${{ needs.changes.outputs.code }} + run: | + echo "changes=$CHANGES_RESULT code=$CODE_CHANGED check=$CHECK_RESULT" + if [ "$CHANGES_RESULT" != "success" ]; then + echo "error: the changes filter did not succeed, so the gate cannot be trusted" >&2 + exit 1 + fi + case "$CHECK_RESULT" in + success) echo "macOS gate passed" ;; + skipped) + if [ "$CODE_CHANGED" = "false" ]; then + echo "docs-only change; macOS gate intentionally skipped" + else + echo "error: check was skipped but code changed" >&2 + exit 1 + fi + ;; + *) echo "error: macOS gate result was '$CHECK_RESULT'" >&2; exit 1 ;; + esac diff --git a/AGENTS.md b/AGENTS.md index 1705b9d..d2a988b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ This file is the canonical agent guide for working with this repository. Blurt is a macOS dictation app powered by [AssemblyAI](https://www.assemblyai.com). Tap or hold a trigger key, speak, and polished text is typed into the focused app. Transcription happens in a single remote AssemblyAI Sync STT call — a per-utterance `prompt` (a transcription directive plus contextual priming built from the focused app/window/field and the user's key terms) rides along with the request, so the transcript comes back already polished (there is no separate LLM pass). The user supplies an API key. Two layers: -- **`Sources/BlurtEngine/`** — Swift Package (`swift-tools-version:6.2`, macOS 26+) owning the pipeline. Pure logic behind protocol seams, no AppKit-shell dependencies and **no external SPM dependencies** (just Foundation/Security/AVFoundation plus toolchain modules like Synchronization). +- **`Sources/BlurtEngine/`** — Swift Package (`swift-tools-version:6.2`, macOS 15+) owning the pipeline. Pure logic behind protocol seams, no AppKit-shell dependencies and **no external SPM dependencies** (just Foundation/Security/AVFoundation plus toolchain modules like Synchronization). - **`App/Blurt/`** — AppKit/SwiftUI shell (Xcode project generated by XcodeGen) that wires the engine to an overlay window, a single setup/settings window, and a hotkey. It has **no external SPM dependencies** — its only package is the local `BlurtEngine` (declared in `App/Blurt/project.yml`); the former `mxcl/AppUpdater` dependency and its in-place self-updater were removed (see [Updates](#updates)). The rebindable dictation trigger is **home-grown** — a `CGEventTap` (`DictationKeyTap`) over a pure engine state machine (`DictationKeyGate`), no package. The engine's hard no-external-dependencies rule is a **rule**; the app merely happens to carry none today. The AssemblyAI API key is stored in the macOS Keychain via `APIKeyStore` (`Sources/BlurtEngine/Config/APIKeyStore.swift`) and read by the transcriber. The injectable seam over it is `APIKeyGateway` (`Sources/BlurtEngine/Config/APIKeyGateway.swift`): `ProductionAPIKeyStore` forwards to the Keychain, and `InMemoryAPIKeyStore` keeps automated runs away from the real item. @@ -47,7 +47,7 @@ The xcodebuild post-build script (`App/Blurt/project.yml`) copies the bundle to ## Working without a macOS toolchain (remote / Linux sandboxes) -This is a macOS-only project (`platforms: [.macOS(.v26)]`, AppKit + AVFoundation). On Linux or +This is a macOS-only project (`platforms: [.macOS(.v15)]`, AppKit + AVFoundation). On Linux or web sandboxes you **cannot build, test, or run** it — `swift test`, `xcodebuild`, `xcodegen`, and Xcode's `swift-format` are all unavailable, and the engine imports AVFoundation so even the SPM package won't compile. @@ -95,15 +95,15 @@ DictationKeyTap (CGEventTap + DictationKeyGate) → AppCoordinator → Dictation KeyInjector → focused app (clipboard-paste via a synthesized Cmd-V CGEvent) ``` -**`MicCapture`** (`Sources/BlurtEngine/Audio/MicCapture.swift`) is an actor implementing `MicCaptureProtocol`. It captures with **`AVAudioRecorder`**, recording straight to a temp 16 kHz / mono / 16-bit PCM WAV — the exact geometry the Sync API wants, so `stop()` reads the file back as raw S16LE bytes (`Data`, via `AVAudioFile`'s int16 common format) with no resampling or float-conversion pass — the very blob the Sync request uploads. A **fresh recorder per session** resolves the current default input device at `record()` time; this is deliberate. A long-lived `AVAudioEngine` was used previously and its input graph bound to one device, going stale on a device switch (mic ↔ built-in) — raising `-10868` (`kAudioUnitErr_FormatNotSupported`) or capturing all-zero buffers. **Don't reintroduce an `AVAudioEngine`/`installTap` capture path.** The overlay meter (`levels`) comes from the recorder's dBFS power on a ~30 Hz timer, mapped to `0…1` by `linearLevel(fromPowerDB:)` (floored so room ambient reads as empty bars rather than a meter that never rests). `levels` and `warmUp()` are part of `MicCaptureProtocol` itself, with an empty-stream / no-op default each, so stubs conform with just `start()`/`stop()` and hosts read the meter through the seam they inject. +**`MicCapture`** (`Sources/BlurtEngine/Audio/MicCapture.swift`) is an actor implementing `MicCaptureProtocol`. It captures with **`AVAudioRecorder`**, recording straight to a temp 16 kHz / mono / 16-bit PCM WAV — the exact geometry the Sync API wants, so `stop()` reads the file back as raw S16LE bytes (`Data`, via `AVAudioFile`'s int16 common format) with no resampling or float-conversion pass — the very blob the Sync request uploads. A **fresh recorder per session** resolves the current default input device at `record()` time; this is deliberate. A long-lived `AVAudioEngine` was used previously and its input graph bound to one device, going stale on a device switch (mic ↔ built-in) — raising `-10868` (`kAudioUnitErr_FormatNotSupported`) or capturing all-zero buffers. **Don't reintroduce an `AVAudioEngine`/`installTap` capture path.** The overlay meter (`levels`) comes from the recorder's dBFS power on a ~20 Hz timer (`MicCapture.meterIntervalSeconds` — public because the pill caps its animation redraws to the same cadence and reads it from here rather than restating it), mapped to `0…1` by `linearLevel(fromPowerDB:)` (floored so room ambient reads as empty bars rather than a meter that never rests). `levels` and `warmUp()` are part of `MicCaptureProtocol` itself, with an empty-stream / no-op default each, so stubs conform with just `start()`/`stop()` and hosts read the meter through the seam they inject. -**`AssemblyAITranscriber`** (`Sources/BlurtEngine/STT/AssemblyAITranscriber.swift`) implements `TranscriberProtocol` against AssemblyAI's **Sync** STT API (the endpoint `assembly dictate` uses): a single `POST https://sync.assemblyai.com/transcribe` with the captured audio as a raw S16LE PCM blob (exactly the bytes `MicCapture.stop()` returned — there is no encoding pass) in the `audio` multipart part plus a JSON `config` part (`sample_rate`, `channels`, and a `prompt`), `X-AAI-Model: u3-sync-pro`. The `prompt` (built per utterance by `TranscriptionPrompt.build(context:)`) is prepended to the model's system prompt, so the transcript that comes back is **already polished** — there is no separate LLM post-processing step. The finished transcript comes back in the response body — no `/v2/upload`, no job submission, no polling. Truly synchronous: `TranscriberProtocol.transcribe(pcm:sampleRate:)` is a single `async throws -> String` that returns the whole transcript at once (no streaming/deltas). The sync model handles audio from ~80 ms up to 120 s (server-side ~30 s inference deadline); those limits live in `SyncSTTLimits` and back `DictationSession`'s auto-release timeout (so a held hotkey stops before the cap). +**`AssemblyAITranscriber`** (`Sources/BlurtEngine/STT/AssemblyAITranscriber.swift`) implements `TranscriberProtocol` against AssemblyAI's **Sync** STT API (the endpoint `assembly dictate` uses): a single `POST https://sync.assemblyai.com/transcribe` with the captured audio as a raw S16LE PCM blob (exactly the bytes `MicCapture.stop()` returned — there is no encoding pass) in the `audio` multipart part plus a JSON `config` part (`sample_rate`, `channels`, and a `prompt`), `X-AAI-Model: universal-3-5-pro`. The `prompt` (built per utterance by `TranscriptionPrompt.build(context:)`) is prepended to the model's system prompt, so the transcript that comes back is **already polished** — there is no separate LLM post-processing step. The finished transcript comes back in the response body — no `/v2/upload`, no job submission, no polling. Truly synchronous: `TranscriberProtocol.transcribe(pcm:sampleRate:)` is a single `async throws -> String` that returns the whole transcript at once (no streaming/deltas). The sync model handles audio from ~80 ms up to 120 s (server-side ~30 s inference deadline); those limits live in `SyncSTTLimits` and back `DictationSession`'s auto-release timeout (so a held hotkey stops before the cap). -**`DictationSession`** (`Sources/BlurtEngine/Pipeline/DictationSession.swift`, with the command surface in `+Commands.swift`, the phase stream / signposts in `+Observation.swift`, and the post-release transcribe→inject pipeline in `+Pipeline.swift`) is the central actor. It exposes `press()` / `release()` / `cancel()` / `cancelRecording()`, a synchronous fire-and-forget `submit(_: Command)` mirroring those four for callback-shaped hosts (commands run in exact emit order — the tap wires straight into it, no per-callback `Task` spawning), and a `phase: PipelinePhase` (`idle | recording | transcribing | injecting | failed | cancelled`). A host-supplied `readinessCheck` closure can refuse a press before any capture begins (the app passes a key-presence check, so a missing API key surfaces as `.failed(.apiKeyMissing)` at press time — never after the user has spoken). Its three collaborators are protocol-typed (`MicCaptureProtocol`, `TranscriberProtocol`, `InjectorProtocol`) so tests can stub them — see `Tests/BlurtEngineTests/Stubs/`. STT errors that are already `BlurtError` (e.g. `.apiKeyMissing`) surface verbatim; other errors are wrapped in `.sttFailed`. The pipeline is just transcribe → inject (the cleanup happens server-side), and an empty transcript returns to `.idle` without injecting. Two perceived-latency choices to preserve: `release()` claims `.transcribing` _before_ `mic.stop()` (the stop chime and pill switch fire at key-up, not after the recording is read back — this ordering also closes the double-release window `ReleaseRaceTests` pins), and the press-time AX context read is consumed with a bounded wait (`contextWaitBudget`, 500 ms) so an unresponsive frontmost app costs the transcript its priming, never a multi-second stall. +**`DictationSession`** (`Sources/BlurtEngine/Pipeline/DictationSession.swift`, with the command surface in `+Commands.swift`, the phase stream / signposts in `+Observation.swift`, and the post-release transcribe→inject pipeline in `+Pipeline.swift`) is the central actor. It exposes `press()` / `release()` / `cancel()` / `cancelRecording()`, a synchronous fire-and-forget `submit(_: Command)` mirroring those four for callback-shaped hosts (commands run in exact emit order — the tap wires straight into it, no per-callback `Task` spawning), and a `phase: PipelinePhase` (`idle | recording | transcribing | injecting | failed | cancelled`). A host-supplied `readinessCheck` closure can refuse a press before any capture begins (the app passes a key-presence check, so a missing API key surfaces as `.failed(.apiKeyMissing)` at press time — never after the user has spoken). Its three collaborators are protocol-typed (`MicCaptureProtocol`, `TranscriberProtocol`, `InjectorProtocol`) so tests can stub them — see `Tests/BlurtEngineTests/Stubs/`. STT errors that are already `BlurtError` (e.g. `.apiKeyMissing`) surface verbatim; other errors are wrapped in `.sttFailed`. The pipeline is just transcribe → inject (the cleanup happens server-side), and an empty transcript returns to `.idle` without injecting. Three perceived-latency choices to preserve: `.injecting` projects to `OverlayUIState.processing`, not `.idle` — the shell reads an idle projection as "dismiss", so mapping this working phase to idle faded the pill out mid-dictation and blinked it back for "Pasted"; `release()` claims `.transcribing` _before_ `mic.stop()` (the stop chime and pill switch fire at key-up, not after the recording is read back — this ordering also closes the double-release window `ReleaseRaceTests` pins), and the press-time AX context read is consumed with a bounded wait (`contextWaitBudget`, 500 ms) so an unresponsive frontmost app costs the transcript its priming, never a multi-second stall. -**`KeyInjector.insert`** always injects via clipboard-paste: it saves the current pasteboard contents, writes the transcript, posts Cmd-V (`CGEvent` to the HID event tap), waits `pasteSettleNanos` (default 400 ms) for the target to read the clipboard, then clears and restores the prior pasteboard contents. It first activates the captured target app; if that app has terminated or won't activate, it leaves the transcript on the clipboard and throws `.targetAppLost`, which the pipeline degrades to the quiet "copied" notice (`.noTarget`) rather than a red failure. It joins consecutive dictations with a leading separator space via `withLeadingSeparator`. There is no keystroke-by-keystroke typing path and no length threshold — every insert pastes. +**`KeyInjector.insert`** always injects via clipboard-paste: it saves the current pasteboard contents, writes the transcript, then posts Cmd-V (`CGEvent` to the `.cgAnnotatedSessionEventTap`, not the HID tap). `insert` returns once the paste is posted; the wait for the target to read the clipboard (`pasteSettleDuration`, default 400 ms) and the restore of the prior pasteboard contents run on a chained follow-up task (`pendingSettle`), which also serializes back-to-back inserts so a second paste can't snapshot the first one's clipboard. The restore is deliberately conservative: `SystemClipboard.snapshot()` returns `nil` when the pasteboard can't be read at all (distinct from it being _empty_), and `restore` builds the replacement items before clearing, so a snapshot it can't reproduce leaves the clipboard alone instead of emptying it. Promised (lazily provided) representations can't be copied, so a restore of those degrades to the plain-text floor rather than a byte-faithful round trip. It first activates the captured target app; if that app has terminated or won't activate, it leaves the transcript on the clipboard and throws `.targetAppLost`, which the pipeline degrades to the quiet "copied" notice (`.noTarget`) rather than a red failure. It joins consecutive dictations with a leading separator space via `withLeadingSeparator`. There is no keystroke-by-keystroke typing path and no length threshold — every insert pastes. -**`AppCoordinator`** (`App/Blurt/Blurt/AppCoordinator.swift`) is the only place the engine is composed for the real app. It builds concrete instances and owns a `DictationKeyTap` whose `onStart` → `session.submit(.press)`, `onStop` → `session.submit(.release)`, `onCancel` → `session.submit(.cancel)`, and `onRecordingDiscarded` → `session.submit(.cancelRecording)`, then observes `session.phaseStream()` to drive the overlay (routing `.failed(.apiKeyMissing)` to the settings window instead of an error flash). It also owns `hasAPIKey` (drives the wizard/Settings UI; the press-time gate itself is the session's `readinessCheck`) and `saveAPIKey`. It calls `keyTap.ensureRunning()` to install/enable the tap (returns false until the process is Accessibility-trusted, so it retries once permissions land) and `keyTap.refreshBinding()` after the user picks a different trigger key. +**`AppCoordinator`** (`App/Blurt/Blurt/AppCoordinator.swift`) is the only place the engine is composed for the real app. It builds concrete instances and owns a `DictationKeyTap` whose `onStart` → `session.submit(.press)`, `onStop` → `session.submit(.release)`, `onCancel` → `session.submit(.cancel)`, and `onRecordingDiscarded` → `session.submit(.cancelRecording)`, then observes `session.phaseStream()` to drive the overlay (routing any `PipelinePhase.setupBlocker` — today a missing API key — to the settings window instead of an error flash, via its `onSetupBlocked` closure; which failures count as unfinished _setup_ rather than faults is the engine's single classification, so the shell's navigation and the pill's calm-idle projection can't disagree). It also owns `hasAPIKey` (drives the wizard/Settings UI; the press-time gate itself is the session's `readinessCheck`) and `saveAPIKey`. It calls `keyTap.ensureRunning()` to install/enable the tap (returns false until the process is Accessibility-trusted, so it retries once permissions land) and `keyTap.refreshBinding()` after the user picks a different trigger key. ## Hotkey @@ -114,13 +114,13 @@ The dictation trigger is a **single lone modifier key** (tap-to-toggle or hold-t - **`DictationKeyGate`** (`Sources/BlurtEngine/Hotkey/DictationKeyGate.swift`) — pure, clock-free state machine (`idle`/`armed`/`latched`) that turns modifier-down/up and other-key-down into `start`/`stop`/`cancel`/`none`. Recording starts the instant the modifier goes down; on key-up a release ≥ `holdThreshold` (default 1 s) is a **hold** (push-to-talk → stop) while a shorter release **latches** (tap-to-toggle; next tap stops). A combo (modifier + another key, e.g. ⌘C) from idle cancels the fresh capture; over a latched recording it passes through as a normal shortcut. Callers pass monotonic timestamps so every decision is deterministic and unit-tested. - **`DictationKeyRouter`** (`Sources/BlurtEngine/Hotkey/DictationKeyRouter.swift`) — the event-routing layer over the gate: only the bound keycode's flag changes drive the modifier, only genuine down/up **edges** reach the gate (`flagsChanged` deliveries re-report the bit whether or not it changed, so a repeat must not double-fire), and `reset()`/`rebind(triggerKeyCode:)` report whether they discarded a live recording the host must cancel upstream. Unit-tested (`DictationKeyRouterTests`). -The app side, **`DictationKeyTap`** (`App/Blurt/Blurt/Hotkey/DictationKeyTap.swift`), reduces each `CGEventTap` delivery (watching `flagsChanged` for the bound modifier and `keyDown` for any other key) to a `DictationKeyRouter.Event` and owns the tap lifecycle. It **swallows nothing** — a lone modifier types nothing, and combos pass through so normal shortcuts keep working — unlike the old chord trigger. +The app side, **`DictationKeyTap`** (`App/Blurt/Blurt/Hotkey/DictationKeyTap.swift`), reduces each `CGEventTap` delivery (watching `flagsChanged` for the bound modifier and `keyDown` for any other key) to a `DictationKeyRouter.Event` and owns the tap lifecycle. `AppCoordinator` calls its `syncAfterTerminalPhase()` on every terminal phase: a dictation can end with no key event to close the gate (the auto-release cap, or a refused/failed press), which would leave the gate `.latched` and silently swallow the user's next press entirely — a latched `modifierDown` returns `.none`, and the `modifierUp` after it returns `.stop`, which no-ops on an already-terminal session. It **swallows nothing** — a lone modifier types nothing, and combos pass through so normal shortcuts keep working — unlike the old chord trigger. The trigger is editable in the Shortcut section of the setup/settings window (`HotkeyStepView`), which is a `Picker` over `TriggerKey.allCases` that writes `TriggerKeyStore` (then `AppCoordinator.dictationBindingChanged()` re-reads it into the tap). For display strings, use `TriggerKeyStore().triggerKey.label` for one-shot reads, or `@AppStorage(TriggerKeyStore.defaultsKey)` + `TriggerKey.fromPersisted` in views that must re-render live on a Settings change. ## Transcription prompt -`Sources/BlurtEngine/STT/TranscriptionPrompt.swift` builds the instruction passed as the Sync STT request's `config.prompt` (see `AssemblyAITranscriber`). It's unit-tested in `Tests/BlurtEngineTests/TranscriptionPromptTests.swift`. Every built prompt opens with the fixed `baseInstruction` — `"Transcribe without speaker labels, audio event descriptions, or emotion markers."` — a negative-exclusion clause that suppresses the annotation markers (`[Speaker]`, `[door creaks]`, `[laughing]`) the model would otherwise paste into the user's text. There is deliberately **no** language directive: pinning the prompt to English hurt non-English transcription, so language is left to the model's own detection — don't reintroduce a "Transcribe in English"-style clause. `build(context:)` wraps that pivot in _contextual priming_: prior-cursor text, the selected text (the highlighted run the dictation will replace, so the model is primed on what's being rewritten — read via `kAXSelectedTextAttribute` and skipped in secure fields), a topic hint from the window title, a destination sentence from the app/field, and inline keyword boosting from the user's key terms. It's phrased per AssemblyAI's Universal-3 Pro prompting guidance (positive/authoritative wording, no "Don't"/"Avoid"/"Never") and stays under the API's 4096-character cap. Note: a "remove filler words (um, uh, like)" _content_ directive is **not** in the model's trained instruction set, so it's a no-op and was deliberately dropped — don't reintroduce it. `build(context:)` returns `nil` when there's no usable context, and passing `prompt: nil` to the transcriber omits the field so the server applies its own default prompt. +`Sources/BlurtEngine/STT/TranscriptionPrompt.swift` builds the instruction passed as the Sync STT request's `config.prompt` (see `AssemblyAITranscriber`). It's unit-tested in `Tests/BlurtEngineTests/TranscriptionPromptTests.swift`. Every built prompt opens with the fixed `baseInstruction` — `"Transcribe without speaker labels, audio event descriptions, or emotion markers."` — a negative-exclusion clause that suppresses the annotation markers (`[Speaker]`, `[door creaks]`, `[laughing]`) the model would otherwise paste into the user's text. There is deliberately **no** language directive: pinning the prompt to English hurt non-English transcription, so language is left to the model's own detection — don't reintroduce a "Transcribe in English"-style clause. `build(context:)` wraps that pivot in _contextual priming_: prior-cursor text, the selected text (the highlighted run the dictation will replace, so the model is primed on what's being rewritten — read via `kAXSelectedTextAttribute` and skipped in secure fields — detected by AX role **or** subrole, failing closed when the role can't be read, so a password can't reach the prompt), a topic hint from the window title, a destination sentence from the app/field, and inline keyword boosting from the user's key terms. It's phrased per AssemblyAI's Universal-3 Pro prompting guidance (positive/authoritative wording, no "Don't"/"Avoid"/"Never") and stays under a self-imposed 4096-character ceiling (`characterCap` — the Sync API reference documents no cap on `config.prompt`; the 4096 figure it does document belongs to `conversation_context`, where over-cap content is trimmed rather than rejected). Note: a "remove filler words (um, uh, like)" _content_ directive is **not** in the model's trained instruction set, so it's a no-op and was deliberately dropped — don't reintroduce it. `build(context:)` returns `nil` when there's no usable context, and passing `prompt: nil` to the transcriber omits the field so the server applies its own default prompt. ## Conventions @@ -136,12 +136,13 @@ Engine logic is covered by the Swift Testing unit suites above; the **app shell* the user drives it) is covered by an XCUITest bundle, **`App/Blurt/BlurtUITests/`**. It's a `bundle.ui-testing` target declared in `project.yml` and wired into the `Blurt` scheme's test action, so it runs via `xcodebuild test` — see -**`scripts/uitest.sh`** (or `RUN_UI_TESTS=1 scripts/check.sh`). It's kept out of -the default required gate because it needs a GUI session. +**`scripts/uitest.sh`**, which `scripts/check.sh` also invokes as part of the +required gate (so it needs a GUI session — it is skipped only by `--portable`). The suite drives the **real app** against deterministic, offline doubles rather than mocking the UI. A `-BlurtUITest` launch argument flips on a Debug-only -harness (`App/Blurt/Blurt/UITestSupport.swift`, all `#if DEBUG`) that: +harness (`App/Blurt/Blurt/UITestSupport.swift`, all `#if UITEST_HOOKS` — defined for the +Debug config, so `scripts/dev-build.sh` can strip it) that: - swaps the mic / transcriber / injector for stubs and the Keychain key store for an in-memory one (so no microphone, network, Accessibility, or real Keychain — diff --git a/App/Blurt/Blurt/AppCoordinator+UITesting.swift b/App/Blurt/Blurt/AppCoordinator+UITesting.swift index f9dc7f1..06d33e6 100644 --- a/App/Blurt/Blurt/AppCoordinator+UITesting.swift +++ b/App/Blurt/Blurt/AppCoordinator+UITesting.swift @@ -4,8 +4,8 @@ // XCUITest can't synthesize the lone right-modifier `flagsChanged` event the // real trigger relies on — and the `CGEventTap` that reads it needs the // process to be Accessibility-trusted, which the CI test host isn't. The test - // harness window instead calls `beginDictation`/`endDictation`/ - // `cancelDictation`, which drive the *same* `DictationSession` (and its + // harness window instead calls `session.press()`/`release()`/`cancel()` + // directly, driving the *same* `DictationSession` (and its // press-time missing-key readiness check) the key tap's closures reach via // `session.submit(_:)`. Compiled only in Debug, so nothing here ships in the // notarized Release build. diff --git a/App/Blurt/Blurt/AppCoordinator.swift b/App/Blurt/Blurt/AppCoordinator.swift index 9dc92e9..a04a37b 100644 --- a/App/Blurt/Blurt/AppCoordinator.swift +++ b/App/Blurt/Blurt/AppCoordinator.swift @@ -8,11 +8,12 @@ final class AppCoordinator { /// so the panel and its SwiftUI host aren't built until the app is fully /// configured and the pill is about to appear. Stays nil through onboarding. private var overlay: OverlayWindowController? - /// Invoked when the user triggers dictation without a saved API key. The app - /// shell wires this to bring the setup/settings window forward so the user can - /// add a key — the actionable fix — rather than flashing a message that - /// disappears. - let onMissingAPIKey: @MainActor () -> Void + /// Invoked when a press is refused because setup isn't finished — today a + /// missing API key, and whatever else the engine classifies as a + /// `PipelinePhase.setupBlocker`. The app shell wires this to bring the + /// setup/settings window forward so the user lands on the actionable fix rather + /// than seeing a message that disappears. + let onSetupBlocked: @MainActor () -> Void let session: DictationSession /// The mic seam, kept beyond session construction for its two side features — @@ -50,11 +51,11 @@ final class AppCoordinator { /// in-memory store with an offline validator — the engine's `APIKeySubmission` /// still owns the never-persist-an-unverified-key invariant either way. init( - onMissingAPIKey: @escaping @MainActor () -> Void, + onSetupBlocked: @escaping @MainActor () -> Void, components: DictationComponents = .production(), apiKey: APIKeyModel = APIKeyModel() ) { - self.onMissingAPIKey = onMissingAPIKey + self.onSetupBlocked = onSetupBlocked self.apiKey = apiKey // Unbounded: the Recent list is append-only, so every transcript must survive @@ -109,7 +110,7 @@ final class AppCoordinator { /// `submit(_:)` command feed (see its doc for the FIFO-ordering guarantee /// that rules out spawning a `Task {}` per callback). Drives the /// hold-to-dictate hotkey from a CGEventTap (see `DictationKeyTap`) rather - /// than a Carbon global hotkey: the latter leaks the chord's auto-repeat key + /// than a Carbon global hotkey: the latter leaks the trigger's auto-repeat key /// events into the focused app while held. private func startDictationDriver() { let session = session @@ -191,33 +192,10 @@ final class AppCoordinator { overlay?.hide() } - // MARK: - Dictation drivers - // - // The await-able begin/end/cancel path used by the UI-test harness - // (`UITestSupport`). The key tap drives the same `DictationSession` through - // its synchronous `submit(_:)` feed, so both paths hit the same engine - // guards and race rules. - - /// Begins a dictation as the hotkey would (the missing-key gate is the - /// session's `readinessCheck`; `render(_:)` routes the refusal to Settings). - func beginDictation() async { - await session.press() - } - - /// Stops recording and runs transcribe→inject. - func endDictation() async { - await session.release() - } - - /// Abandons the in-flight dictation. - func cancelDictation() async { - await session.cancel() - } - - /// Called when the user rebinds (or clears) the dictation shortcut in the - /// recorder, so the event tap starts matching the new chord. The shortcut no - /// longer gates readiness (it has a default and lives in Settings), so there's - /// nothing else to re-evaluate here. + /// Called when the user rebinds the dictation trigger in the Shortcut picker, + /// so the event tap starts matching the new key. The shortcut no longer gates + /// readiness (it has a default and lives in Settings), so there's nothing else + /// to re-evaluate here. func dictationBindingChanged() { keyTap?.refreshBinding() } @@ -234,12 +212,13 @@ final class AppCoordinator { } private func render(_ phase: PipelinePhase) { - // A missing key is a setup state, not a fault: the engine projections - // below render it as calm idle (no red flash) and Monitoring ignores it — - // the only app-level part is the navigation side effect, bringing the - // settings window forward so the user lands on the fix. - if case .failed(.apiKeyMissing) = phase { - onMissingAPIKey() + // A setup blocker is a state, not a fault: the engine projections below + // render it as calm idle (no red flash) and the menu bar ignores it — the only + // app-level part is the navigation side effect, bringing the settings window + // forward so the user lands on the fix. Which failures count as setup is the + // engine's call (`PipelinePhase.setupBlocker`), not re-derived here. + if phase.setupBlocker != nil { + onSetupBlocked() } // Reveal the pill first, then fire the cue: the sound must never sit in // front of the visual state change. Pure phase→pill mapping lives in the @@ -251,5 +230,15 @@ final class AppCoordinator { menuBarStatus = phase.menuBarStatus cues.transition(for: phase) + + // A dictation that ended without a key event (auto-release cap, a refused or + // failed press) leaves the trigger's gate latched, which would swallow the + // user's next press entirely. Clearing it here — the one place that sees every + // phase — keeps the tap's state honest without the gate needing to know about + // pipeline phases. No-op whenever the gate is already idle, which is every + // normal flow. + if phase.isTerminal { + keyTap?.syncAfterTerminalPhase() + } } } diff --git a/App/Blurt/Blurt/AppDelegate.swift b/App/Blurt/Blurt/AppDelegate.swift index a42d081..2dcd47c 100644 --- a/App/Blurt/Blurt/AppDelegate.swift +++ b/App/Blurt/Blurt/AppDelegate.swift @@ -95,7 +95,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { // launch), so it's set by the time the hotkey fires. // The overlay pill isn't built here — `AppCoordinator` creates it lazily in // `showOverlay()` once the app is fully configured. - let onMissingAPIKey: @MainActor () -> Void = { [weak self] in self?.surfaceMainWindow() } + let onSetupBlocked: @MainActor () -> Void = { [weak self] in self?.surfaceMainWindow() } let coord: AppCoordinator #if UITEST_HOOKS // Under UI testing, compose the app with offline stub collaborators and an @@ -114,7 +114,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { defaults.removeObject(forKey: key) } coord = AppCoordinator( - onMissingAPIKey: onMissingAPIKey, + onSetupBlocked: onSetupBlocked, components: .uiTest(), apiKey: APIKeyModel( keyStore: InMemoryAPIKeyStore(), @@ -124,10 +124,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate { // `lazy` default is ever read (first check), so it replaces it cleanly. updateCheckModel = .uiTest() } else { - coord = AppCoordinator(onMissingAPIKey: onMissingAPIKey) + coord = AppCoordinator(onSetupBlocked: onSetupBlocked) } #else - coord = AppCoordinator(onMissingAPIKey: onMissingAPIKey) + coord = AppCoordinator(onSetupBlocked: onSetupBlocked) #endif self.coordinator = coord diff --git a/App/Blurt/Blurt/Hotkey/DictationKeyTap.swift b/App/Blurt/Blurt/Hotkey/DictationKeyTap.swift index 5ec2e91..4b36f58 100644 --- a/App/Blurt/Blurt/Hotkey/DictationKeyTap.swift +++ b/App/Blurt/Blurt/Hotkey/DictationKeyTap.swift @@ -115,6 +115,40 @@ final class DictationKeyTap { return true } + /// Discard stale gate state after the session reached a terminal phase, so the + /// user's next trigger press isn't swallowed. + /// + /// A dictation can end *without* a key event ending it: the auto-release cap + /// fires on a long hold, or the press is refused (no API key) / fails (no input + /// device). The gate is then left `.latched` — or `.armed`, which a quick release + /// promotes to `.latched` — and nothing can clear it, because + /// `DictationKeyGate.modifierDown` from `.latched` returns `.none` (so the next + /// tap starts nothing) and the `modifierUp` after it returns `.stop`, which + /// no-ops on an already-terminal session. The user gets one completely dead + /// press: no pill, no chime, nothing — and if that press was a hold, a whole + /// utterance spoken into a session that never started. + /// + /// Resets unconditionally, unlike the disabled-tap recovery above. That guard + /// exists to preserve a *live* recording whose key events were dropped; here the + /// dictation is already over, so there is no state worth keeping even if the + /// trigger is still physically held — a later key-up just finds + /// `modifierIsDown == false` and routes to `.none`. + /// + /// Acts on `reset()`'s discarded-recording result the same way `refreshBinding` + /// does. In the stale case the session is already terminal, so the + /// `cancelRecording` is a no-op. It matters for the one residual race: `render` + /// consumes `phaseStream()` asynchronously, so if that loop falls behind, + /// dictation N's terminal phase can arrive *after* the gate has armed for + /// dictation N+1 — the reset then clears N+1's live state, its key-up routes to + /// `.none`, and the recording would otherwise run to the ~115 s auto-release cap. + /// Cancelling it turns a silent two-minute hang into a clean stop. + /// + /// A no-op in every normal flow, where the gate is already idle by the time a + /// terminal phase lands. + func syncAfterTerminalPhase() { + if router.reset() { onRecordingDiscarded() } + } + /// Re-read the bound trigger key into the router. Call after the user /// rebinds. The router's reset reports a discarded live recording: rebinding /// mid-dictation means the old key's up-event will never match, so the capture diff --git a/App/Blurt/Blurt/Overlay/OverlayView.swift b/App/Blurt/Blurt/Overlay/OverlayView.swift index a00832e..8ecea75 100644 --- a/App/Blurt/Blurt/Overlay/OverlayView.swift +++ b/App/Blurt/Blurt/Overlay/OverlayView.swift @@ -9,7 +9,7 @@ private enum OverlayBrandPalette { struct OverlayView: View { // The single source of pill state. The live mic level is *not* read in this // body: that would rebuild the whole pill (capsule, shadow, REC tag) on - // every ~30 Hz meter tick. Only `bridge.state` is read here (via `state` + // every ~20 Hz meter tick. Only `bridge.state` is read here (via `state` // below); the leaf bar view (`WaveformBarsLevel`) reads `bridge.level`, so // @Observable confines the per-tick invalidation to the bars — the rest of // the pill stays stable. @@ -176,6 +176,34 @@ private struct StatusLineText: View { } } +/// Redraw cap for the pill's continuous animations — the REC dot's pulse, the +/// "Transcribing…" breath, and the waveform's idle wave. +/// +/// Read from the engine rather than restated: the cap exists *because* of the mic +/// meter's cadence (the level feed and these slow sines can't show anything +/// faster, so rendering at the display's full refresh rate — up to 120 Hz on +/// ProMotion — would only burn energy), so it tracks that one number instead of +/// duplicating it behind a comment that could go stale. +private let overlayAnimationInterval = MicCapture.meterIntervalSeconds + +extension View { + /// Raised-cosine opacity breathing over `period`: 1 → `minOpacity` → 1, so the + /// view eases through the dim point instead of bouncing off it. Shared by the + /// REC dot and the "Transcribing…" label so the pill's two heartbeats stay one + /// curve. With `animated` false (Reduce Motion) the view renders untouched. + @ViewBuilder + func pulsingOpacity(period: Double, minOpacity: Double, animated: Bool) -> some View { + if animated { + TimelineView(.animation(minimumInterval: overlayAnimationInterval)) { timeline in + let osc = (cos(timeline.date.timeIntervalSinceReferenceDate / period * 2 * .pi) + 1) / 2 // 0...1 + self.opacity(minOpacity + (1 - minOpacity) * osc) + } + } else { + self + } + } +} + /// The "Transcribing…" status line with a slow breathing pulse — the processing /// counterpart of the recording bars' idle shimmer, so the pill keeps visibly /// working while the app waits on the Sync API and pastes the result. Driven by @@ -195,16 +223,7 @@ private struct TranscribingLabel: View { private let minOpacity: Double = 0.55 var body: some View { - if animated { - // ~20 Hz is plenty for a 1.8 s opacity ramp — rendering at the display's - // full refresh rate would only burn energy (same reasoning as the 30 Hz - // cap on WaveformBars). - TimelineView(.animation(minimumInterval: 1.0 / 20.0)) { timeline in - label.opacity(breathOpacity(at: timeline.date.timeIntervalSinceReferenceDate)) - } - } else { - label - } + label.pulsingOpacity(period: breathPeriod, minOpacity: minOpacity, animated: animated) } // Shared with the "Pasted" notice (OverlayView's `.pasted` case) so the @@ -212,13 +231,6 @@ private struct TranscribingLabel: View { private var label: some View { StatusLineText("Transcribing…") } - - /// Raised cosine over `breathPeriod`: 1 → `minOpacity` → 1, so the label - /// eases through the dim point instead of bouncing off it. - private func breathOpacity(at time: TimeInterval) -> Double { - let osc = (cos(time / breathPeriod * 2 * .pi) + 1) / 2 // 0...1 - return minOpacity + (1 - minOpacity) * osc - } } /// The "● REC" recording tag: a pulsing magenta dot + "REC" caption, sitting to @@ -250,15 +262,8 @@ private struct RecordingTag: View { } /// The magenta record dot, breathing while recording. - @ViewBuilder private var dot: some View { - if animated { - // ~20 Hz is plenty for a 1.2 s opacity ramp (same cap as WaveformBars). - TimelineView(.animation(minimumInterval: 1.0 / 20.0)) { timeline in - circle.opacity(pulseOpacity(at: timeline.date.timeIntervalSinceReferenceDate)) - } - } else { - circle - } + private var dot: some View { + circle.pulsingOpacity(period: pulsePeriod, minOpacity: minOpacity, animated: animated) } private var circle: some View { @@ -266,16 +271,9 @@ private struct RecordingTag: View { .fill(OverlayBrandPalette.magenta) .frame(width: 5, height: 5) } - - /// Raised cosine over `pulsePeriod`: 1 → `minOpacity` → 1, easing through the - /// dim point instead of bouncing off it (matches TranscribingLabel). - private func pulseOpacity(at time: TimeInterval) -> Double { - let osc = (cos(time / pulsePeriod * 2 * .pi) + 1) / 2 // 0...1 - return minOpacity + (1 - minOpacity) * osc - } } -/// The only view that reads `bridge.level`, so @Observable scopes the ~30 Hz +/// The only view that reads `bridge.level`, so @Observable scopes the ~20 Hz /// meter invalidation to this leaf (and its `WaveformBars` child) instead of the /// enclosing `OverlayView`. `WaveformBars` stays a pure value view — easy to /// reason about and drive from a fixed level — with the observation isolated @@ -331,11 +329,8 @@ private struct WaveformBars: View { Group { if animated { // Continuous clock so the idle breathing is smooth and never depends - // on a one-shot state toggle. Capped at ~20 Hz to match the mic meter - // (MicCapture.meterInterval) — the level feed and the slow breathing - // sine can't show anything faster, so rendering at the display's full - // refresh rate (up to 120 Hz on ProMotion) would only burn energy. - TimelineView(.animation(minimumInterval: 1.0 / 20.0)) { timeline in + // on a one-shot state toggle; capped at `overlayAnimationInterval`. + TimelineView(.animation(minimumInterval: overlayAnimationInterval)) { timeline in row(count: count, maxBarHeight: maxBarHeight, time: timeline.date.timeIntervalSinceReferenceDate) } } else { @@ -361,7 +356,7 @@ private struct WaveformBars: View { let weight = envelopeWeight(index, count: count) // Voice-driven height: the current level, gamma-shaped, scaled by this bar's // envelope weight so the middle leads. - let voice = pow(CGFloat(max(0, min(1, level))), levelGamma) * weight + let voice = pow(CGFloat(level), levelGamma) * weight var breath: CGFloat = 0 // Full breathing when quiet, fading to none once the voice is moderate. Once // it's faded out (voice ≥ 0.25) the per-bar sine below would multiply to ~0, diff --git a/App/Blurt/Blurt/Overlay/OverlayWindowController.swift b/App/Blurt/Blurt/Overlay/OverlayWindowController.swift index 8b34fb8..0e9ec73 100644 --- a/App/Blurt/Blurt/Overlay/OverlayWindowController.swift +++ b/App/Blurt/Blurt/Overlay/OverlayWindowController.swift @@ -11,16 +11,21 @@ final class OverlayBridge { var level: Float = 0 func pushLevel(_ value: Float) { - // `value` is already a fixed 0...1 scale from the recorder's dBFS meter - // (MicCapture.linearLevel) — store the latest as-is. (No auto-gain - // normalizing to a running peak: that stretched sustained speech to full.) - level = min(1, max(0, value)) + // `value` arrives on the fixed 0...1 scale `MicCaptureProtocol.levels` + // documents (MicCapture.linearLevel). Clamp once here — the single seam where + // any capture implementation crosses into the view layer — so the bars can + // trust the range instead of re-checking it. (No auto-gain normalizing to a + // running peak: that stretched sustained speech to full.) + let clamped = min(1, max(0, value)) + // @Observable invalidates on assignment, not on change, and `linearLevel` is + // floored so room ambient maps to exactly 0 — without this guard every silent + // tick would rebuild the whole bar row 20×/s with an unchanged value. + guard clamped != level else { return } + level = clamped } } final class OverlayWindowController { - private static let customOriginXKey = "BlurtOverlayCustomOriginX" - private static let customOriginYKey = "BlurtOverlayCustomOriginY" // The panel is sized larger than the visible pill so SwiftUI's drop shadow // (see `OverlayView`'s `.shadow`, which documents staying within // `shadowMargin`) has room to render without being clipped by the window's @@ -136,9 +141,15 @@ final class OverlayWindowController { } /// Hides the pill immediately, without the fade. Called when the app drops out - /// of its fully-configured state (a permission revoked, the key cleared, the - /// shortcut unbound) so the pill is only ever on screen while dictation can - /// actually work. + /// of its fully-configured state (a permission revoked, the key cleared). + /// + /// This hides the pill; it does **not** disarm the trigger — the `CGEventTap` + /// stays installed, so a press while not-ready still runs and the pill comes back + /// to report the failure. That's deliberate: tearing the tap down would depend on + /// `ensureRunning()` succeeding again to restore dictation, and a tap that failed + /// to reinstall is a far worse failure than an error flash. `WizardController` + /// surfaces the setup window on the same not-ready edge, which is what actually + /// routes the user to the fix. func hide() { errorRevertTask?.cancel() errorRevertTask = nil @@ -155,6 +166,14 @@ final class OverlayWindowController { panel.orderOut(nil) panel.alphaValue = 1 bridge.state = .idle + // Clear the level too, or the pill's next appearance renders its bars at the + // PREVIOUS dictation's loudness until the first new meter tick (~50 ms) + // replaces it — a one-frame "already talking" flash at the start of every + // dictation. `MicCapture.stop()` cancels the meter task without a final zero + // yield, so nothing else resets this. Reached by every dismiss that actually + // had a panel on screen; `hide()` returns early when the panel is already + // hidden, but it had to come through here to get hidden, so it's already zero. + bridge.level = 0 } /// Drives the pill on/off screen, fading unless Reduce Motion is on. Idempotent @@ -204,15 +223,15 @@ final class OverlayWindowController { private func reposition() { guard let screen = NSScreen.main else { return } - // The placement rules (default bottom-center, clamping a stale dragged - // origin back on screen) are the engine's `OverlayPlacement`, unit-tested - // there. The 80 pt clearance is measured to the visible *pill*, so the - // panel origin backs off by the transparent shadow margin. - let origin = OverlayPlacement.origin( + // All the placement policy — default bottom-center, the clearance, the + // pill-vs-panel shadow correction, and clamping a stale dragged origin back on + // screen — is the engine's `OverlayPlacement`, unit-tested there. This passes + // only what the shell owns: the panel size, the screen, and the shadow inset. + let origin = OverlayPlacement.panelOrigin( panelSize: panel.frame.size, visibleFrame: screen.visibleFrame, - customOrigin: storedCustomOrigin(), - bottomOffset: 80 - Self.shadowMargin) + customOrigin: Self.originStore.origin, + shadowMargin: Self.shadowMargin) suppressOriginPersist = true panel.setFrameOrigin(origin) suppressOriginPersist = false @@ -220,17 +239,11 @@ final class OverlayWindowController { private func handleDidMove() { guard !suppressOriginPersist else { return } - let origin = panel.frame.origin - UserDefaults.standard.set(Double(origin.x), forKey: Self.customOriginXKey) - UserDefaults.standard.set(Double(origin.y), forKey: Self.customOriginYKey) + Self.originStore.origin = panel.frame.origin } - private func storedCustomOrigin() -> NSPoint? { - let defaults = UserDefaults.standard - guard - let x = defaults.object(forKey: Self.customOriginXKey) as? Double, - let y = defaults.object(forKey: Self.customOriginYKey) as? Double - else { return nil } - return NSPoint(x: x, y: y) - } + /// Persistence for the dragged origin lives in the engine next to the clamping + /// it feeds, and is registered in `PersistedSettings.allDefaultsKeys` so reset + /// sweeps clear it. + private static let originStore = OverlayOriginStore() } diff --git a/App/Blurt/Blurt/UITestSupport.swift b/App/Blurt/Blurt/UITestSupport.swift index d305639..c6d097f 100644 --- a/App/Blurt/Blurt/UITestSupport.swift +++ b/App/Blurt/Blurt/UITestSupport.swift @@ -172,12 +172,11 @@ // MARK: - Harness window - /// The test harness window. It exposes buttons that drive the same pipeline - /// the hotkey would (`AppCoordinator.beginDictation`/`…end`/`…cancel`), a - /// field to set the canned - /// transcript, and read-outs for the live pipeline status and the last - /// "pasted" text — everything the XCUITest suite needs to observe the - /// record → transcribe → paste flow deterministically. + /// The test harness window. It exposes buttons that drive the same + /// `DictationSession` the hotkey would (`press`/`release`/`cancel`) and + /// read-outs for the live pipeline status and the last "pasted" text — + /// everything the XCUITest suite needs to observe the record → transcribe → + /// paste flow deterministically. struct UITestHarnessView: View { var appDelegate: AppDelegate @Environment(\.openWindow) private var openWindow @@ -200,11 +199,11 @@ HStack(spacing: 8) { Button("Set API Key") { coordinator?.apiKey.save(UITestIdentifiers.validAPIKey) } .accessibilityIdentifier(UITestIdentifiers.setKeyButton) - Button("Start") { Task { await coordinator?.beginDictation() } } + Button("Start") { Task { await coordinator?.session.press() } } .accessibilityIdentifier(UITestIdentifiers.startButton) - Button("Stop") { Task { await coordinator?.endDictation() } } + Button("Stop") { Task { await coordinator?.session.release() } } .accessibilityIdentifier(UITestIdentifiers.stopButton) - Button("Cancel") { Task { await coordinator?.cancelDictation() } } + Button("Cancel") { Task { await coordinator?.session.cancel() } } .accessibilityIdentifier(UITestIdentifiers.cancelButton) } diff --git a/App/Blurt/Blurt/Wizard/ReadyView.swift b/App/Blurt/Blurt/Wizard/ReadyView.swift index 17b1a3f..ba38bcc 100644 --- a/App/Blurt/Blurt/Wizard/ReadyView.swift +++ b/App/Blurt/Blurt/Wizard/ReadyView.swift @@ -72,6 +72,11 @@ private struct RecentDictationsSection: View { private static let rowHeight: CGFloat = 28 private static let separatorThickness: CGFloat = 1 + + /// How often the relative timestamps re-render. Half the engine's "just now" + /// window, so a row can't read as stale for longer than that window lasts — + /// derived from the threshold rather than a bare `30` in case it changes. + private static let timestampRefresh = RecentDictations.Entry.justNowThreshold / 2 /// Height of a full `capacity`-row list (rows + the separators between them); /// the container is pinned to this whether it holds 0, 1, or `capacity` rows. private var reservedHeight: CGFloat { @@ -110,9 +115,9 @@ private struct RecentDictationsSection: View { .frame(maxWidth: .infinity, maxHeight: .infinity) } else { // Live relative timestamps ("2 minutes ago") without a stored clock: the - // TimelineView re-renders on a coarse cadence and each row formats against - // its current date. 30 s is fine — the smallest unit shown is minutes. - TimelineView(.periodic(from: .now, by: 30)) { timeline in + // TimelineView re-renders on a coarse cadence (`timestampRefresh`) and each + // row formats against its current date. + TimelineView(.periodic(from: .now, by: Self.timestampRefresh)) { timeline in VStack(spacing: 0) { ForEach(entries) { entry in RecentDictationRow(entry: entry, now: timeline.date) @@ -252,10 +257,22 @@ private struct RecentDictationRow: View { } private struct ReadyBrandingView: View { + /// Loaded once for the process rather than per `body` evaluation: this view + /// sits in `ReadyView`, whose body re-runs on every new dictation + /// (`recentDictations.entries`), and a bundle lookup plus a PNG decode is not + /// something to re-do on the main thread each time. + /// + /// Left to the target's `SWIFT_DEFAULT_ACTOR_ISOLATION: MainActor` default + /// rather than spelled `nonisolated(unsafe)`: `body` reads it on the main actor + /// either way, and this can't then break if a future SDK marks `NSImage` + /// `Sendable` (which would make the attribute an "unnecessary" warning, and the + /// app target builds with warnings-as-errors). + private static let logo: NSImage? = Bundle.main + .url(forResource: "blurt-ready-logo", withExtension: "png") + .flatMap(NSImage.init(contentsOf:)) + var body: some View { - if let brandingURL, - let image = NSImage(contentsOf: brandingURL) - { + if let image = Self.logo { Image(nsImage: image) .interpolation(.none) .resizable() @@ -276,10 +293,6 @@ private struct ReadyBrandingView: View { .accessibilityLabel("Blurt is ready") } } - - private var brandingURL: URL? { - Bundle.main.url(forResource: "blurt-ready-logo", withExtension: "png") - } } /// A single rounded key-cap, e.g. "⌃" or "D". A quiet semantic chip, not diff --git a/App/Blurt/Blurt/Wizard/Steps/APIKeyStepView.swift b/App/Blurt/Blurt/Wizard/Steps/APIKeyStepView.swift index 78b2384..7ce6811 100644 --- a/App/Blurt/Blurt/Wizard/Steps/APIKeyStepView.swift +++ b/App/Blurt/Blurt/Wizard/Steps/APIKeyStepView.swift @@ -13,8 +13,11 @@ import SwiftUI struct APIKeyStepView: View { var apiKey: APIKeyModel - /// The key currently in the Keychain, loaded on appear (empty when none). - @State private var savedKey = "" + /// The in-progress edit. Seeded from the Keychain when the user enters edit + /// mode (see the "Change" button), never held as a long-lived mirror — this view + /// is mounted in BOTH the wizard and Settings, each with its own `@State`, so a + /// cached copy in one goes stale the moment the other saves. Whether a key + /// exists is read from `apiKey.hasAPIKey`, the observable single source. @State private var draft = "" /// True while the editable field is shown for an already-saved key (the user /// tapped "Change"). When a key is saved and we're not editing, the section @@ -41,11 +44,11 @@ struct APIKeyStepView: View { private var canSubmit: Bool { trimmedKey != nil && !isValidating } /// "Save" for the first key, "Update" once one exists. - private var actionTitle: String { savedKey.isEmpty ? "Save" : "Update" } + private var actionTitle: String { apiKey.hasAPIKey ? "Update" : "Save" } var body: some View { Section { - if savedKey.isEmpty || isEditing { + if !apiKey.hasAPIKey || isEditing { keyField } else { savedRow @@ -56,8 +59,6 @@ struct APIKeyStepView: View { statusFooter } .onAppear { - savedKey = apiKey.current ?? "" - draft = savedKey // Keep the readiness gate in sync with what's actually in the Keychain. If // a key is already saved, this flips `hasAPIKey` true so the wizard advances // to the ready screen instead of stranding the user here with a pre-filled @@ -83,6 +84,11 @@ struct APIKeyStepView: View { .accessibilityIdentifier(UITestIdentifiers.apiKeySavedStatus) } Button("Change") { + // Read the stored key here rather than trusting a cached copy: if the + // key was rotated in the other window since this view appeared, a stale + // mirror would pre-fill the OLD key and "Update" would silently write it + // back over the new one. + draft = apiKey.current ?? "" isEditing = true } .accessibilityIdentifier(UITestIdentifiers.apiKeyChange) @@ -138,7 +144,7 @@ struct APIKeyStepView: View { // entry — there's no saved key to cancel back to. if isEditing { Button("Cancel") { - draft = savedKey + draft = "" errorMessage = nil isEditing = false } @@ -172,7 +178,7 @@ struct APIKeyStepView: View { // `Link` view: a `Link` forces its own (body) font, which reads larger than // the surrounding footer copy. As plain `Text` it inherits the grouped // Form's native footer font, so the whole footer matches the other sections. - if savedKey.isEmpty { + if !apiKey.hasAPIKey { getKeyLink } } @@ -195,10 +201,11 @@ struct APIKeyStepView: View { isValidating = false switch result { case .valid: - // Saved — record it and collapse the section to the "✓ Saved" status - // row. The controller reveals the overlay via its hasAPIKey observer. - savedKey = key - draft = key + // Saved — collapse the section to the "✓ Saved" status row. `submit` + // already refreshed `apiKey.hasAPIKey`, which is what the branch above + // reads, so every mounted copy of this view updates together. Clear the + // draft rather than parking the key in view state. + draft = "" isEditing = false case .invalid: errorMessage = "AssemblyAI rejected that key. Double-check it and try again." diff --git a/App/Blurt/Blurt/Wizard/Steps/HotkeyStepView.swift b/App/Blurt/Blurt/Wizard/Steps/HotkeyStepView.swift index 1899cb0..b88d63f 100644 --- a/App/Blurt/Blurt/Wizard/Steps/HotkeyStepView.swift +++ b/App/Blurt/Blurt/Wizard/Steps/HotkeyStepView.swift @@ -15,7 +15,14 @@ struct HotkeyStepView: View { TriggerKey.fromPersisted(triggerKeyCode) }, set: { newValue in - triggerKeyCode = newValue.rawValue + // Write through the store, not the raw `@AppStorage` slot: the store owns + // how a `TriggerKey` is encoded, and `@AppStorage` is here to *observe* the + // key so this view re-renders (it picks up the store's external write). + // Assigning `triggerKeyCode` directly left `TriggerKeyStore`'s setter with + // no production caller, so a change to the encoding — versioning the key, + // storing the case name, a migration — would keep `swift test` green while + // the picker silently kept writing the old form. + TriggerKeyStore().triggerKey = newValue coordinator.dictationBindingChanged() }) } diff --git a/App/Blurt/Blurt/Wizard/Steps/KeyTermsStepView.swift b/App/Blurt/Blurt/Wizard/Steps/KeyTermsStepView.swift index 91bd969..504f71c 100644 --- a/App/Blurt/Blurt/Wizard/Steps/KeyTermsStepView.swift +++ b/App/Blurt/Blurt/Wizard/Steps/KeyTermsStepView.swift @@ -8,6 +8,15 @@ import SwiftUI /// spellings. Optional — it never gates setup; an empty list just sends no terms. struct KeyTermsStepView: View { /// Stored in UserDefaults so multiple settings windows/readers see edits live. + /// + /// `@AppStorage` is the **only** writer of this slot, which is why the store + /// deliberately exposes no setter. Normalization happens on read instead — + /// `KeyTermsStore.get()` trims, and `parse` trims and dedupes each term — so a + /// blank field still reads back as "no terms". A normalizing setter fought the + /// text field: it wrote a second, differently-normalized value on every + /// keystroke, `@AppStorage` observed that as an external write and pushed the + /// trimmed string back into the binding, so a trailing space was deleted as the + /// user typed it and could never be entered at all. @AppStorage(KeyTermsStore.defaultsKey) private var text = "" var body: some View { @@ -28,7 +37,6 @@ struct KeyTermsStepView: View { .font(.body) .disableAutocorrection(true) .accessibilityIdentifier(UITestIdentifiers.keyTermsField) - .onChange(of: text) { _, newValue in KeyTermsStore.set(newValue) } } header: { Text("Key Terms") } footer: { diff --git a/App/Blurt/Blurt/Wizard/Steps/PermissionsStepView.swift b/App/Blurt/Blurt/Wizard/Steps/PermissionsStepView.swift index 9bfff7e..715ef3f 100644 --- a/App/Blurt/Blurt/Wizard/Steps/PermissionsStepView.swift +++ b/App/Blurt/Blurt/Wizard/Steps/PermissionsStepView.swift @@ -9,6 +9,17 @@ import SwiftUI struct PermissionsStepView: View { var controller: WizardController + /// Observed rather than read once via `TriggerKeyStore()`: Settings is reachable + /// with ⌘, while this page is showing, so a one-shot read left the footer naming + /// the old key after a rebind until something else re-rendered the view. Same + /// pattern as `ReadyView` / `MenuBarScene` / `HotkeyStepView`. + @AppStorage(TriggerKeyStore.defaultsKey) private var triggerKeyCode = TriggerKey.rightCommand + .rawValue + + /// The bound trigger's display label, re-read on every render so a rebind in + /// Settings updates the footer immediately. + private var triggerLabel: String { TriggerKey.fromPersisted(triggerKeyCode).label } + /// Set when the user taps a settings button so the section can show a /// "waiting for you to come back" cue until the poll sees the grant. @State private var openedAccessibilitySettings = false @@ -50,7 +61,8 @@ struct PermissionsStepView: View { opened: openedMicrophoneSettings, granted: controller.permissions.microphone, waiting: "Waiting for you to turn on Blurt under Microphone…", - description: "Blurt records only after you start dictating with \(TriggerKeyStore().triggerKey.label)." + description: + "Blurt records only after you start dictating with \(triggerLabel)." ) } } diff --git a/App/Blurt/Blurt/Wizard/Steps/SoundStepView.swift b/App/Blurt/Blurt/Wizard/Steps/SoundStepView.swift index 0782555..06a1543 100644 --- a/App/Blurt/Blurt/Wizard/Steps/SoundStepView.swift +++ b/App/Blurt/Blurt/Wizard/Steps/SoundStepView.swift @@ -12,10 +12,12 @@ struct SoundStepView: View { private var selection: Binding { Binding( get: { - SoundPack.find(id: soundPackID) ?? .defaultPack + SoundPack.fromPersisted(soundPackID) }, set: { newValue in - soundPackID = newValue.id + // Write through the store (see `HotkeyStepView` for why): the store owns the + // encoding, `@AppStorage` observes the key to re-render. + SoundPackStore().soundPack = newValue coordinator.soundPackChanged() }) } @@ -30,7 +32,7 @@ struct SoundStepView: View { ForEach(SoundPack.groups, id: \.self) { group in Section(group) { ForEach(SoundPack.voices(in: group)) { pack in - Text(displayLabel(for: pack)).tag(pack) + Text(pack.label).tag(pack) } } } @@ -41,9 +43,4 @@ struct SoundStepView: View { Text("Set to None to silence start and stop cues.") } } - - private func displayLabel(for pack: SoundPack) -> String { - guard let synth = pack.synth else { return pack.label } - return "\(pack.label) · \(synth)" - } } diff --git a/App/Blurt/BlurtUITests/BlurtUITestSupport.swift b/App/Blurt/BlurtUITests/BlurtUITestSupport.swift index 3d46dac..65f0948 100644 --- a/App/Blurt/BlurtUITests/BlurtUITestSupport.swift +++ b/App/Blurt/BlurtUITests/BlurtUITestSupport.swift @@ -169,3 +169,15 @@ class BlurtUITestCase: XCTestCase { XCTAssertEqual(result, .completed, failure) } } + +@MainActor +extension XCUIElement { + /// The first descendant carrying `identifier`, matched across *all* element + /// types. AppKit may expose a given SwiftUI control as a switch, a checkbox, or + /// a static text — and which one can change between macOS releases — so these + /// lookups must not be scoped by element type. Stated once here so a future + /// change in how a control surfaces is one fix, not four. + func anyDescendant(identified identifier: String) -> XCUIElement { + descendants(matching: .any).matching(identifier: identifier).firstMatch + } +} diff --git a/App/Blurt/BlurtUITests/DictationPipelineUITests.swift b/App/Blurt/BlurtUITests/DictationPipelineUITests.swift index a093a29..c58369f 100644 --- a/App/Blurt/BlurtUITests/DictationPipelineUITests.swift +++ b/App/Blurt/BlurtUITests/DictationPipelineUITests.swift @@ -42,7 +42,7 @@ final class DictationPipelineUITests: BlurtUITestCase { } /// The same happy path driven through the harness's Start/Stop buttons, which - /// call the session directly (`beginDictation`/`endDictation`) rather than the + /// call the session directly (`press()`/`release()`) rather than the /// key tap. Complements the hotkey test above: it covers the direct-session /// seam the overlay/menu-bar affordances use, independent of the tap wiring. func testStartStopButtonsDriveRecordTranscribePaste() { @@ -99,8 +99,8 @@ final class DictationPipelineUITests: BlurtUITestCase { let status = harness.staticTexts[UITestIdentifiers.statusLabel] // The pill lives on a separate floating panel, so search the whole app tree - // (not just the harness window) and match by identifier across element types. - let pill = app.descendants(matching: .any).matching(identifier: UITestIdentifiers.overlayPill).firstMatch + // rather than just the harness window. + let pill = app.anyDescendant(identified: UITestIdentifiers.overlayPill) harness.buttons[UITestIdentifiers.startButton].click() waitForLabel(status, equals: "recording", "Start should drive status to recording") diff --git a/App/Blurt/BlurtUITests/README.md b/App/Blurt/BlurtUITests/README.md index 95bc485..2d3fe46 100644 --- a/App/Blurt/BlurtUITests/README.md +++ b/App/Blurt/BlurtUITests/README.md @@ -23,15 +23,14 @@ observable UI rather than mocking views. These tests don't need a microphone, the network, Accessibility permission, or the real Keychain. Launching with `-BlurtUITest` (set by `BlurtUITestCase.setUpWithError`) activates the harness in -`App/Blurt/Blurt/UITestSupport.swift` (`#if DEBUG`), which: +`App/Blurt/Blurt/UITestSupport.swift` (`#if UITEST_HOOKS`), which: - injects offline stub collaborators (`UITestMic`, `UITestTranscriber`, `UITestInjector`) and an `InMemoryAPIKeyStore` via the `DictationComponents` / `APIKeyGateway` seams on `AppCoordinator`, and -- presents a harness window with buttons that call the same pipeline entry points - the hotkey uses (`AppCoordinator.beginDictation` / `endDictation` / - `cancelDictation`), plus read-outs for the live pipeline phase and the - injector's last "paste". +- presents a harness window with buttons that drive the same `DictationSession` + the hotkey does (`press()` / `release()` / `cancel()`), plus read-outs for the + live pipeline phase and the injector's last "paste". The lone-modifier `CGEventTap` trigger can't be synthesized by XCUITest (and needs an Accessibility-trusted process), so the harness drives the pipeline @@ -41,7 +40,7 @@ directly; the tap → `DictationKeyGate` wiring is covered by the engine unit te ```bash scripts/uitest.sh # just the UI suite (macOS + Xcode) -RUN_UI_TESTS=1 scripts/check.sh # full health check including the UI suite +scripts/check.sh # full health check; runs the UI suite too ``` ## Maintenance diff --git a/App/Blurt/BlurtUITests/ReadyViewUITests.swift b/App/Blurt/BlurtUITests/ReadyViewUITests.swift index 1e8d517..91dc75b 100644 --- a/App/Blurt/BlurtUITests/ReadyViewUITests.swift +++ b/App/Blurt/BlurtUITests/ReadyViewUITests.swift @@ -124,8 +124,7 @@ final class ReadyViewUITests: BlurtUITestCase { private func driveDictation(via harness: XCUIElement) { harness.buttons[UITestIdentifiers.hotkeyPressButton].click() harness.buttons[UITestIdentifiers.hotkeyReleaseButton].click() - let echo = harness.descendants(matching: .any) - .matching(identifier: UITestIdentifiers.transcriptEchoLabel).firstMatch + let echo = harness.anyDescendant(identified: UITestIdentifiers.transcriptEchoLabel) waitForLabel(echo, equals: UITestIdentifiers.defaultCannedTranscript) } diff --git a/App/Blurt/BlurtUITests/SettingsUITests.swift b/App/Blurt/BlurtUITests/SettingsUITests.swift index 77aa3e5..e9ef0cd 100644 --- a/App/Blurt/BlurtUITests/SettingsUITests.swift +++ b/App/Blurt/BlurtUITests/SettingsUITests.swift @@ -12,7 +12,7 @@ final class SettingsUITests: BlurtUITestCase { let field = settings.secureTextFields[UITestIdentifiers.apiKeyField] XCTAssertTrue(field.waitForExistence(timeout: 10), "API key field not found") field.click() - field.typeText("a-valid-looking-key") + field.typeText(UITestIdentifiers.validAPIKey) settings.buttons[UITestIdentifiers.apiKeySave].click() @@ -92,8 +92,7 @@ final class SettingsUITests: BlurtUITestCase { let settings = openSettingsWindow() let advanced = selectSettingsTab(settings, named: UITestIdentifiers.advancedSettingsTab) - let toggle = advanced.descendants(matching: .any) - .matching(identifier: UITestIdentifiers.developerToggle).firstMatch + let toggle = advanced.anyDescendant(identified: UITestIdentifiers.developerToggle) XCTAssertTrue(toggle.waitForExistence(timeout: 10), "Developer mode toggle not found") XCTAssertEqual("\(toggle.value ?? "")", "0", "Developer mode should start switched off") @@ -110,8 +109,7 @@ final class SettingsUITests: BlurtUITestCase { let settings = openSettingsWindow() let advanced = selectSettingsTab(settings, named: UITestIdentifiers.advancedSettingsTab) - let button = advanced.descendants(matching: .any) - .matching(identifier: UITestIdentifiers.updateCheck).firstMatch + let button = advanced.anyDescendant(identified: UITestIdentifiers.updateCheck) XCTAssertTrue(button.waitForExistence(timeout: 10), "Check for Updates button not found") button.click() @@ -131,7 +129,7 @@ final class SettingsUITests: BlurtUITestCase { let field = settings.secureTextFields[UITestIdentifiers.apiKeyField] XCTAssertTrue(field.waitForExistence(timeout: 10), "API key field not found") field.click() - field.typeText("a-valid-looking-key") + field.typeText(UITestIdentifiers.validAPIKey) settings.buttons[UITestIdentifiers.apiKeySave].click() let change = settings.buttons[UITestIdentifiers.apiKeyChange] @@ -153,7 +151,7 @@ final class SettingsUITests: BlurtUITestCase { let field = settings.secureTextFields[UITestIdentifiers.apiKeyField] XCTAssertTrue(field.waitForExistence(timeout: 10), "API key field not found") field.click() - field.typeText("a-valid-looking-key") + field.typeText(UITestIdentifiers.validAPIKey) settings.buttons[UITestIdentifiers.apiKeySave].click() let change = settings.buttons[UITestIdentifiers.apiKeyChange] diff --git a/BLURTENGINE.md b/BLURTENGINE.md index f6b28da..de15b85 100644 --- a/BLURTENGINE.md +++ b/BLURTENGINE.md @@ -51,7 +51,7 @@ Before the first dictation can succeed the host must have: ```text press() ──▶ MicCapture.start() release() ──▶ MicCapture.stop() → Data (raw S16LE PCM) (16 kHz mono 16-bit PCM) AssemblyAITranscriber.transcribe(pcm:sampleRate:context:) - + focus/context capture (one POST sync.assemblyai.com/transcribe, X-AAI-Model: u3-sync-pro) + + focus/context capture (one POST sync.assemblyai.com/transcribe, X-AAI-Model: universal-3-5-pro) + connection warm-up KeyInjector.insert(text, after: priorText) (clipboard paste via synthesized ⌘V) ``` @@ -88,7 +88,7 @@ idle → recording → transcribing → injecting → pasted | noTarget └── failed(BlurtError) / cancelled (from any stage) ``` -- `phaseStream()` yields the current phase immediately, then every transition. It is a **single-observer** stream: each call supersedes (finishes) the previous one. That's all a host needs — one renderer — but don't fan it out to multiple long-lived consumers; project the phase into your own state instead. +- `phaseStream()` yields the current phase immediately, then every transition. It is a **multi-observer** stream: every call gets its own continuation and all of them see later transitions, so fanning it out is supported. That's all a host needs — one renderer — but don't fan it out to multiple long-lived consumers; project the phase into your own state instead. - `.pasted` and `.noTarget` are terminal _success_ states, not errors. `.noTarget` means transcription worked but nothing editable was focused (or the target app quit), so the text was left on the clipboard — show a quiet "copied" notice, not a failure. - Two ready-made projections keep UI mapping out of your shell: `phase.overlayState` (`OverlayUIState`: idle / recording / processing / error(message:) / pasted / noTarget, with accessibility labels and — for the transient notices — `noticeDwellSeconds`, how long to hold one before reverting to idle) and `phase.menuBarStatus` (coarser: idle / recording / transcribing, never shows errors, with `symbolName`/`accessibilityLabel` presentation). @@ -123,7 +123,7 @@ Only `start()`/`stop()` must be implemented — `levels` and `warmUp()` have def `MicCapture` records with `AVAudioRecorder` straight to a temp 16 kHz / mono / 16-bit PCM WAV — exactly the geometry the Sync API wants — and reads it back as raw S16LE bytes on `stop()` (no float detour; the blob uploads as-is). A **fresh recorder per session** resolves the current default input device at `record()` time, which is why device switches (headset ↔ built-in) just work. Do **not** replace this with a long-lived `AVAudioEngine`/`installTap` graph: that design was tried, bound itself to one device, and failed with `-10868` or all-zero buffers on device switches. -`MicCapture`'s `levels` is a ~30 Hz meter of the recorder's dBFS power mapped to `0…1` (floored at −50 dBFS so room ambient reads as silence) — feed it to a voice-bars view; it costs nothing when unobserved. Its `warmUp()` pre-creates and prepares a recorder so the first `start()` skips hardware route discovery (Blurt calls it at launch, once mic permission is granted, so warming never triggers the permission prompt). +`MicCapture`'s `levels` is a ~20 Hz meter of the recorder's dBFS power mapped to `0…1` (floored at −50 dBFS so room ambient reads as silence) — feed it to a voice-bars view; it costs nothing when unobserved. Its `warmUp()` pre-creates and prepares a recorder so the first `start()` skips hardware route discovery (Blurt calls it at launch, once mic permission is granted, so warming never triggers the permission prompt). ### `TranscriberProtocol` → `AssemblyAITranscriber` @@ -132,7 +132,7 @@ func transcribe(pcm: Data, sampleRate: Int, context: TranscriptionContext?) asyn func warmUp() async // optional; no-op default ``` -`AssemblyAITranscriber` is a stateless `Sendable` struct. One `POST https://sync.assemblyai.com/transcribe` per utterance: the audio as raw S16LE PCM (the `pcm` blob, byte-for-byte) in the `audio` multipart part, plus a JSON `config` part (`sample_rate`, `channels`, and the rendered `prompt`), with `X-AAI-Model: u3-sync-pro` and the API key in `Authorization`. Its initializer takes an `apiKeyProvider` closure (defaults to `APIKeyStore.get`), a `baseURL`, and an `HTTPTransport` — inject a fake transport (see `Tests/BlurtEngineTests/Stubs/FakeHTTPTransport.swift`) to test against canned responses. `warmUp()` fires a throwaway GET at the host root to pre-pool the connection; it never throws and any failure just means the real request pays connection setup as before. +`AssemblyAITranscriber` is a stateless `Sendable` struct. One `POST https://sync.assemblyai.com/transcribe` per utterance: the audio as raw S16LE PCM (the `pcm` blob, byte-for-byte) in the `audio` multipart part, plus a JSON `config` part (`sample_rate`, `channels`, and the rendered `prompt`), with `X-AAI-Model: universal-3-5-pro` and the API key in `Authorization`. Its initializer takes an `apiKeyProvider` closure (defaults to `APIKeyStore.get`), a `baseURL`, and an `HTTPTransport` — inject a fake transport (see `Tests/BlurtEngineTests/Stubs/FakeHTTPTransport.swift`) to test against canned responses. `warmUp()` fires a throwaway GET at the host root to pre-pool the connection; it never throws and any failure just means the real request pays connection setup as before. The model's limits live in `SyncSTTLimits` (16 kHz sample rate, ~0.1 s–120 s audio, and the auto-release math) — the single source shared by the mic, the session, and the request so recorded and declared geometry can't drift. @@ -143,7 +143,7 @@ func setTargetApp(_ app: NSRunningApplication?) async func insert(_ text: String, after priorText: String?, windowTitle: String?) async throws ``` -`KeyInjector.insert` **always** pastes: it saves the current pasteboard, writes the transcript, activates the captured target app, posts a synthesized ⌘V, waits for the target to read the clipboard (`pasteSettleDuration`, default 400 ms, tunable in the initializer), then restores the prior pasteboard contents. There is no keystroke-by-keystroke typing path and no length threshold. If the target app is gone or nothing editable is focused it leaves the text on the clipboard and throws `.targetAppLost` / `.noEditableTarget` — which the session turns into the quiet `.noTarget` outcome. `priorText` (the text before the caret, captured at press time) drives `withLeadingSeparator`, which joins consecutive dictations with a space so they don't run together. When `priorText` is unreadable (an Accessibility-opaque editor, or a browser tab like Google Docs whose canvas-rendered body exposes no AX text), `separatorBasis` falls back to what was last pasted — but only when both the target app **and** `windowTitle` match the last successful insert, so the fallback tracks "the same window," not just "the same process" (a browser hosts many unrelated tabs/documents under one PID). +`KeyInjector.insert` **always** pastes: it saves the current pasteboard, writes the transcript, activates the captured target app, posts a synthesized ⌘V, waits for the target to read the clipboard (`pasteSettleDuration`, default 400 ms, tunable in the initializer), then restores the prior pasteboard contents. That restore never destroys what it can't put back: an unreadable pasteboard snapshots as `nil` (not as empty) and is skipped, and the replacement items are built before `clearContents()`, with the plain-string flavor as a floor when promised representations can't be materialized. There is no keystroke-by-keystroke typing path and no length threshold. If the target app is gone or nothing editable is focused it leaves the text on the clipboard and throws `.targetAppLost` / `.noEditableTarget` — which the session turns into the quiet `.noTarget` outcome. `priorText` (the text before the caret, captured at press time) drives `withLeadingSeparator`, which joins consecutive dictations with a space so they don't run together. When `priorText` is unreadable (an Accessibility-opaque editor, or a browser tab like Google Docs whose canvas-rendered body exposes no AX text), `separatorBasis` falls back to what was last pasted — but only when both the target app **and** `windowTitle` match the last successful insert, so the fallback tracks "the same window," not just "the same process" (a browser hosts many unrelated tabs/documents under one PID). The session calls `setTargetApp` at press time with the app that was frontmost when recording started — so the paste lands where the user was, even if focus moved during transcription. @@ -152,7 +152,7 @@ The session calls `setTargetApp` at press time with the app that was frontmost w Recognition quality comes from per-utterance priming, assembled automatically inside `press()` — hosts don't call these APIs directly, but should know what's collected: - **`TranscriptionContext`** carries the frontmost app name, window title, focused-field label, the text before the caret, the selected text (which a paste will replace), and the user's key terms. It's captured via Accessibility at press time (skipped in secure fields), off the hot path — and consumed at transcribe time with a bounded wait (`DictationSession.contextWaitBudget`), so a hung read is abandoned rather than stalling the transcript. -- **`TranscriptionPrompt.build(context:)`** renders that into the Sync request's `config.prompt`, opening with the fixed `baseInstruction` ("Transcribe without speaker labels, audio event descriptions, or emotion markers.") and staying under the API's 4096-character cap. An empty context yields `nil`, which omits the field so the server applies its own default. Two deliberate omissions, both regression-tested: no language directive (pinning to English hurt non-English speech) and no "remove filler words" clause (not in the model's trained instruction set — a no-op). Don't reintroduce either. +- **`TranscriptionPrompt.build(context:)`** renders that into the Sync request's `config.prompt`, opening with the fixed `baseInstruction` ("Transcribe without speaker labels, audio event descriptions, or emotion markers.") and staying under a self-imposed 4096-character ceiling (`characterCap`; the Sync API documents no cap on `config.prompt`). An empty context yields `nil`, which omits the field so the server applies its own default. Two deliberate omissions, both regression-tested: no language directive (pinning to English hurt non-English speech) and no "remove filler words" clause (not in the model's trained instruction set — a no-op). Don't reintroduce either. - **`KeyTermsStore`** persists the user's domain vocabulary (names, jargon) in `UserDefaults`; `DictationSession` re-reads it at every press via its `keyTermsProvider` closure, so Settings edits apply to the next utterance without rebuilding the session. Pass your own provider to source terms from elsewhere. For key storage, compose against **`APIKeyGateway`** — the injectable get/set/`hasKey` seam over the key store. `ProductionAPIKeyStore` forwards to the Keychain-backed `APIKeyStore`; `InMemoryAPIKeyStore` is a ready-made in-memory conformance for tests and harnesses (Blurt's XCUITest runs use it so the real Keychain item is never touched, and its `hasKey` backs the session's `readinessCheck`). For a settings UI, **`APIKeySubmission`** wraps the gateway with the validate-then-save flow (`submit(_:)` → valid / invalid / unreachable / saveFailed, via `APIKeyValidator`): it saves only a key AssemblyAI actively accepts, so an unverified key never persists. @@ -163,7 +163,7 @@ Each completed dictation is appended to **`DictationLog`** (a local JSONL histor The engine ships the _decision logic_ for a lone-modifier trigger; the host supplies the event source (in Blurt, a `CGEventTap` — see `App/Blurt/Blurt/Hotkey/DictationKeyTap.swift` for the reference wiring). -- **`TriggerKey`** — the curated lone modifiers usable as a trigger (right ⌘, right ⌥, right ⌃, `fn`), with keycodes, display labels, and the device-modifier masks the event source needs. +- **`TriggerKey`** — the curated lone modifiers usable as a trigger (right ⌘, right ⌥, `fn`), with keycodes, display labels, and the device-modifier masks the event source needs. - **`TriggerKeyStore`** — persists the chosen key in `UserDefaults` (`BlurtTriggerKeyCode`), defaulting to right ⌘. - **`DictationKeyGate`** — a pure, clock-free state machine that turns `modifierDown(at:)` / `modifierUp(at:)` / `otherKeyDown()` into `.start` / `.stop` / `.cancel` / `.none`. Recording starts the instant the modifier goes down; on key-up, a release held ≥ `holdThreshold` (default 1 s) is push-to-talk (stop), a shorter release latches tap-to-toggle (next tap stops). A modifier+key combo from idle cancels the fresh capture; over a latched recording it passes through as a normal shortcut. Callers pass monotonic timestamps, so every decision is deterministic and unit-tested (`DictationKeyGateTests`, `HotkeyRaceTests`). - **`DictationKeyRouter`** — the recommended layer over the gate: reduce each raw event to `.flagsChanged(keyCode:triggerFlagIsOn:)` / `.keyDown(keyCode:)` and `handle(_:at:)` applies the filters every event source needs — only the bound keycode's flag changes count, and only genuine down/up _edges_ reach the gate (`flagsChanged` deliveries re-report the bit whether or not it changed, so a repeat must not double-start a dictation). `reset()` / `rebind(triggerKeyCode:)` clear state that can no longer be trusted (dropped events, a rebound trigger) and return whether they discarded a live recording. Unit-tested (`DictationKeyRouterTests`). diff --git a/README.md b/README.md index ccee886..1dd6266 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ Sources/BlurtEngine/ Swift 6 package owning the pipeline — no external dep Audio/ MicCapture: fresh AVAudioRecorder per session, 16 kHz mono PCM, live level meter; DX7/Juno-106 sound packs STT/ AssemblyAITranscriber: one POST to sync.assemblyai.com/transcribe - (u3-sync-pro) + TranscriptionPrompt contextual priming + (universal-3-5-pro) + TranscriptionPrompt contextual priming Pipeline/ DictationSession actor: press/release/cancel commands, phase stream, auto-release before the API's recording cap Hotkey/ DictationKeyGate/Router: pure, unit-tested state machine for the diff --git a/Sources/BlurtEngine/Audio/MicCapture.swift b/Sources/BlurtEngine/Audio/MicCapture.swift index e4399b6..b403aa1 100644 --- a/Sources/BlurtEngine/Audio/MicCapture.swift +++ b/Sources/BlurtEngine/Audio/MicCapture.swift @@ -37,12 +37,20 @@ public actor MicCapture: MicCaptureProtocol { private var activeRecorder: AVAudioRecorder? /// Polls the active recorder's meter and feeds `levels` while recording. private var meterTask: Task? - /// How often the meter is sampled for the overlay. 20 Hz reads as smooth for a - /// voice-level meter while cutting the per-tick work (recorder poll + stream - /// yield, and the SwiftUI bar redraw it drives) by a third versus 30 Hz — the - /// bars' `TimelineView` cap is matched to it so it never redraws faster than - /// the level actually changes. - private static let meterInterval = Duration.milliseconds(50) + /// How often the meter is sampled for the overlay, in seconds. 20 Hz reads as + /// smooth for a voice-level meter while cutting the per-tick work (recorder poll + /// + stream yield, and the SwiftUI bar redraw it drives) by a third versus 30 Hz. + /// + /// Public, and the single definition: the pill's continuous animations cap their + /// redraw to this cadence because the level feed can't show anything faster, so + /// the view reads it here rather than restating `1.0 / 20.0` behind a comment + /// claiming the two match — a coupling nothing enforced, which a change here + /// would silently break, leaving the view under-sampling frames the meter is + /// paying to produce. + public static let meterIntervalSeconds: Double = 0.05 + + /// `meterIntervalSeconds` as the `Duration` the meter task sleeps for. + private static let meterInterval = Duration.seconds(meterIntervalSeconds) public init() { // The continuation is fed from a ~20 Hz meter timer; the levels stream is a @@ -105,7 +113,7 @@ public actor MicCapture: MicCaptureProtocol { let pcm = try Self.decodePCM(fromFileAt: url) let sampleCount = pcm.count / SyncSTTLimits.bytesPerSample - let durationMs = Int((Double(sampleCount) / Self.targetSampleRate) * 1000) + let durationMs = SyncSTTLimits.durationMs(ofPCMBytes: pcm.count) Self.logger.info("stop samples=\(sampleCount) durationMs=\(durationMs)") return pcm } @@ -139,7 +147,7 @@ public actor MicCapture: MicCaptureProtocol { while !Task.isCancelled { // Rebound per iteration so the actor stays releasable across the sleep. // Bail when it's gone: deinit doesn't cancel this task, so the weak - // capture is what stops an orphaned meter from spinning at ~30 Hz for + // capture is what stops an orphaned meter from spinning at ~20 Hz for // the rest of the process if a capture is dropped without stop(). guard let self else { return } await self.emitLevel() diff --git a/Sources/BlurtEngine/Audio/SoundPack.swift b/Sources/BlurtEngine/Audio/SoundPack.swift index 2348128..d096011 100644 --- a/Sources/BlurtEngine/Audio/SoundPack.swift +++ b/Sources/BlurtEngine/Audio/SoundPack.swift @@ -14,30 +14,41 @@ public struct SoundPack: Sendable, Hashable, Identifiable { /// The "no sound" choice. public static let none = SoundPack(id: "none", label: "None", group: nil) + /// True for the `.none` pack — the one voice with no bundled cues and no synth + /// credit. A catalog entry always carries a `group` (its picker section), so its + /// absence is what marks the silent pack. Internal: the app asks via + /// `startFileName`/`stopFileName`, so exporting this would only trip + /// periphery's redundant-public check. + var isSilent: Bool { group == nil } + /// Bundled cue stem for the start cue, or nil when no sound plays. - public var startFileName: String? { group == nil ? nil : "\(id)-start" } + public var startFileName: String? { isSilent ? nil : "\(id)-start" } /// Bundled cue stem for the stop cue, or nil when no sound plays. - public var stopFileName: String? { group == nil ? nil : "\(id)-stop" } - - /// The synth this voice comes from, for the ready-screen credit, e.g. - /// "Yamaha DX-7". nil when no sound plays. - public var synth: String? { - guard let group else { return nil } - if group.hasPrefix("Yamaha DX7") { return "Yamaha DX-7" } - if group.hasPrefix("Roland Juno-106") { return "Roland Juno-106" } - return nil - } + public var stopFileName: String? { isSilent ? nil : "\(id)-stop" } - /// `.none` followed by the full catalog — the picker's complete option set. - public static var all: [SoundPack] { [.none] + catalog } + /// `.none` followed by the full catalog. Private: the picker builds itself from + /// `.none` + `groups` + `voices(in:)`, so this exists only to back `find(id:)`. + private static var all: [SoundPack] { [.none] + catalog } /// The default pack: ORCHESTRA (Yamaha DX7 ROM1A voice 6), falling back to /// `.none` only if the catalog is somehow empty. public static var defaultPack: SoundPack { catalog.first { $0.id == "rom1a-6" } ?? .none } - /// Looks a pack up by its persisted `id`. - public static func find(id: String) -> SoundPack? { all.first { $0.id == id } } + /// Looks a pack up by its persisted `id`. Internal for the same reason as + /// `isSilent`: the app reaches packs through `fromPersisted`, so exporting this + /// would only trip periphery's redundant-public check. + static func find(id: String) -> SoundPack? { all.first { $0.id == id } } + + /// Decodes a persisted pack id, falling back to `defaultPack` when the id is + /// unset or names no known pack. The single decode-with-default rule shared by + /// `SoundPackStore` and the `@AppStorage` views that read the raw id directly + /// (so they re-render live on a Settings change) — mirroring + /// `TriggerKey.fromPersisted`. + public static func fromPersisted(_ id: String?) -> SoundPack { + guard let id, let pack = find(id: id) else { return .defaultPack } + return pack + } /// Distinct group names, in catalog order — the picker's sections. public static var groups: [String] { diff --git a/Sources/BlurtEngine/Audio/SoundPackStore.swift b/Sources/BlurtEngine/Audio/SoundPackStore.swift index 5323afb..88b5a19 100644 --- a/Sources/BlurtEngine/Audio/SoundPackStore.swift +++ b/Sources/BlurtEngine/Audio/SoundPackStore.swift @@ -15,10 +15,7 @@ public struct SoundPackStore { public var soundPack: SoundPack { get { - guard let id = defaults.string(forKey: Self.defaultsKey), - let pack = SoundPack.find(id: id) - else { return .defaultPack } - return pack + SoundPack.fromPersisted(defaults.string(forKey: Self.defaultsKey)) } nonmutating set { defaults.set(newValue.id, forKey: Self.defaultsKey) diff --git a/Sources/BlurtEngine/Config/APIKeyStore.swift b/Sources/BlurtEngine/Config/APIKeyStore.swift index abbfd6b..0fe290c 100644 --- a/Sources/BlurtEngine/Config/APIKeyStore.swift +++ b/Sources/BlurtEngine/Config/APIKeyStore.swift @@ -36,9 +36,23 @@ public enum APIKeyStore { public static func get() -> String? { cache.withLock { state in if case .loaded(let value) = state { return value } - let value = store.get() - state = .loaded(value) - return value + switch store.read() { + case .value(let key): + state = .loaded(key) + return key + case .absent: + state = .loaded(nil) + return nil + case .unavailable: + // Deliberately NOT memoized. A transient read failure — a locked login + // keychain, or the user denying the item's ACL prompt (which + // re-signing the app forces) — would otherwise pin "no API key" for the + // rest of the process: the wizard claims setup is incomplete and every + // press fails `.apiKeyMissing` with no way back short of a relaunch or + // retyping the key. Leaving the cache `.unloaded` means the next press + // retries the Keychain, which is how this self-healed before the memo. + return nil + } } } @@ -46,13 +60,24 @@ public enum APIKeyStore { /// deletes the stored key. Returns `true` on success. @discardableResult public static func set(_ key: String?) -> Bool { - let ok = store.set(key) - // Refresh the memo from the store rather than caching `key` verbatim: `set` - // trims/normalizes (and maps empty → deleted), so a re-read reflects exactly - // what `get()` would now return, and a failed write leaves no stale value. - let stored = store.get() - cache.withLock { $0 = .loaded(stored) } - return ok + // The write, the read-back, and the memo update all happen under one lock, so + // two overlapping writers can't interleave into a memo that disagrees with the + // Keychain (write A, write B, memo B, memo A would leave `hasKey` true for a + // key the Keychain no longer holds — a 401 the user can't explain until + // relaunch). `store` does not take this lock, so there's no reentrancy. + cache.withLock { state in + let ok = store.set(key) + // Re-read rather than caching `key` verbatim: `set` trims/normalizes (and + // maps empty → deleted), so a read-back reflects exactly what `get()` would + // now return, and a failed write leaves no stale value. An unreadable + // Keychain leaves the memo unloaded so the next `get()` retries. + switch store.read() { + case .value(let stored): state = .loaded(stored) + case .absent: state = .loaded(nil) + case .unavailable: state = .unloaded + } + return ok + } } // "Has a key?" lives on the injectable seam: `APIKeyGateway.hasKey` diff --git a/Sources/BlurtEngine/Config/APIKeyValidator.swift b/Sources/BlurtEngine/Config/APIKeyValidator.swift index 80bc3f1..66766ae 100644 --- a/Sources/BlurtEngine/Config/APIKeyValidator.swift +++ b/Sources/BlurtEngine/Config/APIKeyValidator.swift @@ -54,14 +54,16 @@ public struct APIKeyValidator: Sendable { guard let http = response as? HTTPURLResponse else { return .unreachable } switch http.statusCode { case 200..<300: return .valid - // 408 (request timeout) and 429 (rate limited) are transient — the key - // may be perfectly good, so report unreachable (a retry-when-online error) - // rather than rejecting it as invalid. - case 408, 429: return .unreachable - // Any other 4xx is a client error that means the key/request itself is - // bad (401/403 auth rejection, 400/422 malformed). Treat as invalid so - // the wizard shows a real error instead of silently saving a dead key. - case 400..<500: return .invalid + // Only the statuses that actually mean "this credential was rejected" or + // "this key is malformed" may report `.invalid`, because `.invalid` HARD + // BLOCKS setup: `APIKeySubmission.submit` refuses to persist the key, so the + // user can't finish onboarding with a key that works fine for dictation. + // A blanket `400..<500` also caught the cases that say nothing about the key + // — AssemblyAI retiring or moving this endpoint (404/405/410), or a corporate + // proxy or captive portal interposing its own 403/451 page — and told the + // user their good key was rejected. Everything else falls through to + // `.unreachable`, the outcome designed for "couldn't determine". + case 401, 403, 400, 422: return .invalid // 5xx and anything unexpected: server-side / can't determine — report // unreachable so the user retries rather than seeing a false rejection. default: return .unreachable diff --git a/Sources/BlurtEngine/Config/KeyTermsStore.swift b/Sources/BlurtEngine/Config/KeyTermsStore.swift index f5c92f2..abf68c0 100644 --- a/Sources/BlurtEngine/Config/KeyTermsStore.swift +++ b/Sources/BlurtEngine/Config/KeyTermsStore.swift @@ -6,9 +6,16 @@ import Foundation /// them correctly (see `TranscriptionPrompt.build`). /// /// Unlike the API key these aren't secret, so they live in `UserDefaults` rather -/// than the Keychain. The setup wizard and the Settings window read/write the -/// raw string via `get`/`set`; the transcription pipeline reads the parsed list -/// via `terms()`. +/// than the Keychain. The transcription pipeline reads the parsed list via +/// `terms()`; the editor reads the raw string via `get()`. +/// +/// Read-only by design: the Settings field binds `@AppStorage` straight to +/// `defaultsKey`, which is the sole writer. There is deliberately no setter — +/// normalizing on write fought the text field, because the trimmed value was +/// pushed back into the binding as an external change and a trailing space was +/// deleted as the user typed it. Normalization lives on the read side instead +/// (`get()` trims, `parse` trims and dedupes), so a blank field still reads back +/// as "no terms". See the note on `KeyTermsStepView.text`. public enum KeyTermsStore { /// `UserDefaults` key for the raw, comma-separated string the user typed. /// Public so the app can clear it when resetting to a clean state under UI @@ -23,15 +30,6 @@ public enum KeyTermsStore { defaults.string(forKey: defaultsKey).trimmedNonEmpty() } - /// Stores the raw string. Passing `nil` or a blank string clears it. - public static func set(_ raw: String?) { - if let trimmed = raw.trimmedNonEmpty() { - defaults.set(trimmed, forKey: defaultsKey) - } else { - defaults.removeObject(forKey: defaultsKey) - } - } - /// The stored terms parsed into a clean list: split on commas, trimmed, with /// blanks and duplicates removed (case-insensitively, keeping first spelling). public static func terms() -> [String] { diff --git a/Sources/BlurtEngine/Config/KeychainStore.swift b/Sources/BlurtEngine/Config/KeychainStore.swift index aaccdc5..5f6893a 100644 --- a/Sources/BlurtEngine/Config/KeychainStore.swift +++ b/Sources/BlurtEngine/Config/KeychainStore.swift @@ -7,23 +7,43 @@ struct KeychainStore: Sendable { let service: String let account: String - /// The stored value, or `nil` if none has been saved (or it's empty). - func get() -> String? { + /// Why a read produced no value. Collapsing these to a bare `nil` is unsafe for + /// any caller that *memoizes* the answer: `errSecItemNotFound` is a durable fact + /// ("nothing saved"), while a locked keychain or a denied ACL prompt is a + /// transient failure that must not be cached as "nothing saved". + enum ReadResult: Sendable, Equatable { + /// The item is absent, or present but empty / not valid UTF-8 — a durable + /// "there is no usable value here". + case absent + /// The stored value. + case value(String) + /// The read itself failed, so whether a value exists is unknown. Seen when the + /// login keychain is locked, when the user denies the "wants to use your + /// confidential information" ACL prompt, or when the item's ACL needs + /// re-approval after the app is re-signed. + case unavailable(OSStatus) + } + + /// Reads the item, distinguishing "nothing saved" from "couldn't read it". + func read() -> ReadResult { var query = baseQuery() query[kSecReturnData as String] = true query[kSecMatchLimit as String] = kSecMatchLimitOne var item: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &item) - guard - status == errSecSuccess, - let data = item as? Data, - let key = String(data: data, encoding: .utf8), - !key.isEmpty - else { - return nil + switch status { + case errSecSuccess: + guard let data = item as? Data, + let key = String(data: data, encoding: .utf8), + !key.isEmpty + else { return .absent } + return .value(key) + case errSecItemNotFound: + return .absent + default: + return .unavailable(status) } - return key } /// Stores `value` (trimmed). Passing `nil` or an empty/whitespace string diff --git a/Sources/BlurtEngine/Config/PersistedSettings.swift b/Sources/BlurtEngine/Config/PersistedSettings.swift index 55b7a6b..f1b54fd 100644 --- a/Sources/BlurtEngine/Config/PersistedSettings.swift +++ b/Sources/BlurtEngine/Config/PersistedSettings.swift @@ -1,8 +1,8 @@ -/// The roster of `UserDefaults` keys the engine's settings stores persist -/// (trigger key, sound pack, key terms, developer mode). Owned here — next to the stores — -/// so adding a store and adding it to every "reset to a clean state" sweep -/// (e.g. the app's UI-test launch reset) are the same edit, instead of a -/// hand-maintained list in the app shell that silently goes stale. +/// The roster of `UserDefaults` keys the engine's settings stores persist: +/// trigger key, sound pack, key terms, developer mode, overlay origin. Owned +/// here — next to the stores — so adding a store and adding it to every "reset +/// to a clean state" sweep (e.g. the app's UI-test launch reset) are the same +/// edit, instead of a hand-maintained list in the app shell that goes stale. public enum PersistedSettings { /// Every defaults key an engine store writes. Keep in sync by adding the new /// store's key here in the same change that introduces the store. @@ -11,5 +11,7 @@ public enum PersistedSettings { SoundPackStore.defaultsKey, KeyTermsStore.defaultsKey, DeveloperModeStore.defaultsKey, + OverlayOriginStore.xDefaultsKey, + OverlayOriginStore.yDefaultsKey, ] } diff --git a/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift b/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift new file mode 100644 index 0000000..9b97c1e --- /dev/null +++ b/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift @@ -0,0 +1,101 @@ +import AppKit +import ApplicationServices + +// The "can this target accept a paste?" half of `FocusCapture`, split out of +// `FocusCapture.swift` to stay within the lint file-length budget — the same +// reason `DictationSession` is split across `+Commands`/`+Observation`/`+Pipeline`. +// +// Cohesive as its own file: everything here serves `KeyInjector`'s pre-paste +// "no beep" guard, whereas `FocusCapture.swift` proper serves the press-time +// context capture that primes the STT prompt. +extension FocusCapture { + /// AX roles a focused element reports when it accepts typed/pasted text. + /// Includes `secureFieldRole`: a password field is a valid *paste* target even + /// though its contents are never read (see `isSecureField`). + private static let editableRoles: Set = [ + "AXTextField", "AXTextArea", "AXComboBox", secureFieldRole, "AXSearchField", + ] + + /// Pure decision: does a focused element with these signals accept pasted text? + /// The injector calls this just before a synthesized ⌘V — if it returns false the + /// paste is skipped (so macOS doesn't beep into a non-editable target) and the + /// transcript is left on the clipboard with a quiet "Copied" notice. + /// + /// Requires a *positive* editability signal: a known text role, a settable value, + /// or an insertion point. Anything else — a non-text control, an unknown role, or + /// no readable role — is treated as not editable, so we copy rather than beep a + /// ⌘V into a target that can't take it. + /// + /// AX-opaque Electron editors (VS Code, Slack) expose *none* of these signals + /// even for a genuine text field, so this returns false for them too — but the + /// injector still pastes into those via a separate Electron-app check (see + /// `isElectronApp` / `KeyInjector.insert`), so the user's words aren't dropped + /// to copy-only there. + static func isEditableTarget( + hasFocusedElement: Bool, role: String?, valueSettable: Bool, hasInsertionPoint: Bool + ) -> Bool { + guard hasFocusedElement else { return false } + if let role, editableRoles.contains(role) { return true } + return valueSettable || hasInsertionPoint + } + + /// Whether `app` is an Electron/Chromium-based app, detected by the bundled + /// Electron framework. Such apps ship with their accessibility tree off, so even + /// a focused text field exposes no editable AX signal and + /// `hasEditableFocusedElement` reads them as non-editable. They're the one case + /// the injector still pastes into on no signal (dropping the user's words into a + /// copy-only fallback would be the worse mistake). A native app with genuinely no + /// editable focus bundles no such framework and correctly falls back to copy. + static func isElectronApp(_ app: NSRunningApplication?) -> Bool { + guard let bundleURL = app?.bundleURL else { return false } + let electronFramework = bundleURL.appendingPathComponent( + "Contents/Frameworks/Electron Framework.framework") + return FileManager.default.fileExists(atPath: electronFramework.path) + } + + /// Whether the system-wide focused element can accept pasted text right now. + /// Read by `KeyInjector` (off the main actor, after it has activated the target + /// app) just before pasting — the Accessibility *client* read APIs are + /// thread-safe. Returns `true` whenever AX can't be consulted (process not + /// trusted) or can't resolve a focused element, so an unknowable state still + /// attempts the paste — the injector's own trust check then handles the + /// missing-permission case. + nonisolated static func hasEditableFocusedElement() -> Bool { + guard AXIsProcessTrusted() else { return true } + + guard let element = systemFocusedElement() else { + // AX is trusted but reports no focused element — e.g. a native app frontmost + // with nothing editable focused (Finder, the desktop, a button-only window). + // Posting ⌘V there only beeps, so treat it as non-editable and copy instead. + // AX-opaque Electron apps (VS Code, Slack) also expose no focused element + // here, but the injector's Electron-app check still pastes into those (see + // `KeyInjector.insert` / `isElectronApp`). + return false + } + + // Same checked reader `captureFieldContext` uses for this attribute, so the + // editability path and the secure-field path can't disagree about what a + // misbehaving app's role reads as. + let role = stringValue(element, kAXRoleAttribute) + + var settable = DarwinBoolean(false) + let valueSettable = + AXUIElementIsAttributeSettable(element, kAXValueAttribute as CFString, &settable) == .success + && settable.boolValue + + // A readable selected-text *range* means the element has an insertion point — + // the hallmark of a text input even when its value isn't reported settable. + // Require the range to actually decode, not merely that the read succeeded: a + // `.success` carrying a non-CFRange payload is not an insertion point, and this + // signal is one of the two that can green-light a ⌘V on an unknown role. + var rangeRef: CFTypeRef? + let hasInsertionPoint = + AXUIElementCopyAttributeValue( + element, kAXSelectedTextRangeAttribute as CFString, &rangeRef) == .success + && rangeRef.flatMap(axRange) != nil + + return isEditableTarget( + hasFocusedElement: true, role: role, valueSettable: valueSettable, + hasInsertionPoint: hasInsertionPoint) + } +} diff --git a/Sources/BlurtEngine/FocusCapture/FocusCapture.swift b/Sources/BlurtEngine/FocusCapture/FocusCapture.swift index 224cd84..7cb023c 100644 --- a/Sources/BlurtEngine/FocusCapture/FocusCapture.swift +++ b/Sources/BlurtEngine/FocusCapture/FocusCapture.swift @@ -47,9 +47,10 @@ enum FocusCapture { /// best-effort. Requires the same Accessibility permission the app already /// holds for paste injection, so it adds no new prompt. /// - /// Secure text fields (password inputs) are detected by role and never have - /// their contents read, so a typed password — selected or not — can't leak - /// into the STT prompt. + /// Secure text fields (password inputs) are detected by role **or** subrole and + /// never have their contents read, so a typed password — selected or not — can't + /// leak into the STT prompt. The check fails closed: an unreadable role is + /// treated as secure, since it can't be shown not to be. /// /// Deliberately `nonisolated`: each read below is a synchronous cross-process /// IPC round trip into the frontmost app, and an unresponsive app blocks the @@ -62,8 +63,14 @@ enum FocusCapture { guard AXIsProcessTrusted() else { return .empty } guard let element = systemFocusedElement() else { return .empty } - // Don't read the value of a password field into the prompt. - let isSecure = isSecureFieldRole(stringValue(element, kAXRoleAttribute)) + // Don't read the value of a password field into the prompt. Fails *closed*: + // an unreadable role (AX timeout against a busy app, a non-string value from a + // buggy one) means we can't prove the field isn't secure, so treat it as + // secure. The cost is losing priming for an already-degraded app; the cost of + // failing open is a password in an outbound API request — and, with developer + // mode on, in `dictations.jsonl`. + let role = stringValue(element, kAXRoleAttribute) + let isSecure = role == nil || isSecureField(role: role, subrole: stringValue(element, kAXSubroleAttribute)) // `visibleTextOrNil` collapses an all-invisible read (e.g. Google Docs' lone // U+200B before the caret) to nil so it can't masquerade as real prior text. let prior = isSecure ? nil : visibleTextOrNil(priorText(of: element, maxChars: maxPriorChars)) @@ -82,11 +89,13 @@ enum FocusCapture { /// context capture is best-effort priming, so partial answers beat waiting. private static let axMessagingTimeoutSeconds: Float = 1 + // Internal, not private: `hasEditableFocusedElement` in + // FocusCapture+Editability.swift calls it from another file. /// The system-wide focused UI element, or `nil` when none is resolvable /// (process not trusted, or nothing focused). The Accessibility *client* read /// APIs are thread-safe, so this serves both the off-main context capture /// and the injector's off-main editability check. - private nonisolated static func systemFocusedElement() -> AXUIElement? { + nonisolated static func systemFocusedElement() -> AXUIElement? { let system = AXUIElementCreateSystemWide() // Setting the timeout on the system-wide element applies it process-wide // (per AXUIElement.h), bounding this focused-element lookup AND every later @@ -166,8 +175,10 @@ enum FocusCapture { } } - // Fallback: read the full value and clip to the caret (or the tail). - return priorSlice(full: stringValue(element, kAXValueAttribute) ?? "", caret: caret, maxChars: maxChars) + // Fallback: read the full value and clip to the caret (or the tail). Read it + // RAW — `caret` is a UTF-16 offset into the untrimmed value, so a trimmed + // string would be indexed with offsets that no longer refer to it. + return priorSlice(full: rawStringValue(element, kAXValueAttribute) ?? "", caret: caret, maxChars: maxChars) } /// Up to `maxChars` of selected text, or `nil` when the element exposes no @@ -210,14 +221,31 @@ enum FocusCapture { roleDescription: stringValue(element, kAXRoleDescriptionAttribute)) } + // Internal for the same reason as `systemFocusedElement`: the editability path in + // FocusCapture+Editability.swift reads the role through it. /// Reads a `String`-valued AX attribute, returning `nil` for missing, - /// non-string, or blank values. - private nonisolated static func stringValue(_ element: AXUIElement, _ attribute: String) -> String? { + /// non-string, or blank values. Trims, which is what the *label-ish* attributes + /// want (role, title, placeholder, description). Do **not** use it for + /// `kAXValueAttribute` when a caret offset will index the result — see + /// `rawStringValue`. + nonisolated static func stringValue(_ element: AXUIElement, _ attribute: String) -> String? { + rawStringValue(element, attribute)?.trimmedNonEmpty() + } + + /// Reads a `String`-valued AX attribute **verbatim** — no trimming, so a + /// caret offset taken from the same element still indexes it correctly. + /// + /// `kAXSelectedTextRange` locations are UTF-16 offsets into the element's + /// *original* value. Trimming shifts every offset past a leading whitespace run + /// and shortens the string, so slicing a trimmed value with an untrimmed caret + /// silently returns the wrong text (or falls back to the whole tail, which + /// destroys the trailing-whitespace signal `withLeadingSeparator` reads). + private nonisolated static func rawStringValue(_ element: AXUIElement, _ attribute: String) -> String? { var ref: CFTypeRef? guard AXUIElementCopyAttributeValue(element, attribute as CFString, &ref) == .success, let value = ref as? String else { return nil } - return value.trimmedNonEmpty() + return value } /// Reads an `AXUIElement`-valued AX attribute (e.g. the containing window). @@ -229,94 +257,6 @@ enum FocusCapture { return axElement(ref) } - // MARK: - Editable-target detection (for the injector's "no beep" guard) - - /// AX roles a focused element reports when it accepts typed/pasted text. - /// Includes `secureFieldRole`: a password field is a valid *paste* target even - /// though its contents are never read (see `isSecureFieldRole`). - private static let editableRoles: Set = [ - "AXTextField", "AXTextArea", "AXComboBox", secureFieldRole, "AXSearchField", - ] - - /// Pure decision: does a focused element with these signals accept pasted text? - /// The injector calls this just before a synthesized ⌘V — if it returns false the - /// paste is skipped (so macOS doesn't beep into a non-editable target) and the - /// transcript is left on the clipboard with a quiet "Copied" notice. - /// - /// Requires a *positive* editability signal: a known text role, a settable value, - /// or an insertion point. Anything else — a non-text control, an unknown role, or - /// no readable role — is treated as not editable, so we copy rather than beep a - /// ⌘V into a target that can't take it. - /// - /// AX-opaque Electron editors (VS Code, Slack) expose *none* of these signals - /// even for a genuine text field, so this returns false for them too — but the - /// injector still pastes into those via a separate Electron-app check (see - /// `isElectronApp` / `KeyInjector.insert`), so the user's words aren't dropped - /// to copy-only there. - static func isEditableTarget( - hasFocusedElement: Bool, role: String?, valueSettable: Bool, hasInsertionPoint: Bool - ) -> Bool { - guard hasFocusedElement else { return false } - if let role, editableRoles.contains(role) { return true } - return valueSettable || hasInsertionPoint - } - - /// Whether `app` is an Electron/Chromium-based app, detected by the bundled - /// Electron framework. Such apps ship with their accessibility tree off, so even - /// a focused text field exposes no editable AX signal and - /// `hasEditableFocusedElement` reads them as non-editable. They're the one case - /// the injector still pastes into on no signal (dropping the user's words into a - /// copy-only fallback would be the worse mistake). A native app with genuinely no - /// editable focus bundles no such framework and correctly falls back to copy. - static func isElectronApp(_ app: NSRunningApplication?) -> Bool { - guard let bundleURL = app?.bundleURL else { return false } - let electronFramework = bundleURL.appendingPathComponent( - "Contents/Frameworks/Electron Framework.framework") - return FileManager.default.fileExists(atPath: electronFramework.path) - } - - /// Whether the system-wide focused element can accept pasted text right now. - /// Read by `KeyInjector` (off the main actor, after it has activated the target - /// app) just before pasting — the Accessibility *client* read APIs are - /// thread-safe. Returns `true` whenever AX can't be consulted (process not - /// trusted) or can't resolve a focused element, so an unknowable state still - /// attempts the paste — the injector's own trust check then handles the - /// missing-permission case. - nonisolated static func hasEditableFocusedElement() -> Bool { - guard AXIsProcessTrusted() else { return true } - - guard let element = systemFocusedElement() else { - // AX is trusted but reports no focused element — e.g. a native app frontmost - // with nothing editable focused (Finder, the desktop, a button-only window). - // Posting ⌘V there only beeps, so treat it as non-editable and copy instead. - // AX-opaque Electron apps (VS Code, Slack) also expose no focused element - // here, but the injector's Electron-app check still pastes into those (see - // `KeyInjector.insert` / `isElectronApp`). - return false - } - - var roleRef: CFTypeRef? - let role = - AXUIElementCopyAttributeValue(element, kAXRoleAttribute as CFString, &roleRef) == .success - ? roleRef as? String : nil - - var settable = DarwinBoolean(false) - let valueSettable = - AXUIElementIsAttributeSettable(element, kAXValueAttribute as CFString, &settable) == .success - && settable.boolValue - - // A readable selected-text *range* means the element has an insertion point — - // the hallmark of a text input even when its value isn't reported settable. - var rangeRef: CFTypeRef? - let hasInsertionPoint = - AXUIElementCopyAttributeValue( - element, kAXSelectedTextRangeAttribute as CFString, &rangeRef) == .success - - return isEditableTarget( - hasFocusedElement: true, role: role, valueSettable: valueSettable, - hasInsertionPoint: hasInsertionPoint) - } - // MARK: - Pure helpers (no Accessibility I/O — unit-testable in isolation) /// The AX role password inputs report. `captureFieldContext` never reads the @@ -326,9 +266,14 @@ enum FocusCapture { static let secureFieldRole = "AXSecureTextField" /// Pure decision behind the password-redaction guard in `captureFieldContext`: - /// does this focused-element role mean its contents must never be read? - static func isSecureFieldRole(_ role: String?) -> Bool { - role == secureFieldRole + /// do this focused element's role/subrole mean its contents must never be read? + static func isSecureField(role: String?, subrole: String?) -> Bool { + // AppKit ships both NSAccessibilitySecureTextFieldRole and + // NSAccessibilitySecureTextFieldSubrole (both the same string), and an element + // may report a generic `AXTextField` role while expressing secure-ness only + // through its subrole — browser password inputs and custom secure fields are + // the population that does this. Checking role alone misses them entirely. + role == secureFieldRole || subrole == secureFieldRole } /// Collapses an AX text read that carries no *visible* content to `nil`. diff --git a/Sources/BlurtEngine/Injection/SystemClipboard.swift b/Sources/BlurtEngine/Injection/SystemClipboard.swift index 439616e..e9456fd 100644 --- a/Sources/BlurtEngine/Injection/SystemClipboard.swift +++ b/Sources/BlurtEngine/Injection/SystemClipboard.swift @@ -7,6 +7,31 @@ struct SendablePasteboardItem: Sendable { let dataMap: [NSPasteboard.PasteboardType: Data] } +/// A best-effort snapshot of the whole pasteboard, taken before a paste so the +/// user's contents can be put back afterwards. +/// +/// The distinction that matters: this type is only ever produced when the +/// pasteboard was *readable*. `SystemClipboard.snapshot()` returns `nil` when the +/// read itself failed, so a failure can never be mistaken for "the clipboard was +/// empty" — conflating those two is what made the restore clear the user's +/// clipboard instead of restoring it. +/// +/// Known, inherent limitation: pasteboard data can be *promised* (provided lazily +/// by the owning app on demand). A promise cannot be copied, only materialized, and +/// an app may decline. Types that decline are absent from `dataMap`, so a restore +/// of promise-backed contents is a best-effort downgrade, not a byte-faithful +/// round trip — `plainText` exists to keep at least the text when that happens. +struct PasteboardSnapshot: Sendable { + /// One entry per item the pasteboard held, in order. An entry with an empty + /// `dataMap` means the item existed but none of its representations could be + /// materialized — distinct from the pasteboard having held no items at all, + /// which is `items.isEmpty`. + let items: [SendablePasteboardItem] + /// The pasteboard's plain-string flavor, kept separately as a restore floor for + /// when no item's representations could be materialized. + let plainText: String? +} + /// The two clipboard operations `KeyInjector` actually performs around a paste — /// a plain overwrite, or an overwrite that can later restore what it displaced — /// rather than exposing NSPasteboard's raw change-count/multi-item bookkeeping. @@ -31,7 +56,7 @@ struct SystemClipboard: ClipboardAccess { func write(_ text: String) { setString(text) } func writeAndPrepareRestore(_ text: String) -> @Sendable () -> Void { - let saved = currentItems() + let saved = snapshot() setString(text) // Snapshot the change count our own write produced. If anything else writes // to the pasteboard before the restore fires (e.g. the user copies @@ -40,6 +65,11 @@ struct SystemClipboard: ClipboardAccess { let ourChangeCount = changeCount return { [self] in guard changeCount == ourChangeCount else { return } + // A nil snapshot means the pasteboard could not be read at all. There is + // nothing to put back, so leave the transcript on the clipboard (the same + // degraded-but-recoverable outcome as the `.noTarget` path) rather than + // clearing the user's clipboard to nothing. + guard let saved else { return } restore(saved) } } @@ -48,17 +78,24 @@ struct SystemClipboard: ClipboardAccess { var changeCount: Int { NSPasteboard.general.changeCount } - func currentItems() -> [SendablePasteboardItem] { - guard let items = NSPasteboard.general.pasteboardItems else { return [] } - return items.map { item in + /// Snapshots the pasteboard, or `nil` when it can't be read at all + /// (`pasteboardItems` is documented to return nil on error). Callers must treat + /// nil as "don't restore" — never as an empty clipboard. + func snapshot() -> PasteboardSnapshot? { + let pb = NSPasteboard.general + guard let items = pb.pasteboardItems else { return nil } + let captured = items.map { item in var dataMap: [NSPasteboard.PasteboardType: Data] = [:] for type in item.types { + // A nil read is a promised representation the owning app declined to + // materialize; record what we did get and let `plainText` be the floor. if let data = item.data(forType: type) { dataMap[type] = data } } return SendablePasteboardItem(dataMap: dataMap) } + return PasteboardSnapshot(items: captured, plainText: pb.string(forType: .string)) } func setString(_ text: String) { @@ -67,17 +104,51 @@ struct SystemClipboard: ClipboardAccess { pb.setString(text, forType: .string) } - func restore(_ items: [SendablePasteboardItem]) { + func restore(_ saved: PasteboardSnapshot) { let pb = NSPasteboard.general - pb.clearContents() - guard !items.isEmpty else { return } - let pbItems = items.map { item in + + // The pasteboard genuinely held nothing, so restoring it means emptying it. + // Safe to clear because the snapshot succeeded — a *failed* read is nil and + // never reaches here. + guard !saved.items.isEmpty else { + pb.clearContents() + return + } + + // Build the items BEFORE clearing. Clearing first and then discovering there + // is nothing to write is how the user's clipboard got destroyed whenever a + // snapshot came back degraded. + let pbItems = saved.items.compactMap { item -> NSPasteboardItem? in + guard !item.dataMap.isEmpty else { return nil } let pbItem = NSPasteboardItem() for (type, data) in item.dataMap { pbItem.setData(data, forType: type) } return pbItem } - pb.writeObjects(pbItems) + + if !pbItems.isEmpty { + pb.clearContents() + // `writeObjects` can refuse the batch; fall through to the text floor + // rather than leaving the pasteboard empty. + if pb.writeObjects(pbItems) { return } + } + + // Either nothing was materializable, or the write above was refused. Put the + // text back if we have it. + // + // Precise about the one case this does NOT recover: if items DID materialize, + // `writeObjects` refused them, and there was no plain-string flavor, the + // pasteboard has already been cleared and stays empty. That is not gated on + // `plainText` being non-nil on purpose — skipping the item write whenever + // there's no text flavor would refuse to restore an image-only or + // file-only clipboard, which is a far more common clipboard than a refused + // batch write. `writeObjects` failing on a freshly-cleared pasteboard holding + // valid `NSPasteboardItem`s is a programming error, not a runtime condition, + // and NSPasteboard offers no way to test a write before clearing. + if let plainText = saved.plainText { + pb.clearContents() + pb.setString(plainText, forType: .string) + } } } diff --git a/Sources/BlurtEngine/Pipeline/DictationSession+Pipeline.swift b/Sources/BlurtEngine/Pipeline/DictationSession+Pipeline.swift index d34bbfb..b4877e4 100644 --- a/Sources/BlurtEngine/Pipeline/DictationSession+Pipeline.swift +++ b/Sources/BlurtEngine/Pipeline/DictationSession+Pipeline.swift @@ -5,9 +5,6 @@ import os // the press-time context read — split from `DictationSession.swift` to stay // within the lint file-length budget, like `+Commands` and `+Observation`. extension DictationSession { - /// Sample rate the mic delivers and the Sync request declares (`SyncSTTLimits`). - private static let captureSampleRate = SyncSTTLimits.sampleRate - /// The longest `runTranscribeInject` waits for the press-time AX /// field-context read before transcribing without it. In the common case the /// read finished while the user was speaking and the buffered stream hands @@ -37,13 +34,23 @@ extension DictationSession { // Resolve the press-time AX field read now that it's actually needed — // waiting at most `contextWaitBudget` (see its doc), so a hung read costs // the transcript its priming, not multiple seconds of stall. - if let contextStream { + // Take the stream out of the actor's state in the SAME turn it's read, before + // the suspension below. Reading it and clearing it across an `await` let a + // cancelled pipeline clear a *newer* press's stream: `cancel()` detaches this + // task while it's parked in `firstValue`, a fresh `press()` installs its own + // `contextStream`, and this task's resumption then nils that one out — so + // dictation #2 transcribes with `context: nil`, losing not just its priming but + // `baseInstruction`, and `[Speaker]`-style markers can reach the pasted text. + // The window is microseconds, but the invariant is now local instead of + // depending on scheduling. + let stream = contextStream + contextStream = nil + if let stream { capturedContext = await Self.firstValue( - of: contextStream, within: Self.contextWaitBudget, clock: clock) + of: stream, within: Self.contextWaitBudget, clock: clock) } else { capturedContext = nil } - contextStream = nil // The Sync STT API applies the cleanup prompt server-side, so the transcript // it returns is already the final, polished text — there is no separate @@ -95,7 +102,7 @@ extension DictationSession { private func transcribe(pcm: Data) async -> String? { do { return try await transcriber.transcribe( - pcm: pcm, sampleRate: Self.captureSampleRate, context: capturedContext) + pcm: pcm, sampleRate: SyncSTTLimits.sampleRate, context: capturedContext) } catch { // A cancel() that landed mid-request already tore this task down and set // .cancelled; the transport then surfaces a cancellation-shaped error diff --git a/Sources/BlurtEngine/Pipeline/DictationSession.swift b/Sources/BlurtEngine/Pipeline/DictationSession.swift index aa4b341..3cb5ccf 100644 --- a/Sources/BlurtEngine/Pipeline/DictationSession.swift +++ b/Sources/BlurtEngine/Pipeline/DictationSession.swift @@ -2,6 +2,12 @@ import Foundation import os public actor DictationSession { + /// Off-pool home for the press-time AX field read — see its use in + /// `performPress` for why blocking IPC must not run on the cooperative pool. + private static let contextQueue = DispatchQueue( + label: "\(BlurtIdentity.subsystem).FieldContext", qos: .userInitiated, + attributes: .concurrent) + public private(set) var phase: PipelinePhase = .idle // Split for the lint file-length budget: `phaseStream()`/os_signpost live in @@ -189,7 +195,18 @@ public actor DictationSession { let (stream, contextFeed) = AsyncStream.makeStream( of: TranscriptionContext?.self, bufferingPolicy: .bufferingNewest(1)) contextStream = stream - Task.detached { + // A Dispatch queue, not `Task.detached`: `captureFieldContext` is documented + // as making ~6 synchronous cross-process AX round trips, each bounded only by + // the 1 s messaging timeout, so against a beachballing frontmost app one + // press can *block* a thread for seconds. The Swift cooperative pool is sized + // to the core count and does not overcommit, so a few press/cancel cycles + // against a hung app could park every cooperative thread and stall the whole + // non-main runtime — including this actor. Dispatch overcommits, so a blocked + // capture costs a thread instead of the pool. Same reasoning as + // `DictationLog`'s serial queue. Concurrent so a hung capture can't delay the + // next press's. The body is fully synchronous and captures only Sendable + // values, so it needs no task context. + Self.contextQueue.async { let field = FocusCapture.captureFieldContext() let context = TranscriptionContext( appName: captured?.processName, diff --git a/Sources/BlurtEngine/Pipeline/OverlayOriginStore.swift b/Sources/BlurtEngine/Pipeline/OverlayOriginStore.swift new file mode 100644 index 0000000..01150f9 --- /dev/null +++ b/Sources/BlurtEngine/Pipeline/OverlayOriginStore.swift @@ -0,0 +1,50 @@ +import CoreGraphics +import Foundation + +/// Persists the overlay pill's user-dragged origin. Same shape as +/// `TriggerKeyStore` / `SoundPackStore`, and registered in +/// `PersistedSettings.allDefaultsKeys`. +/// +/// Owned by the engine rather than the AppKit controller for the reason the +/// roster exists: these two keys used to be private to `OverlayWindowController`, +/// so no reset sweep knew about them — a pill dragged during a UI-test run (or by +/// `reset-install.sh`'s "clean install" path) survived into later runs, which is +/// exactly the staleness `PersistedSettings` was created to prevent. The clamping +/// this value feeds is already engine-side in `OverlayPlacement`, so its +/// persistence belongs next to it. +public struct OverlayOriginStore { + /// Public so the reset sweep and `@AppStorage`-style observers can name them. + public static let xDefaultsKey = "BlurtOverlayCustomOriginX" + public static let yDefaultsKey = "BlurtOverlayCustomOriginY" + + private let defaults: UserDefaults + + public init(defaults: UserDefaults = .standard) { + self.defaults = defaults + } + + /// The dragged origin, or `nil` when the pill has never been moved. + /// + /// Both components must be present: `double(forKey:)` reports 0 for a missing + /// key, so a half-written pair would otherwise place the pill at an implied + /// origin instead of falling back to the default placement. + public var origin: CGPoint? { + get { + guard defaults.object(forKey: Self.xDefaultsKey) != nil, + defaults.object(forKey: Self.yDefaultsKey) != nil + else { return nil } + return CGPoint( + x: defaults.double(forKey: Self.xDefaultsKey), + y: defaults.double(forKey: Self.yDefaultsKey)) + } + nonmutating set { + guard let newValue else { + defaults.removeObject(forKey: Self.xDefaultsKey) + defaults.removeObject(forKey: Self.yDefaultsKey) + return + } + defaults.set(Double(newValue.x), forKey: Self.xDefaultsKey) + defaults.set(Double(newValue.y), forKey: Self.yDefaultsKey) + } + } +} diff --git a/Sources/BlurtEngine/Pipeline/OverlayPlacement.swift b/Sources/BlurtEngine/Pipeline/OverlayPlacement.swift index 042fb28..5fa659b 100644 --- a/Sources/BlurtEngine/Pipeline/OverlayPlacement.swift +++ b/Sources/BlurtEngine/Pipeline/OverlayPlacement.swift @@ -5,6 +5,32 @@ import CoreGraphics /// in the screen's visible frame and the persisted drag origin; only the /// `NSScreen`/`NSPanel` plumbing stays in the shell. public enum OverlayPlacement { + /// Default clearance between the bottom of the *visible pill* and the bottom of + /// the screen's visible frame. + /// + /// Measured to the pill, not the panel: the panel is deliberately larger so the + /// drop shadow isn't clipped, so a caller placing the panel must back this off + /// by that transparent inset. `panelOrigin(…shadowMargin:)` does that here, + /// where it's unit-tested — when the arithmetic lived at the AppKit call site, + /// changing the shadow margin silently moved the pill and no test noticed. + public static let defaultBottomClearance: CGFloat = 80 + + /// Resolves the *panel* origin from what the shell genuinely owns — the screen's + /// visible frame, the persisted drag origin, and the panel's transparent shadow + /// inset — keeping the clearance policy and the pill/panel conversion in here. + public static func panelOrigin( + panelSize: CGSize, + visibleFrame: CGRect, + customOrigin: CGPoint?, + shadowMargin: CGFloat + ) -> CGPoint { + origin( + panelSize: panelSize, + visibleFrame: visibleFrame, + customOrigin: customOrigin, + bottomOffset: defaultBottomClearance - shadowMargin) + } + /// Resolves the panel's origin: the user's dragged `customOrigin` clamped /// fully back onto the current screen, or — when the pill has never been /// moved — the default placement, horizontally centered with its bottom edge diff --git a/Sources/BlurtEngine/Pipeline/OverlayUIState.swift b/Sources/BlurtEngine/Pipeline/OverlayUIState.swift index 2afbe97..19b1ca9 100644 --- a/Sources/BlurtEngine/Pipeline/OverlayUIState.swift +++ b/Sources/BlurtEngine/Pipeline/OverlayUIState.swift @@ -55,13 +55,23 @@ extension PipelinePhase { /// How this phase should be presented on the overlay pill. public var overlayState: OverlayUIState { switch self { - case .idle, .injecting, .cancelled: .idle + case .idle, .cancelled: .idle + // `.injecting` maps to `.processing`, NOT `.idle`. The shell reads an `.idle` + // projection as "dismiss the pill", so mapping this working phase to idle + // started a fade-out mid-dictation: two wasted animation groups on the fast + // path, and on the slow one (the injector's activation wait runs up to 350 ms) + // the fade completed, ordering the panel out and tearing down the pill's + // content — then `.pasted` arrived and faded it back in. A visible blink at + // the end of a dictation, worst when the user has switched apps mid-transcribe. + case .injecting: .processing case .recording: .recording case .transcribing: .processing - // A missing API key is an expected setup state, not a fault: the shell - // routes it to the settings window (the actionable fix), so the pill stays - // calm idle rather than flashing red on the way there. - case .failed(.apiKeyMissing): .idle + // A setup blocker (a missing API key) is an expected state, not a fault: the + // shell routes it to the settings window — the actionable fix — so the pill + // stays calm idle rather than flashing red on the way there. The + // classification is `PipelinePhase.setupBlocker`, so this and the shell's + // navigation can't disagree about which failures are setup states. + case .failed(let error) where error.isSetupBlocker: .idle case .failed(let error): .error(message: error.errorDescription ?? "Dictation failed.") case .pasted: .pasted case .noTarget: .noTarget diff --git a/Sources/BlurtEngine/Pipeline/PipelinePhase.swift b/Sources/BlurtEngine/Pipeline/PipelinePhase.swift index 406710d..b5fd53f 100644 --- a/Sources/BlurtEngine/Pipeline/PipelinePhase.swift +++ b/Sources/BlurtEngine/Pipeline/PipelinePhase.swift @@ -23,6 +23,34 @@ public enum PipelinePhase: Equatable, Sendable { default: false } } + + /// The blocker behind this phase when it represents an unfinished **setup** + /// step rather than a fault — something the user must go and fix, not a + /// dictation that broke. Nil for every other phase and every genuine failure. + /// + /// Owned here so the classification exists once. Two consumers act on it and + /// they must agree: `overlayState` renders it as calm `.idle` (no red flash on + /// the way to the fix), and the host routes it to whatever surfaces setup. When + /// both re-derived it by pattern-matching `.failed(.apiKeyMissing)`, adding a + /// second blocker — a press-time mic-permission check is the obvious next one — + /// meant remembering both sites, and missing either gives a red flash with no + /// route to the fix, or a press that silently does nothing. + public var setupBlocker: BlurtError? { + guard case .failed(let error) = self, error.isSetupBlocker else { return nil } + return error + } +} + +extension BlurtError { + /// True for errors that mean "setup isn't finished" rather than "dictation + /// failed". Internal: hosts ask `PipelinePhase.setupBlocker`, which is the form + /// they actually need. + var isSetupBlocker: Bool { + switch self { + case .apiKeyMissing: true + default: false + } + } } extension BlurtError: Equatable { diff --git a/Sources/BlurtEngine/Pipeline/RecentDictations.swift b/Sources/BlurtEngine/Pipeline/RecentDictations.swift index 724218d..223542f 100644 --- a/Sources/BlurtEngine/Pipeline/RecentDictations.swift +++ b/Sources/BlurtEngine/Pipeline/RecentDictations.swift @@ -9,6 +9,15 @@ import Foundation public struct RecentDictations: Equatable, Sendable { /// One recorded dictation: the transcript plus when it landed. public struct Entry: Identifiable, Equatable, Sendable { + /// How long after a dictation `relativeLabel` keeps saying "just now", in + /// seconds — the smallest unit it shows above this is minutes. + /// + /// Public because the ready screen's timestamp refresh cadence is chosen + /// against it: a view redrawing slower than this leaves rows stale. Published + /// rather than restated so the two can't drift, the same reason + /// `MicCapture.meterIntervalSeconds` is public. + public static let justNowThreshold: TimeInterval = 60 + /// Stable identity for SwiftUI list diffing — assigned once at creation, so /// an entry keeps its id as newer dictations push in ahead of it. public let id = UUID() @@ -42,7 +51,7 @@ extension RecentDictations.Entry { /// landed), then the full relative phrasing ("2 minutes ago"). `now` is /// injected so tests are deterministic; `locale` so they can pin the wording. public func relativeLabel(now: Date, locale: Locale = .autoupdatingCurrent) -> String { - if now.timeIntervalSince(timestamp) < 60 { + if now.timeIntervalSince(timestamp) < Self.justNowThreshold { return "just now" } // Built per call rather than cached: a stored formatter would be shared diff --git a/Sources/BlurtEngine/STT/AssemblyAITranscriber.swift b/Sources/BlurtEngine/STT/AssemblyAITranscriber.swift index fd3937d..d26d883 100644 --- a/Sources/BlurtEngine/STT/AssemblyAITranscriber.swift +++ b/Sources/BlurtEngine/STT/AssemblyAITranscriber.swift @@ -14,15 +14,28 @@ private let transcriberLog = Logger(subsystem: BlurtIdentity.subsystem, category /// PCM, exactly the bytes the mic recorded — there is no re-encoding pass) /// plus a JSON `config` part, and the finished transcript comes back in the /// response body. No upload step, no job submission, no polling — one request -/// per utterance. The Universal-3 sync model (`u3-sync-pro`) handles audio from +/// per utterance. The sync model (`universal-3-5-pro`) handles audio from /// ~80 ms up to 120 s with a server-side inference deadline of ~30 s. public struct AssemblyAITranscriber: TranscriberProtocol { private let apiKeyProvider: @Sendable () -> String? private let baseURL: URL private let transport: any HTTPTransport - /// Required on every Sync API request — selects the synchronous STT model. - private static let syncModel = "u3-sync-pro" + /// Required on every Sync API request — selects the synchronous STT model. The + /// canonical identifier, and the only value in the endpoint's schema enum; + /// `u3-sync-pro` and `u3-pro` are accepted only as legacy aliases, so there's no + /// reason for a new request to send one. + private static let syncModel = "universal-3-5-pro" + + /// Idle timeout for the transcribe round trip — `URLRequest.timeoutInterval` is + /// reset each time data moves, so this bounds *stalls*, not total elapsed time. + /// That is the failure worth bounding here: the server enforces its own ~30 s + /// inference deadline (answering 504 past it), so a request that keeps making + /// progress will finish or be rejected on its own, while a connection that stops + /// delivering bytes would otherwise sit on URLRequest's 60 s default with the + /// pill stuck on "Transcribing…". Sized above the server deadline so a slow but + /// live inference is never cut off client-side. + private static let requestTimeoutSeconds: TimeInterval = 45 public init( apiKeyProvider: @escaping @Sendable () -> String? = { APIKeyStore.get() }, @@ -48,14 +61,16 @@ public struct AssemblyAITranscriber: TranscriberProtocol { var request = URLRequest(url: baseURL.appendingPathComponent("transcribe")) request.httpMethod = "POST" + // Bounds a stalled connection; see `requestTimeoutSeconds` for why an idle + // timeout is the right shape here. + request.timeoutInterval = Self.requestTimeoutSeconds request.setValue(apiKey, forHTTPHeaderField: "Authorization") request.setValue(Self.syncModel, forHTTPHeaderField: "X-AAI-Model") request.setValue( "multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") let body = multipartBody(pcm: pcm, config: config, boundary: boundary) - let sampleCount = pcm.count / SyncSTTLimits.bytesPerSample - let audioDurationMs = Int((Double(sampleCount) / Double(sampleRate)) * 1000) + let audioDurationMs = SyncSTTLimits.durationMs(ofPCMBytes: pcm.count, rate: sampleRate) let data = try await send(request, body: body, audioDurationMs: audioDurationMs) guard let response = try? JSONDecoder().decode(SyncTranscriptResponse.self, from: data) else { throw AssemblyAIError.malformedResponse @@ -145,11 +160,14 @@ public struct AssemblyAITranscriber: TranscriberProtocol { return data } - /// Best human-readable explanation for a non-2xx response. The Sync API isn't - /// consistent about the field name across error classes (`error`, `message`, - /// and `detail` have all been seen), so try each; failing that, fall back to - /// the raw body text (trimmed and capped) so a failure never reaches the user - /// as a bare status code with no context. Returns nil only for an empty body. + /// Best human-readable explanation for a non-2xx response: `message`, then + /// `detail` (the two documented shapes — see `ErrorResponse`), then the raw body + /// text, trimmed and capped. + /// + /// The raw-body arm is deliberately kept. It is not compatibility with an old + /// API shape — it is what turns a response the API never promised (a proxy's HTML + /// 502, a captive-portal page) into something diagnosable instead of a bare + /// status code. Returns nil only for an empty body. static func errorMessage(from data: Data) -> String? { if let parsed = try? JSONDecoder().decode(ErrorResponse.self, from: data), let message = parsed.message @@ -180,15 +198,19 @@ public struct AssemblyAITranscriber: TranscriberProtocol { let text: String } - /// A Sync API failure body. The endpoint labels the explanation differently - /// across error classes, so pull it from whichever of `error` / `message` / - /// `detail` is present and string-valued (a non-string `detail`, e.g. FastAPI's - /// validation array, is ignored — the caller then falls back to the raw body). + /// A Sync API failure body. The reference documents exactly two shapes: + /// `{error_code, message}` for the request/audio/server errors (400, 413, 415, + /// 500, 503, 504) and `{detail}` for auth and rate limiting (401, 429) — so read + /// `message`, then `detail`. A non-string `detail` (a FastAPI-style validation + /// array) is ignored and the caller falls back to the raw body. + /// + /// A third `error` key used to be tried first. It is in none of the documented + /// responses, so it was speculative — removed rather than carried as a guess. private struct ErrorResponse: Decodable { let message: String? enum CodingKeys: String, CodingKey { - case error, message, detail + case message, detail } init(from decoder: Decoder) throws { @@ -196,7 +218,7 @@ public struct AssemblyAITranscriber: TranscriberProtocol { func string(_ key: CodingKeys) -> String? { (try? container.decodeIfPresent(String.self, forKey: key)) ?? nil } - message = string(.error) ?? string(.message) ?? string(.detail) + message = string(.message) ?? string(.detail) } } } diff --git a/Sources/BlurtEngine/STT/SyncSTTLimits.swift b/Sources/BlurtEngine/STT/SyncSTTLimits.swift index 32c3651..5317d50 100644 --- a/Sources/BlurtEngine/STT/SyncSTTLimits.swift +++ b/Sources/BlurtEngine/STT/SyncSTTLimits.swift @@ -35,6 +35,23 @@ public enum SyncSTTLimits { /// `DictationSession` applies to the blob `MicCaptureProtocol.stop()` returns. public static let minPCMBytes = minSamples * bytesPerSample + /// Milliseconds of audio a raw S16LE byte count represents at `rate`. Both the + /// capture side (`MicCapture.stop`) and the upload side + /// (`AssemblyAITranscriber.transcribe`) log the clip's duration, and those logs + /// are the documented way to diagnose pipeline latency — deriving it here, from + /// the two facts this type already owns, keeps them from disagreeing if the + /// geometry ever changes. Internal for the same reason as `bytesPerSample`: + /// only the capture/upload code needs it. + static func durationMs(ofPCMBytes byteCount: Int, rate: Int = sampleRate) -> Int { + guard rate > 0 else { return 0 } + // Convert before dividing: `byteCount / bytesPerSample` in integer arithmetic + // truncates a trailing partial sample. That can't happen for well-formed S16LE + // (byte counts are always even), and the error bound is one sample — 0.0625 ms + // at 16 kHz — but there's no reason for the expression to be wrong for an odd + // count it might one day be handed. + return Int((Double(byteCount) / Double(bytesPerSample) / Double(rate)) * 1000) + } + /// Safety margin subtracted from the cap for the auto-release timeout, so the /// session stops recording before it hits the hard limit. public static let autoReleaseMargin: Double = 5 diff --git a/Sources/BlurtEngine/STT/TranscriptionPrompt.swift b/Sources/BlurtEngine/STT/TranscriptionPrompt.swift index 9ff6170..165a5b8 100644 --- a/Sources/BlurtEngine/STT/TranscriptionPrompt.swift +++ b/Sources/BlurtEngine/STT/TranscriptionPrompt.swift @@ -28,7 +28,7 @@ /// prior-chunk context, the topic hint, and the destination sentence precede it /// as the `{context}. {baseInstruction}` shape, and keyword boosting trails it /// inline as `Keywords: a, b, c.` (per the mid-training instruction-type -/// reference). It stays under the API's cap (`characterCap`): the contextual +/// reference). It stays under a self-imposed ceiling (`characterCap`): the contextual /// blocks are clipped upstream in `FocusCapture`, and the key-terms clause is /// fitted to the remaining budget here. Exercised by /// `Tests/BlurtEngineTests/TranscriptionPromptTests.swift`. @@ -42,26 +42,29 @@ enum TranscriptionPrompt { static let baseInstruction = "Transcribe without speaker labels, audio event descriptions, or emotion markers." - /// Hard cap the Sync API places on `config.prompt`; a longer prompt fails - /// the whole request, so `build` must never exceed it. The contextual blocks - /// are all clipped upstream in `FocusCapture`; the user's key terms are the - /// one unbounded input, so `build` fits them to whatever budget remains. + /// Self-imposed ceiling on the built prompt. The Sync API reference documents + /// no cap on `config.prompt` (the 4096-character figure it does document belongs + /// to `conversation_context`, where over-cap content is trimmed rather than + /// rejected), so this is a sanity bound, not a documented limit — an unbounded + /// prompt is a latency and cost risk regardless. The contextual blocks are all + /// clipped upstream in `FocusCapture`; the user's key terms are the one + /// unbounded input, so `build` fits them to whatever budget remains. static let characterCap = 4096 /// Renders `context` into a Sync STT prompt, or `nil` when there is no usable /// context (the server then applies its own default prompt). static func build(context: TranscriptionContext?) -> String? { - guard let context else { return nil } + // `isEmpty` is the context type's own "no usable content" rule — the same + // predicate `DictationSession.performPress` gates on before yielding a + // context. Asking it here (rather than re-deriving the field-by-field test) + // keeps a newly added context signal from being silently dropped. + guard let context, !context.isEmpty else { return nil } let prior = context.priorText.trimmedNonEmpty() ?? "" let selected = context.selectedText.trimmedNonEmpty() ?? "" let app = context.appName.trimmedNonEmpty() ?? "" let window = context.windowTitle.trimmedNonEmpty() ?? "" let field = context.fieldLabel.trimmedNonEmpty() ?? "" let keyTerms = context.keyTerms - guard - !prior.isEmpty || !selected.isEmpty || !app.isEmpty || !window.isEmpty || !field.isEmpty - || !keyTerms.isEmpty - else { return nil } // `baseInstruction` is the pivot of the trained format. Contextual priming // sits *before* it; keyword boosting trails *after* it. The leading blocks, @@ -88,9 +91,9 @@ enum TranscriptionPrompt { // inline `Keywords: a, b, c.` form (Section 2.3) trailing the marker so the // model favors these exact spellings for names/jargon it would guess at. // The terms list is the one input with no upstream length cap, so include - // only as many whole terms as `characterCap` leaves room for — a huge - // Settings list must not push the prompt over the cap and fail every - // dictation with a 400. + // only as many whole terms as `characterCap` leaves room for, so a huge + // Settings list can't crowd out the instruction itself or balloon every + // request. var included: [String] = [] var remaining = characterCap - prompt.count - " Keywords: .".count for term in keyTerms { diff --git a/Tests/BlurtEngineTests/APIKeyValidatorTests.swift b/Tests/BlurtEngineTests/APIKeyValidatorTests.swift index 2edf512..5935cc9 100644 --- a/Tests/BlurtEngineTests/APIKeyValidatorTests.swift +++ b/Tests/BlurtEngineTests/APIKeyValidatorTests.swift @@ -34,12 +34,28 @@ extension HTTPClientTests { #expect(await makeValidator(transport).validate("any-key") == .unreachable) } - @Test("validator treats a 4xx client error (other than 408/429) as invalid") + @Test("validator treats a 400 malformed-request as invalid") func validateClientErrorIsInvalid() async { let transport = FakeHTTPTransport { _ in (400, json(["error": "bad request"])) } #expect(await makeValidator(transport).validate("malformed-key") == .invalid) } + @Test("a 4xx that says nothing about the key is unreachable, not invalid") + func validateEndpointErrorIsUnreachable() async { + // `.invalid` hard-blocks setup: `APIKeySubmission` refuses to persist, so the + // user can't finish onboarding. Only statuses that actually mean "credential + // rejected" or "malformed key" may report it. A 404/405/410 (endpoint retired + // or moved) or a proxy/captive-portal 403-page equivalent says nothing about + // the key, so it must fall through to the retry-later outcome — a blanket + // `400..<500 -> .invalid` told users their working key was rejected. + for status in [404, 405, 410, 451] { + let transport = FakeHTTPTransport { _ in (status, json(["error": "not here"])) } + #expect( + await makeValidator(transport).validate("good-key") == .unreachable, + "status \(status) must not reject the key") + } + } + @Test("validator treats 429 rate-limit as unreachable, not invalid") func validateRateLimitedIsUnreachable() async { let transport = FakeHTTPTransport { _ in (429, json(["error": "rate limited"])) } diff --git a/Tests/BlurtEngineTests/AssemblyAITranscriberTests.swift b/Tests/BlurtEngineTests/AssemblyAITranscriberTests.swift index df63efd..996f0b6 100644 --- a/Tests/BlurtEngineTests/AssemblyAITranscriberTests.swift +++ b/Tests/BlurtEngineTests/AssemblyAITranscriberTests.swift @@ -54,7 +54,7 @@ struct HTTPClientTests { // the sync model selector, and a boundary-tagged multipart body. Anything // else gets a 400 so a header regression fails loudly here. guard request.value(forHTTPHeaderField: "Authorization") == "test-key", - request.value(forHTTPHeaderField: "X-AAI-Model") == "u3-sync-pro", + request.value(forHTTPHeaderField: "X-AAI-Model") == "universal-3-5-pro", request.value(forHTTPHeaderField: "Content-Type")?.hasPrefix("multipart/form-data; boundary=") == true else { return (400, Data()) } return (200, json(["text": "ok"])) @@ -173,21 +173,6 @@ struct HTTPClientTests { } } - @Test("HTTP error message is read from the `error` field too, not just `message`") - func errorMessageFromErrorField() async throws { - let transport = FakeHTTPTransport { _ in (400, json(["error": "audio too short"])) } - - do { - _ = try await collectTranscript(makeTranscriber(apiKey: "k", transport: transport)) - Issue.record("expected a throw") - } catch let AssemblyAIError.http(status, message) { - #expect(status == 400) - #expect(message == "audio too short") - } catch { - Issue.record("expected AssemblyAIError.http, got \(error)") - } - } - @Test("HTTP error falls back to the raw body when the shape is unknown") func errorMessageFallsBackToRawBody() { let body = Data(#"{"unexpected":"shape"}"#.utf8) @@ -199,14 +184,19 @@ struct HTTPClientTests { #expect(AssemblyAITranscriber.errorMessage(from: json(["detail": "audio required"])) == "audio required") } - @Test("HTTP error message field precedence is error > message > detail") + @Test("HTTP error message field precedence is message > detail") func errorMessageFieldPrecedence() { - // The API labels its explanation inconsistently; when several fields - // co-exist the documented priority must hold, so a reorder can't silently - // change which message reaches the user. - #expect( - AssemblyAITranscriber.errorMessage(from: json(["error": "a", "message": "b", "detail": "c"])) == "a") + // The two documented shapes: `{error_code, message}` for request/audio/server + // errors and `{detail}` for auth and rate limiting. They shouldn't co-occur, + // but pin the order so a reorder can't silently change which reaches the user. #expect(AssemblyAITranscriber.errorMessage(from: json(["message": "b", "detail": "c"])) == "b") + #expect(AssemblyAITranscriber.errorMessage(from: json(["detail": "c"])) == "c") + // An `error` key is in none of the documented responses, so it is no longer + // consulted — such a body falls through to the raw-body arm rather than + // yielding the value. (Asserting the behavior, not the serialized bytes.) + let errorShaped = AssemblyAITranscriber.errorMessage(from: json(["error": "a"])) + #expect(errorShaped != "a") + #expect(errorShaped?.contains("error") == true) } @Test("a non-string `detail` (validation array) falls back to the raw body") diff --git a/Tests/BlurtEngineTests/DictationKeyRouterTests.swift b/Tests/BlurtEngineTests/DictationKeyRouterTests.swift index e744dcf..7c92ae2 100644 --- a/Tests/BlurtEngineTests/DictationKeyRouterTests.swift +++ b/Tests/BlurtEngineTests/DictationKeyRouterTests.swift @@ -108,6 +108,43 @@ struct DictationKeyRouterTests { #expect(discarded) } + @Test("a latch left behind by a keyless dictation end doesn't swallow the next press") + func resetClearsLatchSoNextPressStarts() { + // The bug this pins: when a dictation ends WITHOUT a key event — the + // auto-release cap fires, or the press is refused/failed — the gate stays + // `.latched`. A latched `modifierDown` returns `.none` and the `modifierUp` + // after it returns `.stop`, which no-ops on an already-terminal session, so + // the user's whole next press does nothing. `DictationKeyTap`'s + // `syncAfterTerminalPhase()` calls `reset()` to clear it; this pins that a + // reset genuinely restores the next press. + var router = DictationKeyRouter(triggerKeyCode: trigger) + #expect(router.handle(downEvent(trigger), at: .zero) == .start) + #expect(router.handle(upEvent(trigger), at: .milliseconds(200)) == .none) // latched + + // Without the reset, this next tap is swallowed — the exact dead press. + var swallowed = router + #expect(swallowed.handle(downEvent(trigger), at: .seconds(5)) == .none) + + router.reset() + #expect(router.handle(downEvent(trigger), at: .seconds(5)) == .start) + } + + @Test("reset after a keyless end ignores a stale key-up, then starts cleanly") + func resetWhileHeldThenReleaseIsInert() { + // The auto-release/failed-press case where the trigger is still physically + // held when the phase goes terminal. `syncAfterTerminalPhase` resets anyway + // (the dictation is over), which clears the modifier tracker — so the release + // that follows must route to `.none` rather than emitting a spurious `.stop`, + // and the press after that must start normally. + var router = DictationKeyRouter(triggerKeyCode: trigger) + #expect(router.handle(downEvent(trigger), at: .zero) == .start) + + router.reset() // terminal phase arrived while the key is still down + + #expect(router.handle(upEvent(trigger), at: .milliseconds(300)) == .none) + #expect(router.handle(downEvent(trigger), at: .seconds(2)) == .start) + } + @Test("rebind mid-recording discards it and switches keycodes") func rebindMidRecording() { var router = DictationKeyRouter(triggerKeyCode: trigger) diff --git a/Tests/BlurtEngineTests/DictationPerformanceTests.swift b/Tests/BlurtEngineTests/DictationPerformanceTests.swift index 3704341..8508b87 100644 --- a/Tests/BlurtEngineTests/DictationPerformanceTests.swift +++ b/Tests/BlurtEngineTests/DictationPerformanceTests.swift @@ -27,17 +27,10 @@ final class DictationPerformanceTests: XCTestCase { return samples.sorted()[samples.count / 2] } - private func makeSession() -> DictationSession { - DictationSession( - mic: StubMicCapture(), - transcriber: StubTranscriber(mode: .transcript("hello world")), - injector: StubInjector()) - } - /// The release → transcribe → inject hot path (STT round trip + paste), stubbed. func testTranscribeInjectWithinBudget() async { let median = await medianDuration(iterations: 7) { - let session = makeSession() + let session = makeSession().session await session.press() await session.release() await session.waitForIdle() @@ -51,7 +44,7 @@ final class DictationPerformanceTests: XCTestCase { /// The press → `.recording` startup path (focus capture + mic start), stubbed. func testPressStartupWithinBudget() async { let median = await medianDuration(iterations: 7) { - let session = makeSession() + let session = makeSession().session await session.press() await session.cancel() // tear down so the mic/recording doesn't linger } diff --git a/Tests/BlurtEngineTests/DictationSessionGuardTests.swift b/Tests/BlurtEngineTests/DictationSessionGuardTests.swift index 0c52619..8444787 100644 --- a/Tests/BlurtEngineTests/DictationSessionGuardTests.swift +++ b/Tests/BlurtEngineTests/DictationSessionGuardTests.swift @@ -11,42 +11,33 @@ struct DictationSessionGuardTests { @Test("press while already recording is a silent no-op") func pressWhileRecordingDropped() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("hi")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("hi")) - await session.press() - await session.press() // .recording is non-terminal, so the guard drops this + await fixture.session.press() + await fixture.session.press() // .recording is non-terminal, so the guard drops this - #expect(await mic.startCalls == 1) - #expect(await session.phase == .recording) + #expect(await fixture.mic.startCalls == 1) + #expect(await fixture.session.phase == .recording) } @Test("release with nothing recording is a silent no-op") func releaseFromIdleNoOps() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("hi")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("hi")) - await session.release() + await fixture.session.release() - #expect(await session.phase == .idle) - #expect(await mic.stopCalls == 0) + #expect(await fixture.session.phase == .idle) + #expect(await fixture.mic.stopCalls == 0) } @Test("cancel with nothing recording is a silent no-op") func cancelFromIdleNoOps() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("hi")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("hi")) - await session.cancel() + await fixture.session.cancel() - #expect(await session.phase == .idle) - #expect(await mic.stopCalls == 0) + #expect(await fixture.session.phase == .idle) + #expect(await fixture.mic.stopCalls == 0) } @Test("keyTermsProvider is consulted afresh at each press") @@ -75,15 +66,12 @@ struct DictationSessionGuardTests { @Test("multiple phaseStream subscribers all receive later transitions") func phaseStreamBroadcastsToMultipleSubscribers() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("hi")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("hi")) // Subscribe while recording so each stream's initial yield is non-terminal. - await session.press() - let firstStream = await session.phaseStream() - let secondStream = await session.phaseStream() + await fixture.session.press() + let firstStream = await fixture.session.phaseStream() + let secondStream = await fixture.session.phaseStream() func firstTerminal(_ stream: AsyncStream) async -> PipelinePhase? { for await phase in stream where phase.isTerminal { return phase } @@ -91,7 +79,7 @@ struct DictationSessionGuardTests { } async let firstTerminalPhase = firstTerminal(firstStream) async let secondTerminalPhase = firstTerminal(secondStream) - await session.release() + await fixture.session.release() #expect(await firstTerminalPhase == .pasted) #expect(await secondTerminalPhase == .pasted) } diff --git a/Tests/BlurtEngineTests/DictationSessionSubmitTests.swift b/Tests/BlurtEngineTests/DictationSessionSubmitTests.swift index 8ffac16..01214e2 100644 --- a/Tests/BlurtEngineTests/DictationSessionSubmitTests.swift +++ b/Tests/BlurtEngineTests/DictationSessionSubmitTests.swift @@ -12,14 +12,11 @@ struct DictationSessionSubmitTests { @Test("submit runs press → release in emit order through the full pipeline") func submitHappyPath() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("Hello world.")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession() - let stream = await session.phaseStream() - session.submit(.press) - session.submit(.release) + let stream = await fixture.session.phaseStream() + fixture.session.submit(.press) + fixture.session.submit(.release) var seen: [PipelinePhase] = [] for await phase in stream { @@ -31,44 +28,38 @@ struct DictationSessionSubmitTests { #expect(seen.contains(.recording)) #expect(seen.last == .pasted) - #expect(await injector.inserted == ["Hello world."]) + #expect(await fixture.injector.inserted == ["Hello world."]) } @Test("submit(.cancel) after submit(.press) discards the capture in order") func submitCancelHonoredInOrder() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("never")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("never")) - let stream = await session.phaseStream() + let stream = await fixture.session.phaseStream() // The exact shape of the race `submit` exists to prevent: were each of // these a separately spawned Task, the cancel could overtake the press, // no-op on a still-idle session, and strand the recording. - session.submit(.press) - session.submit(.cancel) + fixture.session.submit(.press) + fixture.session.submit(.cancel) for await phase in stream where phase == .cancelled { break } - #expect(await session.phase == .cancelled) - #expect(await mic.stopCalls == 1) - #expect(await injector.inserted.isEmpty) + #expect(await fixture.session.phase == .cancelled) + #expect(await fixture.mic.stopCalls == 1) + #expect(await fixture.injector.inserted.isEmpty) } @Test("submit(.cancelRecording) tears down a live recording") func submitCancelRecording() async throws { - let mic = StubMicCapture() - let stt = StubTranscriber(mode: .transcript("never")) - let injector = StubInjector() - let session = DictationSession(mic: mic, transcriber: stt, injector: injector) + let fixture = makeSession(mode: .transcript("never")) - let stream = await session.phaseStream() - session.submit(.press) - session.submit(.cancelRecording) + let stream = await fixture.session.phaseStream() + fixture.session.submit(.press) + fixture.session.submit(.cancelRecording) for await phase in stream where phase == .cancelled { break } - #expect(await session.phase == .cancelled) - #expect(await injector.inserted.isEmpty) + #expect(await fixture.session.phase == .cancelled) + #expect(await fixture.injector.inserted.isEmpty) } } diff --git a/Tests/BlurtEngineTests/FocusCaptureTests.swift b/Tests/BlurtEngineTests/FocusCaptureTests.swift index 3645bb8..fe4aba2 100644 --- a/Tests/BlurtEngineTests/FocusCaptureTests.swift +++ b/Tests/BlurtEngineTests/FocusCaptureTests.swift @@ -72,6 +72,25 @@ struct FocusCaptureTests { #expect(FocusCapture.priorSlice(full: "hello", caret: 0, maxChars: 320) == nil) } + @Test("priorSlice is given the raw value, so surrounding whitespace keeps caret offsets valid") + func priorSliceUntrimmedValue() { + // These are the two cases that broke while the caller trimmed the value before + // slicing it with an untrimmed caret offset (see `rawStringValue`). + // + // Trailing whitespace: "Hello " with the caret at 6 must yield "Hello " — the + // trailing space is exactly what `withLeadingSeparator` reads to decide it must + // NOT add another one. Trimmed to "Hello" (5 units), 6 > 5 fell through to the + // whole-value tail "Hello", so the separator was added and the field got + // "Hello world". + #expect(FocusCapture.priorSlice(full: "Hello ", caret: 6, maxChars: 320) == "Hello ") + // Leading whitespace (an indented editor line): the caret at 8 in + // " Hello world" is just past " Hello ". Trimmed to "Hello world" the slice + // *succeeded* and returned "Hello wo" — text from AFTER the caret. + #expect(FocusCapture.priorSlice(full: " Hello world", caret: 8, maxChars: 320) == " Hello ") + // A value that is only whitespace still has a real prefix before the caret. + #expect(FocusCapture.priorSlice(full: " ", caret: 2, maxChars: 320) == " ") + } + @Test("priorSlice treats the caret as a UTF-16 offset, not a Character count") func priorCaretIsUTF16() { // AX selected-text ranges are UTF-16: each emoji below is 2 UTF-16 units but @@ -130,16 +149,28 @@ struct FocusCaptureTests { #expect(FocusCapture.visibleTextOrNil("hello ") == "hello ") } - // MARK: isSecureFieldRole + // MARK: isSecureField - @Test("only the password-field role triggers the prompt redaction") + @Test("the password-field role triggers the prompt redaction") func secureFieldRoleDetection() { // The guard that keeps a typed password out of the STT prompt keys on this // exact role string — a rename or typo here would silently stop redacting. - #expect(FocusCapture.isSecureFieldRole("AXSecureTextField")) - #expect(!FocusCapture.isSecureFieldRole("AXTextField")) - #expect(!FocusCapture.isSecureFieldRole("AXTextArea")) - #expect(!FocusCapture.isSecureFieldRole(nil)) + #expect(FocusCapture.isSecureField(role: "AXSecureTextField", subrole: nil)) + #expect(!FocusCapture.isSecureField(role: "AXTextField", subrole: nil)) + #expect(!FocusCapture.isSecureField(role: "AXTextArea", subrole: nil)) + #expect(!FocusCapture.isSecureField(role: nil, subrole: nil)) + } + + @Test("a secure *subrole* under a generic text role also redacts") + func secureFieldSubroleDetection() { + // AppKit exposes secure-ness as either a role or a subrole, and an element can + // report a plain `AXTextField` role while only its subrole says "password" — + // browser password inputs and custom secure fields do this. Keying on role + // alone read those as ordinary text and sent their contents to the STT prompt. + #expect(FocusCapture.isSecureField(role: "AXTextField", subrole: "AXSecureTextField")) + #expect(FocusCapture.isSecureField(role: nil, subrole: "AXSecureTextField")) + // A non-secure subrole must not redact an ordinary field. + #expect(!FocusCapture.isSecureField(role: "AXTextField", subrole: "AXSearchField")) } // MARK: isElectronApp diff --git a/Tests/BlurtEngineTests/HotkeyRaceTests.swift b/Tests/BlurtEngineTests/HotkeyRaceTests.swift index 4541170..876f599 100644 --- a/Tests/BlurtEngineTests/HotkeyRaceTests.swift +++ b/Tests/BlurtEngineTests/HotkeyRaceTests.swift @@ -136,8 +136,7 @@ private actor GatedMicCapture: MicCaptureProtocol { func stop() async throws -> Data { stopCalls += 1 - // Above SyncSTTLimits.minPCMBytes so the transcript isn't dropped by the - // too-short guard — this suite exercises the press/release race, not it. - return Data(count: SyncSTTLimits.minPCMBytes * 2) + // This suite exercises the press/release race, not the too-short guard. + return StubPCM.aboveMinimum } } diff --git a/Tests/BlurtEngineTests/KeyTermsStoreTests.swift b/Tests/BlurtEngineTests/KeyTermsStoreTests.swift index 3850de8..38af255 100644 --- a/Tests/BlurtEngineTests/KeyTermsStoreTests.swift +++ b/Tests/BlurtEngineTests/KeyTermsStoreTests.swift @@ -33,12 +33,17 @@ struct KeyTermsStoreTests { } } -/// `get`/`set` round-trip through `UserDefaults.standard`, so this suite is -/// serialized and saves/restores the real key around each case — it must not -/// leave the dev machine's stored terms changed. -@Suite("KeyTermsStore.get/set", .serialized) -struct KeyTermsStoreGetSetTests { - private func withCleanStore(_ body: () -> Void) { +/// `get`/`terms` read `UserDefaults.standard`, so this suite is serialized and +/// saves/restores the real key around each case — it must not leave the dev +/// machine's stored terms changed. +/// +/// Each case writes the defaults slot directly, which is exactly how production +/// writes it: the store has no setter, and the Settings field's `@AppStorage` +/// binding is the only writer. So these pin the contract that matters — whatever +/// raw text the field happens to hold, the read side normalizes it. +@Suite("KeyTermsStore.get", .serialized) +struct KeyTermsStoreGetTests { + private func withCleanStore(_ stored: String?, _ body: () -> Void) { let key = KeyTermsStore.defaultsKey let original = UserDefaults.standard.string(forKey: key) defer { @@ -48,33 +53,42 @@ struct KeyTermsStoreGetSetTests { UserDefaults.standard.removeObject(forKey: key) } } + if let stored { + UserDefaults.standard.set(stored, forKey: key) + } else { + UserDefaults.standard.removeObject(forKey: key) + } body() } - @Test("set stores the trimmed string; get and terms round-trip it") - func setAndGet() { - withCleanStore { - KeyTermsStore.set(" AssemblyAI, Slack ") + @Test("get trims the stored string; terms parses it") + func getAndTerms() { + withCleanStore(" AssemblyAI, Slack ") { #expect(KeyTermsStore.get() == "AssemblyAI, Slack") #expect(KeyTermsStore.terms() == ["AssemblyAI", "Slack"]) } } - @Test("set(nil) clears the stored value") - func setNilClears() { - withCleanStore { - KeyTermsStore.set("Kubernetes") - KeyTermsStore.set(nil) + @Test("an unset key reads as no terms") + func unsetReadsAsNil() { + withCleanStore(nil) { #expect(KeyTermsStore.get() == nil) + #expect(KeyTermsStore.terms().isEmpty) } } - @Test("set with a blank string clears the stored value") - func setBlankClears() { - withCleanStore { - KeyTermsStore.set("Kubernetes") - KeyTermsStore.set(" \n") + @Test("a blank field reads as no terms rather than an empty term") + func blankReadsAsNil() { + // The field is cleared by emptying it, not by deleting the key, so the slot + // genuinely holds "" (or whitespace mid-edit). That must read as "no terms", + // otherwise the prompt would carry an empty vocabulary clause. + withCleanStore(" \n") { + #expect(KeyTermsStore.get() == nil) + #expect(KeyTermsStore.terms().isEmpty) + } + withCleanStore("") { #expect(KeyTermsStore.get() == nil) + #expect(KeyTermsStore.terms().isEmpty) } } } diff --git a/Tests/BlurtEngineTests/KeychainStoreTests.swift b/Tests/BlurtEngineTests/KeychainStoreTests.swift index 723bbc7..975dbbe 100644 --- a/Tests/BlurtEngineTests/KeychainStoreTests.swift +++ b/Tests/BlurtEngineTests/KeychainStoreTests.swift @@ -18,15 +18,32 @@ struct KeychainStoreTests { func getEmpty() { let store = makeStore() defer { store.set(nil) } - #expect(store.get() == nil) + #expect(store.read() == .absent) } - @Test("set then get round-trips the value") + @Test("set then read round-trips the value") func setThenGet() { let store = makeStore() defer { store.set(nil) } #expect(store.set("sk-abc123")) - #expect(store.get() == "sk-abc123") + #expect(store.read() == .value("sk-abc123")) + } + + @Test("read distinguishes an absent item from a stored value") + func readReportsAbsentVsValue() { + // Why this distinction exists: `APIKeyStore` memoizes the read, and collapsing + // "nothing saved" and "couldn't read it" to a bare nil let a transient failure + // (locked keychain, denied ACL prompt) get cached as a permanent "no API key" + // for the whole process. `.unavailable` can't be provoked here — it needs a + // locked keychain or a denied prompt — so this pins the two reachable arms. + let store = makeStore() + defer { store.set(nil) } + + #expect(store.read() == .absent) + #expect(store.set("sk-abc123")) + #expect(store.read() == .value("sk-abc123")) + #expect(store.set(nil)) + #expect(store.read() == .absent) } @Test("set overwrites an existing value (update path)") @@ -35,7 +52,7 @@ struct KeychainStoreTests { defer { store.set(nil) } #expect(store.set("first")) #expect(store.set("second")) - #expect(store.get() == "second") + #expect(store.read() == .value("second")) } @Test("set trims surrounding whitespace") @@ -43,7 +60,7 @@ struct KeychainStoreTests { let store = makeStore() defer { store.set(nil) } #expect(store.set(" sk-trim \n")) - #expect(store.get() == "sk-trim") + #expect(store.read() == .value("sk-trim")) } @Test("set(nil) deletes the stored value") @@ -52,7 +69,7 @@ struct KeychainStoreTests { defer { store.set(nil) } #expect(store.set("to-be-deleted")) #expect(store.set(nil)) - #expect(store.get() == nil) + #expect(store.read() == .absent) } @Test("set(whitespace) deletes, get returns nil") @@ -61,6 +78,6 @@ struct KeychainStoreTests { defer { store.set(nil) } #expect(store.set("present")) #expect(store.set(" ")) - #expect(store.get() == nil) + #expect(store.read() == .absent) } } diff --git a/Tests/BlurtEngineTests/MenuBarStatusTests.swift b/Tests/BlurtEngineTests/MenuBarStatusTests.swift index 48a9d7b..c66aa17 100644 --- a/Tests/BlurtEngineTests/MenuBarStatusTests.swift +++ b/Tests/BlurtEngineTests/MenuBarStatusTests.swift @@ -7,31 +7,28 @@ import Testing /// shell that renders it has no test target). @Suite("PipelinePhase → MenuBarStatus") struct MenuBarStatusTests { - @Test func recordingMapsToRecording() { - #expect(PipelinePhase.recording.menuBarStatus == .recording) - } - - @Test func transcribingMapsToTranscribing() { - #expect(PipelinePhase.transcribing.menuBarStatus == .transcribing) - } - - @Test func idleMapsToIdle() { - #expect(PipelinePhase.idle.menuBarStatus == .idle) - } - - @Test func injectingMapsToIdle() { + /// One row per `PipelinePhase` case. A table rather than a `@Test` apiece (the + /// precedent `DictationKeyGateTests` sets) so that adding a phase makes the + /// missing row visible here instead of leaving the mapping silently unpinned — + /// which is how `.noTarget` went uncovered. `.failed` keeps its own test below, + /// since its mapping encodes a deliberate policy rather than a coarser icon. + static let projections: [(phase: PipelinePhase, expected: MenuBarStatus)] = [ + (.recording, .recording), + (.transcribing, .transcribing), + (.idle, .idle), // Injection happens silently; the indicator rests at idle through the brief // paste rather than showing a distinct state. - #expect(PipelinePhase.injecting.menuBarStatus == .idle) - } - - @Test func cancelledMapsToIdle() { - #expect(PipelinePhase.cancelled.menuBarStatus == .idle) - } - - @Test func pastedMapsToIdle() { + (.injecting, .idle), + (.cancelled, .idle), // The completed-paste notice lives on the pill; the menu bar stays idle. - #expect(PipelinePhase.pasted.menuBarStatus == .idle) + (.pasted, .idle), + // Likewise the "copied to clipboard" notice — pill only. + (.noTarget, .idle), + ] + + @Test("each phase projects to its menu bar status", arguments: projections) + func phaseProjectsToMenuBarStatus(phase: PipelinePhase, expected: MenuBarStatus) { + #expect(phase.menuBarStatus == expected) } @Test func failedMapsToIdle() { diff --git a/Tests/BlurtEngineTests/OverlayOriginStoreTests.swift b/Tests/BlurtEngineTests/OverlayOriginStoreTests.swift new file mode 100644 index 0000000..df66ce7 --- /dev/null +++ b/Tests/BlurtEngineTests/OverlayOriginStoreTests.swift @@ -0,0 +1,47 @@ +import Foundation +import Testing + +@testable import BlurtEngine + +/// The pill's dragged-origin persistence. Engine-side (rather than private to the +/// AppKit controller) so it lands in `PersistedSettings.allDefaultsKeys` and so +/// these rules are covered by `swift test` at all. +@Suite("OverlayOriginStore") +struct OverlayOriginStoreTests { + @Test("unset reads as nil, so the default placement is used") + func unsetIsNil() { + let store = OverlayOriginStore(defaults: freshDefaults()) + #expect(store.origin == nil) + } + + @Test("round-trips a dragged origin") + func roundTrips() { + let store = OverlayOriginStore(defaults: freshDefaults()) + store.origin = CGPoint(x: 120.5, y: 340.25) + #expect(store.origin == CGPoint(x: 120.5, y: 340.25)) + } + + @Test("a half-written pair reads as nil rather than an implied zero") + func halfWrittenIsNil() { + // `double(forKey:)` reports 0 for a missing key, so keying off the values + // alone would place the pill at an origin the user never chose. Both + // components must be present. + let defaults = freshDefaults() + defaults.set(120.0, forKey: OverlayOriginStore.xDefaultsKey) + #expect(OverlayOriginStore(defaults: defaults).origin == nil) + } + + @Test("nil clears both keys") + func nilClears() { + let defaults = freshDefaults() + let store = OverlayOriginStore(defaults: defaults) + store.origin = CGPoint(x: 10, y: 20) + store.origin = nil + #expect(store.origin == nil) + #expect(defaults.object(forKey: OverlayOriginStore.xDefaultsKey) == nil) + #expect(defaults.object(forKey: OverlayOriginStore.yDefaultsKey) == nil) + } + + // Roster membership for both keys is asserted in `PersistedSettingsTests`, + // alongside the count that pins the roster as a whole. +} diff --git a/Tests/BlurtEngineTests/OverlayPlacementTests.swift b/Tests/BlurtEngineTests/OverlayPlacementTests.swift index 72ed941..741a9e1 100644 --- a/Tests/BlurtEngineTests/OverlayPlacementTests.swift +++ b/Tests/BlurtEngineTests/OverlayPlacementTests.swift @@ -12,6 +12,30 @@ struct OverlayPlacementTests { private let screen = CGRect(x: 0, y: 0, width: 1000, height: 600) private let panel = CGSize(width: 200, height: 60) + @Test("panelOrigin measures the clearance to the pill, not the panel") + func panelOriginBacksOffTheShadowMargin() { + // This arithmetic used to live at the AppKit call site as + // `bottomOffset: 80 - Self.shadowMargin`, so changing the shadow margin moved + // the pill and no test noticed. The clearance is to the *visible pill*; the + // panel is larger by `shadowMargin` on every side, so its origin sits that + // much lower. + let margin: CGFloat = 28 + let origin = OverlayPlacement.panelOrigin( + panelSize: panel, visibleFrame: screen, customOrigin: nil, shadowMargin: margin) + #expect(origin.y == OverlayPlacement.defaultBottomClearance - margin) + // The pill's own bottom edge still lands exactly at the clearance. + #expect(origin.y + margin == OverlayPlacement.defaultBottomClearance) + #expect(origin.x == 400) + } + + @Test("panelOrigin still clamps a stale dragged origin back on screen") + func panelOriginClampsCustom() { + let far = CGPoint(x: 5000, y: 5000) + let origin = OverlayPlacement.panelOrigin( + panelSize: panel, visibleFrame: screen, customOrigin: far, shadowMargin: 28) + #expect(origin == CGPoint(x: 800, y: 540)) + } + @Test("no custom origin: horizontally centered, bottomOffset above the bottom") func defaultPlacement() { let origin = OverlayPlacement.origin( diff --git a/Tests/BlurtEngineTests/OverlayUIStateTests.swift b/Tests/BlurtEngineTests/OverlayUIStateTests.swift index 6b5e248..f3ffb5b 100644 --- a/Tests/BlurtEngineTests/OverlayUIStateTests.swift +++ b/Tests/BlurtEngineTests/OverlayUIStateTests.swift @@ -7,28 +7,40 @@ import Testing /// shell that renders it has no test target). @Suite("PipelinePhase → OverlayUIState") struct OverlayUIStateTests { - @Test func idleMapsToIdle() { - #expect(PipelinePhase.idle.overlayState == .idle) - } - - @Test func recordingMapsToRecording() { - #expect(PipelinePhase.recording.overlayState == .recording) - } - - @Test func transcribingMapsToProcessing() { - #expect(PipelinePhase.transcribing.overlayState == .processing) - } + /// The phase→pill projection, one row per `PipelinePhase` case with a fixed + /// mapping. A table rather than a `@Test` apiece (the precedent + /// `DictationKeyGateTests` sets): adding a phase case makes the missing row + /// visible in one place, and a failure names the phase. The cases whose + /// mapping carries a rationale — `.failed` and its `.apiKeyMissing` + /// carve-out — stay as their own tests below. + static let projections: [(phase: PipelinePhase, expected: OverlayUIState)] = [ + (.idle, .idle), + (.recording, .recording), + (.transcribing, .processing), + // `.injecting` is a *working* phase, so it must not project to `.idle`: the + // shell reads an idle projection as "dismiss the pill" and would start a + // fade-out mid-dictation, blinking the pill out and back in before "Pasted". + // Keeping it `.processing` holds the pill up continuously from transcribe + // through paste; the terminal `.pasted` phase carries the notice. + (.injecting, .processing), + // A completed paste surfaces the quiet "Pasted" notice — the mirror of + // `.noTarget`'s "Copied" — as a transient notice before settling to idle. + (.pasted, .pasted), + // A cancelled capture leaves no trace on the pill — same rest state as idle. + (.cancelled, .idle), + // Transcription succeeded but nothing editable was focused, so the pill + // shows the neutral "copied to clipboard" notice rather than an error. + (.noTarget, .noTarget), + ] - @Test func injectingMapsToIdle() { - // The in-flight injecting phase shows no distinct pill; the terminal - // `.pasted` phase (set once the paste lands) carries the "Pasted" notice. - #expect(PipelinePhase.injecting.overlayState == .idle) + @Test("each phase projects to its pill state", arguments: projections) + func phaseProjectsToOverlayState(phase: PipelinePhase, expected: OverlayUIState) { + #expect(phase.overlayState == expected) } - @Test func pastedMapsToPasted() { - // A completed paste surfaces the quiet "Pasted" notice — the mirror of - // `.noTarget`'s "Copied" — as a transient notice before settling to idle. - #expect(PipelinePhase.pasted.overlayState == .pasted) + @Test func pastedIsATransientNotice() { + // Pinned alongside the mapping: the "Pasted" pill must auto-clear rather + // than stick, which is what makes it a notice and not a steady state. #expect(OverlayUIState.pasted.noticeDwellSeconds != nil) } @@ -47,6 +59,25 @@ struct OverlayUIStateTests { #expect(PipelinePhase.failed(.apiKeyMissing).overlayState == .idle) } + @Test("setupBlocker names the failures that are unfinished setup, not faults") + func setupBlockerClassification() { + // The single classification both consumers derive from: this projection renders + // a setup blocker as calm `.idle`, and the shell routes it to the settings + // window. When each pattern-matched `.failed(.apiKeyMissing)` for itself, + // adding a second blocker meant remembering both sites — miss the engine one + // and the user gets a red flash; miss the shell one and the press silently does + // nothing. + #expect(PipelinePhase.failed(.apiKeyMissing).setupBlocker == .apiKeyMissing) + // A genuine failure is not a setup state. + #expect(PipelinePhase.failed(.targetAppLost).setupBlocker == nil) + // Neither is any non-failed phase. + for phase in [PipelinePhase.idle, .recording, .transcribing, .injecting, .pasted, .noTarget] { + #expect(phase.setupBlocker == nil) + } + // And the pill projection agrees with the classification. + #expect(PipelinePhase.failed(.apiKeyMissing).overlayState == .idle) + } + @Test func failedFallsBackWhenNoDescription() { // Defensive: every BlurtError supplies an errorDescription today, but the // mapping must still produce a non-empty message if one ever returns nil. @@ -56,17 +87,6 @@ struct OverlayUIStateTests { Issue.record("expected .error") } } - - @Test func cancelledMapsToIdle() { - // A cancelled capture leaves no trace on the pill — same rest state as idle. - #expect(PipelinePhase.cancelled.overlayState == .idle) - } - - @Test func noTargetMapsToNoTarget() { - // Transcription succeeded but nothing editable was focused, so the pill - // shows the neutral "copied to clipboard" notice rather than an error. - #expect(PipelinePhase.noTarget.overlayState == .noTarget) - } } /// The pill's VoiceOver label is spoken to the user, so lock the exact wording @@ -74,16 +94,19 @@ struct OverlayUIStateTests { /// silent edit would otherwise ship an unannounced regression. @Suite("OverlayUIState.accessibilityLabel") struct OverlayUIStateAccessibilityLabelTests { - @Test func idleLabel() { - #expect(OverlayUIState.idle.accessibilityLabel == "Blurt.") - } - - @Test func recordingLabel() { - #expect(OverlayUIState.recording.accessibilityLabel == "Recording.") - } + /// One row per fixed-wording state. `.error` is excluded: its label is a rule + /// (echo the carried message verbatim), not a constant, so it keeps its own test. + static let labels: [(state: OverlayUIState, spoken: String)] = [ + (.idle, "Blurt."), + (.recording, "Recording."), + (.processing, "Processing."), + (.pasted, "Your dictation was pasted."), + (.noTarget, "No text field focused. Your dictation was copied to the clipboard."), + ] - @Test func processingLabel() { - #expect(OverlayUIState.processing.accessibilityLabel == "Processing.") + @Test("each state speaks its fixed label", arguments: labels) + func stateSpeaksItsLabel(state: OverlayUIState, spoken: String) { + #expect(state.accessibilityLabel == spoken) } @Test func errorLabelIsTheMessageVerbatim() { @@ -91,17 +114,6 @@ struct OverlayUIStateAccessibilityLabelTests { // so it must echo the message it carries with no wrapping. #expect(OverlayUIState.error(message: "AssemblyAI error 401.").accessibilityLabel == "AssemblyAI error 401.") } - - @Test func pastedLabel() { - #expect(OverlayUIState.pasted.accessibilityLabel == "Your dictation was pasted.") - } - - @Test func noTargetLabel() { - #expect( - OverlayUIState.noTarget.accessibilityLabel - == "No text field focused. Your dictation was copied to the clipboard." - ) - } } /// `noticeDwellSeconds` decides whether the shell holds a state or flashes it diff --git a/Tests/BlurtEngineTests/PersistedSettingsTests.swift b/Tests/BlurtEngineTests/PersistedSettingsTests.swift index 53ff218..c348db6 100644 --- a/Tests/BlurtEngineTests/PersistedSettingsTests.swift +++ b/Tests/BlurtEngineTests/PersistedSettingsTests.swift @@ -13,13 +13,20 @@ struct PersistedSettingsTests { #expect(PersistedSettings.allDefaultsKeys.contains(SoundPackStore.defaultsKey)) #expect(PersistedSettings.allDefaultsKeys.contains(KeyTermsStore.defaultsKey)) #expect(PersistedSettings.allDefaultsKeys.contains(DeveloperModeStore.defaultsKey)) + // OverlayOriginStore persists a point, so it contributes two keys rather + // than one. Both belong to the sweep: while they were private to + // `OverlayWindowController`, no reset knew about them and a pill dragged + // during a UI-test run survived into later runs. + #expect(PersistedSettings.allDefaultsKeys.contains(OverlayOriginStore.xDefaultsKey)) + #expect(PersistedSettings.allDefaultsKeys.contains(OverlayOriginStore.yDefaultsKey)) } @Test("the roster carries no stale or duplicate keys") func rosterHasNoStrays() { - // Exactly the four known stores: a removed store must leave the roster in - // the same change, and a key listed twice would hint at a copy-paste slip. - #expect(PersistedSettings.allDefaultsKeys.count == 4) + // Exactly the five known stores' keys (OverlayOriginStore contributes two): + // a removed store must leave the roster in the same change, and a key listed + // twice would hint at a copy-paste slip. + #expect(PersistedSettings.allDefaultsKeys.count == 6) #expect(Set(PersistedSettings.allDefaultsKeys).count == PersistedSettings.allDefaultsKeys.count) } } diff --git a/Tests/BlurtEngineTests/SoundPackStoreTests.swift b/Tests/BlurtEngineTests/SoundPackStoreTests.swift index 8ad4232..422971f 100644 --- a/Tests/BlurtEngineTests/SoundPackStoreTests.swift +++ b/Tests/BlurtEngineTests/SoundPackStoreTests.swift @@ -31,7 +31,7 @@ struct SoundPackStoreTests { @Test("an unknown stored value falls back to the default") func unknownFallsBack() { let defaults = freshDefaults() - defaults.set("trombone", forKey: "BlurtSoundPack") + defaults.set("trombone", forKey: SoundPackStore.defaultsKey) #expect(SoundPackStore(defaults: defaults).soundPack == .defaultPack) } } diff --git a/Tests/BlurtEngineTests/SoundPackTests.swift b/Tests/BlurtEngineTests/SoundPackTests.swift index 3707783..3a4fcd4 100644 --- a/Tests/BlurtEngineTests/SoundPackTests.swift +++ b/Tests/BlurtEngineTests/SoundPackTests.swift @@ -32,13 +32,6 @@ struct SoundPackTests { #expect(SoundPack.find(id: "juno-0")?.label == "Brass") } - @Test("synth credit names the source synth; nil for none") - func synth() { - #expect(SoundPack.none.synth == nil) - #expect(SoundPack.find(id: "rom1a-6")?.synth == "Yamaha DX-7") - #expect(SoundPack.find(id: "juno-13")?.synth == "Roland Juno-106") - } - @Test("default pack is Orchestra; lookups round-trip and reject unknowns") func lookup() { #expect(SoundPack.defaultPack.id == "rom1a-6") diff --git a/Tests/BlurtEngineTests/Stubs/GatedStopMic.swift b/Tests/BlurtEngineTests/Stubs/GatedStopMic.swift index dcc648e..7396c91 100644 --- a/Tests/BlurtEngineTests/Stubs/GatedStopMic.swift +++ b/Tests/BlurtEngineTests/Stubs/GatedStopMic.swift @@ -24,9 +24,8 @@ actor GatedStopMic: MicCaptureProtocol { stopCalls += 1 await gate.enter() if let stopError { throw stopError } - // Above SyncSTTLimits.minPCMBytes so the transcript isn't dropped by the - // too-short guard — these suites exercise stop races, not it. - return Data(count: SyncSTTLimits.minPCMBytes * 2) + // These suites exercise stop races, not the too-short-audio guard. + return StubPCM.aboveMinimum } func waitUntilStopEntered() async { await gate.waitUntilEntered() } diff --git a/Tests/BlurtEngineTests/Stubs/InjectorTestSupport.swift b/Tests/BlurtEngineTests/Stubs/InjectorTestSupport.swift index 952daea..8cbed64 100644 --- a/Tests/BlurtEngineTests/Stubs/InjectorTestSupport.swift +++ b/Tests/BlurtEngineTests/Stubs/InjectorTestSupport.swift @@ -33,6 +33,14 @@ func makeRecordingInjector() -> (injector: KeyInjector, pasted: StringListBox) { pasted.append(clip.string) return true }, + // Stub the activation. Omitting this defaulted to `KeyInjector.activate`, a + // REAL `NSRunningApplication.activate()` — so these tests yanked the + // developer's foreground app (twice, in the two-app case), and failed + // spuriously whenever `liveTargetApp()`'s unordered pick landed on a + // background-only process whose activate() returns false (the injector then + // throws `.targetAppLost` for reasons unrelated to separator logic). The + // separator tests only need a stable non-nil app identity, not activation. + activateTarget: { _ in true }, clipboard: clip) return (injector, pasted) } diff --git a/Tests/BlurtEngineTests/Stubs/StubMicCapture.swift b/Tests/BlurtEngineTests/Stubs/StubMicCapture.swift index 2da9f50..d22466a 100644 --- a/Tests/BlurtEngineTests/Stubs/StubMicCapture.swift +++ b/Tests/BlurtEngineTests/Stubs/StubMicCapture.swift @@ -5,10 +5,7 @@ import Foundation actor StubMicCapture: MicCaptureProtocol { var startCalls = 0 var stopCalls = 0 - // Comfortably above `SyncSTTLimits.minPCMBytes` so the default press→release - // flow clears the too-short guard and reaches transcribe, tracking the engine - // rule so a raised floor can't silently start dropping the canned audio. - var pcmToReturn = Data(count: SyncSTTLimits.minPCMBytes * 2) + var pcmToReturn = StubPCM.aboveMinimum var startError: (any Error & Sendable)? var stopError: (any Error & Sendable)? diff --git a/Tests/BlurtEngineTests/Stubs/StubPCM.swift b/Tests/BlurtEngineTests/Stubs/StubPCM.swift new file mode 100644 index 0000000..b3a95e2 --- /dev/null +++ b/Tests/BlurtEngineTests/Stubs/StubPCM.swift @@ -0,0 +1,16 @@ +import Foundation + +@testable import BlurtEngine + +/// Canned capture audio for the mic stubs. +/// +/// Every stub that returns a PCM blob needs it to clear `DictationSession`'s +/// too-short-audio guard (`SyncSTTLimits.minPCMBytes`), or the transcript is +/// dropped and the suite sees a missing `.pasted` phase rather than an obviously +/// stale constant. Stated once here so raising the engine floor can't leave one +/// stub behind. +enum StubPCM { + /// Comfortably above `SyncSTTLimits.minPCMBytes`, so the default + /// press→release flow reaches transcribe. + static let aboveMinimum = Data(count: SyncSTTLimits.minPCMBytes * 2) +} diff --git a/Tests/BlurtEngineTests/SyncSTTLimitsTests.swift b/Tests/BlurtEngineTests/SyncSTTLimitsTests.swift index 9f21302..2f4c3ee 100644 --- a/Tests/BlurtEngineTests/SyncSTTLimitsTests.swift +++ b/Tests/BlurtEngineTests/SyncSTTLimitsTests.swift @@ -39,6 +39,22 @@ struct SyncSTTLimitsTests { #expect(SyncSTTLimits.minPCMBytes == 3200) } + @Test("durationMs converts a raw S16LE byte count to milliseconds") + func durationMsDerivation() { + // Both the capture log (MicCapture.stop) and the upload log + // (AssemblyAITranscriber.transcribe) report a clip's duration through this, + // and those logs are the documented way to diagnose pipeline latency — so + // pin the arithmetic rather than only exercising it incidentally. + #expect(SyncSTTLimits.durationMs(ofPCMBytes: SyncSTTLimits.minPCMBytes) == 100) + #expect(SyncSTTLimits.durationMs(ofPCMBytes: 32_000) == 1000) + #expect(SyncSTTLimits.durationMs(ofPCMBytes: 0) == 0) + // An explicit rate covers the transcriber's call, which passes the rate it + // declared on the request rather than defaulting. + #expect(SyncSTTLimits.durationMs(ofPCMBytes: 16_000, rate: 8_000) == 1000) + // A zero/garbage rate must not divide by zero into an Int conversion trap. + #expect(SyncSTTLimits.durationMs(ofPCMBytes: 32_000, rate: 0) == 0) + } + @Test("the capture geometry is the Sync API's 16 kHz") func sampleRatePinned() { // Shared by MicCapture (recording) and the request's declared rate — a diff --git a/Tests/BlurtEngineTests/SystemClipboardTests.swift b/Tests/BlurtEngineTests/SystemClipboardTests.swift index 1a1fe46..974d592 100644 --- a/Tests/BlurtEngineTests/SystemClipboardTests.swift +++ b/Tests/BlurtEngineTests/SystemClipboardTests.swift @@ -11,14 +11,18 @@ import Testing @Suite("SystemClipboard", .serialized) struct SystemClipboardTests { - /// Snapshot/restore the user's clipboard string around a test body so the - /// suite leaves the real pasteboard as it found it. + /// Snapshot/restore the user's clipboard around a test body so the suite leaves + /// the real pasteboard as it found it. + /// + /// Uses the full `snapshot()`/`restore()` primitives rather than just the string + /// flavor: snapshotting only `.string` and then clearing unconditionally meant + /// running these tests wiped a developer's real clipboard whenever it held an + /// image, a file, or styled text. private func withClipboardRestored(_ body: () throws -> Void) rethrows { - let pb = NSPasteboard.general - let saved = pb.string(forType: .string) + let clip = SystemClipboard() + let saved = clip.snapshot() defer { - pb.clearContents() - if let saved { pb.setString(saved, forType: .string) } + if let saved { clip.restore(saved) } } try body() } @@ -37,15 +41,15 @@ struct SystemClipboardTests { } } - @Test("currentItems snapshots contents that restore brings back") - func snapshotRestoreRoundTrips() { - withClipboardRestored { + @Test("snapshot captures contents that restore brings back") + func snapshotRestoreRoundTrips() throws { + try withClipboardRestored { let pb = NSPasteboard.general let clip = SystemClipboard() pb.clearContents() pb.setString("original", forType: .string) - let snapshot = clip.currentItems() + let snapshot = try #require(clip.snapshot()) clip.setString("overwritten") #expect(pb.string(forType: .string) == "overwritten") @@ -56,12 +60,12 @@ struct SystemClipboardTests { } @Test("snapshot/restore preserves every representation of multi-type, multi-item contents") - func multiTypeSnapshotRoundTrips() { + func multiTypeSnapshotRoundTrips() throws { // The reason `SendablePasteboardItem` keys data by pasteboard *type*: a copy // of styled text carries several representations (plain string + RTF), and // the restore must bring all of them back — a string-only round trip would // silently downgrade the user's clipboard to plain text. - withClipboardRestored { + try withClipboardRestored { let pb = NSPasteboard.general let clip = SystemClipboard() @@ -72,7 +76,7 @@ struct SystemClipboardTests { plain.setString("second item", forType: .string) pb.clearContents() pb.writeObjects([styled, plain]) - let snapshot = clip.currentItems() + let snapshot = try #require(clip.snapshot()) clip.setString("overwritten") clip.restore(snapshot) @@ -85,14 +89,54 @@ struct SystemClipboardTests { } } - @Test("restore of an empty snapshot leaves the cleared pasteboard empty") - func restoreEmptyIsNoOp() { + @Test("a degraded snapshot never clears the clipboard it cannot replace") + func degradedSnapshotDoesNotDestroy() { + // The failure this guards: an item that exists but whose representations can't + // be materialized (a promise the owning app declines) used to produce an + // all-empty snapshot, and `restore` cleared first and wrote nothing — so the + // user's clipboard came back EMPTY instead of unchanged. + withClipboardRestored { + let pb = NSPasteboard.general + let clip = SystemClipboard() + + // An item with a declared type but no retrievable data models the promise. + let degraded = PasteboardSnapshot( + items: [SendablePasteboardItem(dataMap: [:])], plainText: nil) + + clip.setString("transcript") + clip.restore(degraded) + + // Left alone rather than emptied — the transcript is still recoverable. + #expect(pb.string(forType: .string) == "transcript") + } + } + + @Test("a partly-readable snapshot falls back to the plain-text floor") + func degradedSnapshotUsesTextFloor() { withClipboardRestored { let pb = NSPasteboard.general let clip = SystemClipboard() + let degraded = PasteboardSnapshot( + items: [SendablePasteboardItem(dataMap: [:])], plainText: "original text") + + clip.setString("transcript") + clip.restore(degraded) + + // Not byte-faithful (the richer flavors are unrecoverable), but the user's + // text survives instead of their clipboard being emptied. + #expect(pb.string(forType: .string) == "original text") + } + } + + @Test("restore of an empty snapshot leaves the cleared pasteboard empty") + func restoreEmptyIsNoOp() throws { + try withClipboardRestored { + let pb = NSPasteboard.general + let clip = SystemClipboard() + pb.clearContents() // an empty pasteboard has no items to snapshot - let empty = clip.currentItems() + let empty = try #require(clip.snapshot()) clip.setString("temp") clip.restore(empty) diff --git a/Tests/BlurtEngineTests/TranscriptionPromptTests.swift b/Tests/BlurtEngineTests/TranscriptionPromptTests.swift index f624289..30ce482 100644 --- a/Tests/BlurtEngineTests/TranscriptionPromptTests.swift +++ b/Tests/BlurtEngineTests/TranscriptionPromptTests.swift @@ -113,7 +113,7 @@ struct TranscriptionPromptTests { #expect(TranscriptionPrompt.build(context: c.context) == c.expected) } - @Test("built prompt fits within the Sync API 4096-character cap for capped prior text") + @Test("built prompt fits within the self-imposed characterCap for capped prior text") func withinCap() { let longPrior = String(repeating: "word ", count: 200) let prompt = TranscriptionPrompt.build( diff --git a/Tests/BlurtEngineTests/TriggerKeyStoreTests.swift b/Tests/BlurtEngineTests/TriggerKeyStoreTests.swift index b540c9d..9ab1871 100644 --- a/Tests/BlurtEngineTests/TriggerKeyStoreTests.swift +++ b/Tests/BlurtEngineTests/TriggerKeyStoreTests.swift @@ -22,7 +22,7 @@ struct TriggerKeyStoreTests { @Test("an unknown stored code falls back to the default") func unknownFallsBack() { let defaults = freshDefaults() - defaults.set(123, forKey: "BlurtTriggerKeyCode") + defaults.set(123, forKey: TriggerKeyStore.defaultsKey) #expect(TriggerKeyStore(defaults: defaults).triggerKey == .rightCommand) } } diff --git a/scripts/check.sh b/scripts/check.sh index d9ca98a..a8feb4c 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -43,6 +43,32 @@ MIN_COVERAGE=80 export OS_ACTIVITY_MODE=disable +# Fail the health check with a message. check.sh doesn't source release-lib.sh +# (it is not part of the release pipeline), so it carries its own. +die_check() { + echo "error: $*" >&2 + exit 1 +} + +# Guard for the optional linters below: true when $1 is on PATH, otherwise emits +# the standard skip note (with install hint $2) and returns false. On success it +# also cds to REPO_ROOT, because every one of those checks runs from the repo +# root — so wiring up a new tool can't forget either half. Use as: +# if tool_ready prettier 'brew install prettier'; then … fi +tool_ready() { + if command -v "$1" >/dev/null 2>&1; then + # `|| return 1` is load-bearing: as an `if` *condition* this function runs + # with errexit suppressed, so a failed cd would otherwise fall through to + # `return 0` and run the linter from the wrong directory. + cd "$REPO_ROOT" || return 1 + return 0 + fi + # ${2:-} so a one-arg call can't abort the whole script under `set -u` — and + # only on a machine where the tool is missing, the hardest path to notice. + echo "note: $1 not installed; skipping (${2:-})" + return 1 +} + # No-external-dependencies guard. The engine is dependency-free by rule and the # app carries only the local BlurtEngine package (see AGENTS.md). A third-party # dependency is the single biggest supply-chain risk, so fail the moment one is @@ -95,26 +121,30 @@ else PROFDATA="$BIN/codecov/default.profdata" XCTEST_BUNDLE="$(find "$BIN" -maxdepth 1 -name '*PackageTests.xctest' -print -quit)" XCTEST_BIN="$XCTEST_BUNDLE/Contents/MacOS/$(basename "$XCTEST_BUNDLE" .xctest)" - if command -v python3 >/dev/null 2>&1 && [ -f "$PROFDATA" ] && [ -f "$XCTEST_BIN" ]; then - # Exclusions (so the figure reflects deterministically-testable engine code): - # - Tests/ : test files themselves, not shipping code. - # - MicCapture.swift : the AVAudioRecorder capture actor. It needs a real - # audio device, so it can't run in CI (its integration - # test, MicCaptureLevelsTests, is env-gated for the same - # reason). Its pure meter math lives in - # MicCapture+Meter.swift, which IS covered. Keep this - # list tight — exclude only code that genuinely cannot - # be exercised without hardware. - COVERAGE="$(xcrun llvm-cov export -summary-only -instr-profile "$PROFDATA" "$XCTEST_BIN" \ - -ignore-filename-regex='Tests/|Audio/MicCapture\.swift' \ - | python3 -c 'import sys,json; print(round(json.load(sys.stdin)["data"][0]["totals"]["lines"]["percent"],2))')" - echo "engine line coverage: ${COVERAGE}%" - if ! awk -v c="$COVERAGE" -v min="$MIN_COVERAGE" 'BEGIN{ exit (c+0 < min+0) }'; then - echo "error: coverage ${COVERAGE}% is below the ${MIN_COVERAGE}% floor" - exit 1 - fi - else - echo "note: skipping coverage gate (need python3 + a coverage build)" + # These must exist after `swift test --enable-code-coverage`. Previously a + # missing one (a renamed test bundle, a coverage build that didn't happen) + # turned the >=80% floor into a printed note and check.sh still exited 0 — the + # gate vanished silently and CI stayed green. Fail instead. + [ -n "$XCTEST_BUNDLE" ] || die_check "no *PackageTests.xctest found under $BIN — coverage gate cannot run" + [ -f "$PROFDATA" ] || die_check "no coverage profile at $PROFDATA — coverage gate cannot run" + [ -f "$XCTEST_BIN" ] || die_check "no test binary at $XCTEST_BIN — coverage gate cannot run" + command -v python3 >/dev/null 2>&1 || die_check "python3 not found — needed to read the coverage summary" + # Exclusions (so the figure reflects deterministically-testable engine code): + # - Tests/ : test files themselves, not shipping code. + # - MicCapture.swift : the AVAudioRecorder capture actor. It needs a real + # audio device, so it can't run in CI (its integration + # test, MicCaptureLevelsTests, is env-gated for the same + # reason). Its pure meter math lives in + # MicCapture+Meter.swift, which IS covered. Keep this + # list tight — exclude only code that genuinely cannot + # be exercised without hardware. + COVERAGE="$(xcrun llvm-cov export -summary-only -instr-profile "$PROFDATA" "$XCTEST_BIN" \ + -ignore-filename-regex='Tests/|Audio/MicCapture\.swift' \ + | python3 -c 'import sys,json; print(round(json.load(sys.stdin)["data"][0]["totals"]["lines"]["percent"],2))')" + echo "engine line coverage: ${COVERAGE}%" + if ! awk -v c="$COVERAGE" -v min="$MIN_COVERAGE" 'BEGIN{ exit (c+0 < min+0) }'; then + echo "error: coverage ${COVERAGE}% is below the ${MIN_COVERAGE}% floor" + exit 1 fi echo "==> swift test --sanitize=thread (data-race detection)" @@ -122,14 +152,19 @@ else # shared mutable state at runtime — catching data races regardless of test # ordering (e.g. an unguarded global touched by parallel tests). Runs the # suite a second time against a TSan-instrumented build. - swift test --sanitize=thread -Xswiftc -warnings-as-errors + # + # Each sanitizer gets its own --scratch-path: the three passes compile with + # different swiftc flags, so sharing the default .build makes every pass + # invalidate the previous one's artifacts (and leaves .build poisoned for the + # next run's plain pass). Separate paths keep three warm incremental caches. + swift test --sanitize=thread --scratch-path "$REPO_ROOT/.build/tsan" -Xswiftc -warnings-as-errors echo "==> swift test --sanitize=address (memory-safety detection)" # AddressSanitizer catches use-after-free, buffer overflows, and other memory # corruption at runtime. (LeakSanitizer is unsupported on Darwin, so this does # NOT find retain-cycle leaks — those are covered by the weak-reference # assertions in MemoryLeakTests.swift.) - swift test --sanitize=address -Xswiftc -warnings-as-errors + swift test --sanitize=address --scratch-path "$REPO_ROOT/.build/asan" -Xswiftc -warnings-as-errors echo "==> xcodegen (App/Blurt)" cd "$APP_DIR" @@ -212,9 +247,8 @@ else echo "note: swift-format not installed; skipping (Swift formatting is checked on CI)" fi -if command -v swiftlint >/dev/null 2>&1; then +if tool_ready swiftlint 'brew install swiftlint'; then echo "==> swiftlint" - cd "$REPO_ROOT" # Covers what swift-format can't: correctness smells and complexity limits # (config in the sibling .swiftlint.yml). --strict promotes warnings to # failures, so any lint violation fails the build — keep the tree lint-clean. @@ -228,46 +262,34 @@ if command -v swiftlint >/dev/null 2>&1; then # OSLog are suppressed via always_keep_imports in .swiftlint.yml. swiftlint analyze --strict --quiet --compiler-log-path "$APP_BUILD_LOG" fi -else - echo "note: swiftlint not installed; skipping (brew install swiftlint)" fi if [ "$PORTABLE" -eq 0 ]; then - if command -v periphery >/dev/null 2>&1; then + if tool_ready periphery 'brew install periphery'; then echo "==> periphery" - cd "$REPO_ROOT" # --strict promotes any unused-code finding to a non-zero exit. # Periphery does its own xcodebuild + index — separate from the build above # because reusing DerivedData reliably across machines is fragile. periphery scan --strict --quiet - else - echo "note: periphery not installed; skipping (brew install periphery)" fi fi -if command -v actionlint >/dev/null 2>&1; then +if tool_ready actionlint 'brew install actionlint'; then echo "==> actionlint" - cd "$REPO_ROOT" actionlint -else - echo "note: actionlint not installed; skipping (brew install actionlint)" fi -if command -v prettier >/dev/null 2>&1; then +if tool_ready prettier 'brew install prettier'; then echo "==> prettier --check" - cd "$REPO_ROOT" # Formatting authority for the repo's non-Swift text: CI/config (yml/yaml), # docs (md), and the GitHub Pages site (html/css — which also covers the # JSON-LD embedded in site/index.html). JSON is intentionally left out of the # glob: the only non-conforming file is the Xcode-generated AppIcon icon.json, # which must not be reformatted by hand. prettier --check '**/*.{yml,yaml,md,html,css}' -else - echo "note: prettier not installed; skipping (brew install prettier)" fi -if command -v xmllint >/dev/null 2>&1; then - cd "$REPO_ROOT" +if tool_ready xmllint 'ships with libxml2'; then # Prettier can't format XML without a plugin (and this repo has no JS toolchain # to add one), so libxml2's xmllint validates well-formedness instead — covers # the GitHub Pages sitemap. A parse error fails the check; --noout drops the @@ -279,13 +301,10 @@ if command -v xmllint >/dev/null 2>&1; then # shellcheck disable=SC2086 xmllint --noout $XML_FILES fi -else - echo "note: xmllint not installed; skipping XML check (ships with libxml2)" fi -if command -v markdownlint >/dev/null 2>&1; then +if tool_ready markdownlint 'brew install markdownlint-cli'; then echo "==> markdownlint" - cd "$REPO_ROOT" # Structural lint for the repo's Markdown (config in .markdownlint.jsonc; # prose-wrapping rules are off there since prettier owns Markdown formatting). # CLAUDE.md is a short compatibility shim that points agents at AGENTS.md, so @@ -293,18 +312,13 @@ if command -v markdownlint >/dev/null 2>&1; then # drafts) is excluded too — prose, not shipped source (also in .markdownlintignore; # filtered here as well since the file list is passed to markdownlint as args). git ls-files '*.md' | grep -vx 'CLAUDE.md' | grep -vE '^docs/' | xargs markdownlint -else - echo "note: markdownlint not installed; skipping (brew install markdownlint-cli)" fi -if command -v shellcheck >/dev/null 2>&1; then +if tool_ready shellcheck 'brew install shellcheck'; then echo "==> shellcheck" - cd "$REPO_ROOT" # Static analysis for the project's shell scripts (release-*, check.sh # itself) — catches quoting bugs, unset vars, and unsafe patterns. shellcheck scripts/*.sh -else - echo "note: shellcheck not installed; skipping (brew install shellcheck)" fi echo "==> release.sh unit tests" diff --git a/scripts/dev-build.sh b/scripts/dev-build.sh index 8800faa..ae24376 100755 --- a/scripts/dev-build.sh +++ b/scripts/dev-build.sh @@ -17,13 +17,10 @@ REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" APP_DIR="$REPO_ROOT/App/Blurt" DERIVED_BASE="/tmp/blurt-build" -info() { printf '\033[34m▸\033[0m %s\n' "$*"; } -die() { - printf '\033[31m✗\033[0m %s\n' "$*" >&2 - exit 1 -} +# shellcheck source=scripts/release-lib.sh +source "$REPO_ROOT/scripts/release-lib.sh" -command -v xcodebuild >/dev/null 2>&1 || die "missing required tool: xcodebuild" +require_tools xcodebuild if command -v xcbeautify >/dev/null 2>&1; then PRETTY=(xcbeautify --quiet) diff --git a/scripts/leaks.sh b/scripts/leaks.sh index a247ce2..4d4fe94 100755 --- a/scripts/leaks.sh +++ b/scripts/leaks.sh @@ -75,13 +75,23 @@ APP_PID=$! sleep "$SETTLE_SECONDS" echo "==> scanning pid $APP_PID with the leak detector" +# The app must still be alive, or `leaks` has nothing to attach to and the report +# below is an error string rather than a scan. +kill -0 "$APP_PID" 2>/dev/null \ + || die "app exited before the scan could run (see /tmp/blurt-leaks-app.log)" # `leaks` returns non-zero when it finds any leak; we do our own attribution # below, so don't let its exit status abort the script. leaks "$APP_PID" >"$REPORT" 2>&1 || true { kill "$APP_PID" && wait "$APP_PID"; } >/dev/null 2>&1 || true -TOTAL_LINE="$(grep -oE '[0-9]+ leaks for [0-9]+ total leaked bytes' "$REPORT" | head -1)" -echo "==> ${TOTAL_LINE:-no leak summary} (full report: $REPORT)" +# `|| true` is required: under `set -euo pipefail` a non-matching grep exits 1 and +# pipefail propagates it, which aborted the whole script *at the assignment* — so a +# scan that couldn't attach printed nothing at all and the `:-` fallback below was +# unreachable. CI then showed a bare non-zero exit indistinguishable from a real leak. +TOTAL_LINE="$(grep -oE '[0-9]+ leaks for [0-9]+ total leaked bytes' "$REPORT" | head -1 || true)" +[ -n "$TOTAL_LINE" ] \ + || die "leaks produced no summary — the scan did not run. Report: $REPORT" +echo "==> $TOTAL_LINE (full report: $REPORT)" # Attribute: count leak-graph / backtrace lines whose module column is Blurt's # own binary (the executable, its Debug dylib, or statically-linked BlurtEngine). diff --git a/scripts/release-build.sh b/scripts/release-build.sh index 36d67a5..80755ca 100755 --- a/scripts/release-build.sh +++ b/scripts/release-build.sh @@ -177,9 +177,8 @@ else fi step "Preflight" -for cmd in xcodegen xcodebuild xcrun hdiutil codesign spctl create-dmg awk shasum; do - command -v "$cmd" >/dev/null 2>&1 || die "missing required tool: $cmd (brew install create-dmg if needed)" -done +require_tools --hint='brew install create-dmg if needed' \ + xcodegen xcodebuild xcrun hdiutil codesign spctl create-dmg awk shasum xcrun notarytool history --keychain-profile "$NOTARY_PROFILE" >/dev/null 2>&1 \ || die "notarytool profile '$NOTARY_PROFILE' not found. Run: xcrun notarytool store-credentials $NOTARY_PROFILE --apple-id --team-id $TEAM_ID --password " diff --git a/scripts/release-bump.sh b/scripts/release-bump.sh index d98a828..d50fe7c 100755 --- a/scripts/release-bump.sh +++ b/scripts/release-bump.sh @@ -19,14 +19,13 @@ NEW_VERSION="$1" is_semver "$NEW_VERSION" || die "version must be X.Y.Z (got: $NEW_VERSION)" step "Preflight" -command -v xcodegen >/dev/null 2>&1 || die "missing required tool: xcodegen" +require_tools xcodegen [ -f "$PROJECT_YML" ] || die "not found: $PROJECT_YML" require_clean_tree "bumping" -CURRENT_VERSION="$(parse_short_version <"$PROJECT_YML")" +CURRENT_VERSION="$(require_project_version "$PROJECT_YML")" CURRENT_BUILD="$(parse_bundle_version <"$PROJECT_YML")" -[ -n "$CURRENT_VERSION" ] || die "could not parse CFBundleShortVersionString from $PROJECT_YML" [ -n "$CURRENT_BUILD" ] || die "could not parse CFBundleVersion from $PROJECT_YML" [[ "$CURRENT_BUILD" =~ ^[0-9]+$ ]] || die "CFBundleVersion is not an integer: $CURRENT_BUILD" diff --git a/scripts/release-install.sh b/scripts/release-install.sh index fc436db..ec89182 100755 --- a/scripts/release-install.sh +++ b/scripts/release-install.sh @@ -21,9 +21,7 @@ source "$REPO_ROOT/scripts/release-lib.sh" LSREGISTER="/System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister" step "Preflight" -for cmd in xcrun hdiutil codesign ditto awk; do - command -v "$cmd" >/dev/null 2>&1 || die "missing required tool: $cmd" -done +require_tools xcrun hdiutil codesign ditto awk VERSION="$(require_project_version "$APP_DIR/project.yml")" info "version: $VERSION" diff --git a/scripts/release-lib.sh b/scripts/release-lib.sh index 03046b4..a66868d 100644 --- a/scripts/release-lib.sh +++ b/scripts/release-lib.sh @@ -1,7 +1,8 @@ #!/usr/bin/env bash -# Shared helpers for the release scripts (release.sh, release-bump.sh, -# release-build.sh, release-install.sh, release-publish.sh). Sourced, never -# executed. Everything here must stay side-effect-free at source time — +# Shared helpers for the repo's bash scripts — the release pipeline (release.sh, +# release-bump.sh, release-build.sh, release-install.sh, release-publish.sh) plus +# dev-build.sh, which reuses the logging and tool-preflight helpers. Sourced, +# never executed. Everything here must stay side-effect-free at source time — # release.test.sh sources release.sh (which sources this) to unit-test the # pure helpers. @@ -16,6 +17,24 @@ die() { # --- shared guards (need REPO_ROOT set by the sourcing script) --- +# Die unless every named command is on PATH. The one definition of the +# required-tool preflight each release step opens with; pass the tools it needs +# (e.g. `require_tools xcrun hdiutil codesign ditto awk`). An optional leading +# `--hint=` is appended to the failure message for tools that aren't +# preinstalled (e.g. `--hint='brew install create-dmg if needed'`). +require_tools() { + local cmd hint="" + case "${1:-}" in + --hint=*) + hint=" (${1#--hint=})" + shift + ;; + esac + for cmd in "$@"; do + command -v "$cmd" >/dev/null 2>&1 || die "missing required tool: $cmd$hint" + done +} + # Die unless the git working tree is clean; $1 names the action for the message # (e.g. "publishing" -> "… commit or stash before publishing"). require_clean_tree() { @@ -24,8 +43,12 @@ require_clean_tree() { } # Echo CFBundleShortVersionString read from the project.yml at $1, dying when -# it can't be parsed. Call as: VERSION="$(require_project_version "$path")" — -# the die inside the substitution fails the assignment under `set -e`. +# it can't be parsed. Call as a PLAIN assignment: +# VERSION="$(require_project_version "$path")" +# so the die inside the substitution fails the assignment under `set -e`. Do NOT +# write `local v="$(require_project_version …)"` — `local`/`declare`/`export` +# swallow the substitution's exit status, so the die prints but execution +# continues with an empty value. Declare first, assign on the next line. require_project_version() { local version version="$(parse_short_version <"$1")" @@ -92,15 +115,26 @@ version_gt() { [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -n1)" = "$1" ] } +# Read the scalar value of YAML key $1 from content on stdin, stripping single or +# double quotes. Matches the key as a whole awk field ($1 on an indented line is +# the key), which the previous unanchored `/key:/` regex did not: `MyCFBundleVersion:` +# matched and returned the wrong value, and a commented-out `# CFBundleVersion:` +# matched with `$2` being the key name itself. (A longer key sharing the prefix, +# like `CFBundleVersionSomethingElse:`, was never a hazard — the old regex already +# required the colon.) All three are pinned in release.test.sh. +parse_yaml_scalar() { + awk -v key="$1:" '$1 == key {gsub(/["'"'"']/, "", $2); print $2; exit}' +} + # Read CFBundleShortVersionString from project.yml content on stdin. The one # definition of the version-read rule every release script gates on. parse_short_version() { - awk '/CFBundleShortVersionString:/ {gsub(/"/, "", $2); print $2; exit}' + parse_yaml_scalar CFBundleShortVersionString } # Read CFBundleVersion (the integer build number) from project.yml on stdin. parse_bundle_version() { - awk '/CFBundleVersion:/ {gsub(/"/, "", $2); print $2; exit}' + parse_yaml_scalar CFBundleVersion } # Read the full commit SHA from build-info.txt content on stdin. diff --git a/scripts/release-publish.sh b/scripts/release-publish.sh index 8322629..471c65b 100755 --- a/scripts/release-publish.sh +++ b/scripts/release-publish.sh @@ -22,9 +22,7 @@ done source "$REPO_ROOT/scripts/release-lib.sh" step "Preflight" -for cmd in gh git xcrun awk; do - command -v "$cmd" >/dev/null 2>&1 || die "missing required tool: $cmd" -done +require_tools gh git xcrun awk VERSION="$(require_project_version "$APP_DIR/project.yml")" DMG="$BUILD_ROOT/Blurt-$VERSION.dmg" @@ -37,6 +35,22 @@ info "version: $VERSION" info "dmg: $DMG" info "dsym: $DSYM_ZIP" +# The artifacts must have been built from the commit we are about to tag. +# Without this, building at commit A and then pulling any commit that doesn't +# touch CFBundleShortVersionString (a code fix, a doc change) lets this script +# tag commit B and publish A's binary under it — a release whose page and whose +# bits disagree, with every other check still passing. `release.sh` already +# makes exactly this comparison to decide whether a rebuild is needed +# (`dmg_already_built`); the publish step is where it actually protects users. +BUILD_INFO="$BUILD_ROOT/build-info.txt" +[ -f "$BUILD_INFO" ] || die "build provenance not found at $BUILD_INFO — rebuild with scripts/release-build.sh" +BUILT_SHA="$(parse_build_info_git_sha <"$BUILD_INFO")" +[ -n "$BUILT_SHA" ] || die "could not parse the built commit from $BUILD_INFO — rebuild with scripts/release-build.sh" +HEAD_SHA="$(git -C "$REPO_ROOT" rev-parse HEAD)" +[ "$BUILT_SHA" = "$HEAD_SHA" ] \ + || die "DMG was built from $BUILT_SHA but HEAD is $HEAD_SHA — rebuild with scripts/release-build.sh before publishing" +info "built at: $BUILT_SHA (matches HEAD)" + step "Validate staple" xcrun stapler validate "$DMG" >/dev/null || die "DMG not stapled — rebuild with release-build.sh" @@ -112,13 +126,18 @@ ln -f "$DSYM_ZIP" "$STABLE_DSYM" # SHA256SUMS itself is NOT attached as an asset: the notarized Developer-ID # signature is the real integrity/authenticity guarantee (Gatekeeper on first # open), so the checksums are informational and live in the release notes body. +# Created as a DRAFT on purpose. `--latest` repoints +# releases/latest/download/Blurt.dmg — the URL README.md links — so publishing +# before the assets are verified put a possibly-truncated upload in front of +# users for the length of the verify step. The draft is flipped live at the end, +# after the re-download + sha + staple checks pass. gh release create "$TAG" \ "$STABLE_DMG" \ "$DMG" \ "$STABLE_DSYM" \ --title "$TAG" \ --generate-notes \ - --latest + --draft # Fold the checksums into the generated notes (see above for why not as an asset). GENERATED_NOTES="$(gh release view "$TAG" --json body -q .body)" @@ -155,5 +174,10 @@ rm -rf "$VERIFY_DIR" trap - EXIT info "published assets verified (sha + staple match the local build)" +# Everything checked out — now make it visible and repoint /latest. +step "Publish" +gh release edit "$TAG" --draft=false --latest \ + || die "assets verified but flipping the draft live failed — re-run with --republish" + URL="$(gh release view "$TAG" --json url -q .url)" info "published: $URL" diff --git a/scripts/release.sh b/scripts/release.sh index d561cc8..e7b066a 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -204,9 +204,7 @@ main() { is_semver "$version" || die "version must be X.Y.Z (got: $version)" fi - for cmd in git gh xcodegen awk; do - command -v "$cmd" >/dev/null 2>&1 || die "missing required tool: $cmd" - done + require_tools git gh xcodegen awk require_clean_tree "releasing" diff --git a/scripts/release.test.sh b/scripts/release.test.sh index d7d7d28..0373cfa 100755 --- a/scripts/release.test.sh +++ b/scripts/release.test.sh @@ -48,10 +48,23 @@ check "parses version" "0.1.5" \ "$(printf ' CFBundleVersion: "6"\n CFBundleShortVersionString: "0.1.5"\n' | parse_short_version)" check "takes first match only" "0.1.5" \ "$(printf ' CFBundleShortVersionString: "0.1.5"\n CFBundleShortVersionString: "9.9.9"\n' | parse_short_version)" - +check "strips single quotes" "1.2.3" \ + "$(printf " CFBundleShortVersionString: '1.2.3'\n" | parse_short_version)" + +# Pins what the whole-field key match (`$1 == key`) buys over the old unanchored +# substring regex. Note `CFBundleVersionSomethingElse` was NOT a real hazard -- +# the old `/CFBundleVersion:/` already required the colon. The two cases that did +# break are a key with a *prefix* and a commented-out key (where the old awk +# printed `$2`, i.e. the literal key name, as the version). echo "== parse_bundle_version ==" check "parses build number" "6" \ "$(printf ' CFBundleShortVersionString: "0.1.5"\n CFBundleVersion: "6"\n' | parse_bundle_version)" +check "ignores a prefixed key" "32" \ + "$(printf ' MyCFBundleVersion: "99"\n CFBundleVersion: "32"\n' | parse_bundle_version)" +check "ignores a commented-out key" "32" \ + "$(printf ' # CFBundleVersion: "99"\n CFBundleVersion: "32"\n' | parse_bundle_version)" +check "ignores a longer key sharing the prefix" "32" \ + "$(printf ' CFBundleVersionSomethingElse: "99"\n CFBundleVersion: "32"\n' | parse_bundle_version)" echo "== parse_build_info_git_sha ==" check "parses build provenance sha" "0123456789abcdef0123456789abcdef01234567" \ diff --git a/scripts/reset-install.sh b/scripts/reset-install.sh index 4eb3fcf..b3237c0 100755 --- a/scripts/reset-install.sh +++ b/scripts/reset-install.sh @@ -36,7 +36,7 @@ if [ -x "$LSREGISTER" ]; then | sort -u \ | while IFS= read -r app; do "$LSREGISTER" -u "$app" >/dev/null 2>&1 && echo " unregistered: $app" || true - done + done || echo " note: lsregister dump failed; skipping unregister sweep" for dest in "/Applications/Blurt.app" "$HOME/Applications/Blurt.app"; do [ -d "$dest" ] && "$LSREGISTER" -f "$dest" >/dev/null 2>&1 && echo " registered: $dest" || true done @@ -49,7 +49,7 @@ defaults delete "$BUNDLE_ID" 2>/dev/null || true # keychain service is `BlurtIdentity.subsystem` (used by APIKeyStore, # Sources/BlurtEngine/Config/APIKeyStore.swift) — the lowercase bundle id to # match macOS convention. Must match that constant. -KEYCHAIN_SERVICE="dev.alex.blurt" +KEYCHAIN_SERVICE="$BUNDLE_ID" KEYCHAIN_ACCOUNT="AssemblyAIAPIKey" echo "==> Deleting AssemblyAI API key from Keychain ($KEYCHAIN_SERVICE / $KEYCHAIN_ACCOUNT)" security delete-generic-password -s "$KEYCHAIN_SERVICE" -a "$KEYCHAIN_ACCOUNT" >/dev/null 2>&1 || true