Commit d6918b0
authored
fix(desktop): cycle listening through all three audio recording modes (#11640)
## Problem
`AssistantSettings.AudioRecordingMode` has three cases — `off`,
`always`, `onlyMeetings` — but the listening control could only reach
two of them. `toggleListening` read:
```swift
let nextMode = currentMode == .off ? .onlyMeetings : .off
```
so `.always` was **unreachable from the top bar and from Home**. Turning
the microphone "on" always armed the meetings gate, which deliberately
holds the mic shut until a call is detected; there was no way to ask for
continuous recording from either shell. Reported from the top-bar
button: it "only toggles between off and only meetings".
Only Meetings was also indistinguishable from Always On at a glance —
same `mic`, same dot — even though the two behave completely
differently.
## Change
- **A cycle, not a flip.** `CaptureListeningLogic.cycleListening`
advances `Off → Always On → Only Meetings → Off`. Both shells already
share this one function, so both gain the third mode.
- **Only Meetings is marked.** It carries `person.2.fill` in the corner.
The mark is *additive corner ink* like the state dot — the base `mic`
silhouette stays byte-identical in every state, which is the rule the
cluster's header sets and `ShellStatusIconLegibilityTests` measures in
pixels.
- **Only Meetings explains itself once.** Selecting it shows a popover —
"the microphone stays closed until Omi detects a call, then records
until it ends" — which auto-dismisses. It fires on the transition only,
never on hover and never at rest, so the resting cluster stays wordless.
- **The tooltip names the destination.** "Click to stop" was true of a
two-state switch and is a false promise from Always On, where a click
selects Only Meetings and stops nothing.
## Product invariants
**INV-CHAT-1** (one shared transcript across surfaces) — cited because
the diff touches
`desktop/macos/Desktop/Sources/MainWindow/Pages/DashboardPage.swift`,
which is inside its path globs.
The change there is a single call-site rename (`toggleListening` →
`cycleListening`); it does not
touch conversation ownership, transcript routing, or any chat surface,
so the invariant's guard tests
are unchanged.
## Failure class
Failure-Class: FC-split-mutation-authority
The control is the authority for the listening mode's transitions, but
it only ever authored two of the three states the mode declares, leaving
the third reachable exclusively from Settings. The fix is the class's
canonical prevention: one transition function owns the whole state
machine, and `testTheCycleCoversEveryDeclaredMode` fails if a future
mode is added that the control cannot select.
## Review follow-ups (second commit)
Both findings from the automated review were valid and are fixed:
**P1 — the microphone stayed open when Only Meetings could not yet prove
a call.** `reconcileCapture`'s
`guard meetingStateReady else { return }` returned *before* either pause
branch. Selecting Only Meetings
from a live Always session builds a fresh detector, so that first pass
runs with
`hasObservedState == false` and the microphone the previous mode opened
kept running until the detector's
first asynchronous probe landed. A gate the user selects in order to
close the mic has to fail closed, so
"not known yet" now means "not in a call". `MeetingGateReadinessPolicy`
names the rule and
`pauseCaptureWhileMeetingGateUnknown` takes the same stop the normal
gating takes, earlier.
This path only became reachable from the button in this PR: before it,
Always On could not be selected
outside Settings, so `Always → Only Meetings` was not a transition the
control could make.
**P2 — the tooltip promised a mode the click could not reach.** Without
the microphone grant, a click
spends itself on the permission prompt and the mode does not move. The
tooltip now says what is actually
missing until the grant exists.
Line-Count-Exception:
desktop/macos/Desktop/Sources/AppState/AppState+Transcription.swift |
1736 -> 1738 | Fail-closed privacy fix for P1. The pause helper and its
policy live in new files (`AppState+MeetingGatePause.swift`,
`MeetingGateReadinessPolicy`); the two lines added here are the guard's
call and its comment, which have to sit at the point reconcileCapture
returns early.
## Verification
- `xcrun swift build -c debug --package-path Desktop --build-tests` —
clean.
- `ShellListeningCycleTests` 14/14, `ShellStatusIconLegibilityTests`
13/13, `DashboardCaptureStateTests` 10/10,
`MeetingGatedSystemAudioTests` — **43 tests, 0 failures**.
- **The regression test fails against the shipped behaviour.** Reverting
`nextAudioRecordingMode` to the two-state flip fails 4 tests and
reproduces the report verbatim:
> the listening cycle ran off → onlyMeetings → off → onlyMeetings. It
has to offer all three modes…
- `make preflight` — all checks pass.
### What is *not* verified, explicitly
The final code has **automated coverage only — it was not click-verified
in a running build.** The full three-state cycle, the `person.2.fill`
badge and the popover *were* exercised live (clicking the real control,
reading `defaults` and screenshotting each state) on a build of the
pre-rebase implementation, which carried the same badge, popover and
cycle order against the older two-setting model. That evidence does not
transfer to this diff.
The blocker was environmental, not the change: the summoned shell places
itself on this machine's secondary display, synthesized clicks are not
delivered there (a control-tab click failed too), and the panel
auto-dismisses on any focus change, so it could not be relocated to the
primary display. Worth a reviewer clicking the button once before merge.
## Notes for review
Three `ShellStatusTooltip.audio` call sites in
`ShellStatusIconLegibilityTests` gained a `next:` argument. Two are
mechanical — their assertions (`hasPrefix("Audio")`, `contains("Meetings
only")`) are byte-identical. The third,
`testTheAwaitingMeetingAudioTooltipDoesNotClaimOffOrStart`, changed one
expected literal from `"Click to turn off"` to `"Click for Off"`: from
Only Meetings a click still lands on Off, so the guard's intent is
unchanged and its real assertion — `XCTAssertFalse(contains("Click to
start"))` — is untouched. Only the copy it looks for moved.
🤖 Generated with [Claude Code](https://claude.com/claude-code)10 files changed
Lines changed: 427 additions & 17 deletions
File tree
- desktop/macos
- Desktop
- Sources
- AppState
- MainWindow
- Pages
- QueryShell
- Tests
- changelog/unreleased
- e2e/flows
Lines changed: 37 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
616 | 616 | | |
617 | 617 | | |
618 | 618 | | |
| 619 | + | |
| 620 | + | |
619 | 621 | | |
620 | 622 | | |
621 | 623 | | |
| |||
Lines changed: 15 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
3 | 18 | | |
4 | 19 | | |
5 | 20 | | |
| |||
Lines changed: 38 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
65 | 94 | | |
66 | 95 | | |
67 | | - | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
68 | 101 | | |
69 | | - | |
| 102 | + | |
70 | 103 | | |
71 | | - | |
| 104 | + | |
72 | 105 | | |
73 | 106 | | |
74 | 107 | | |
75 | | - | |
| 108 | + | |
76 | 109 | | |
77 | 110 | | |
78 | 111 | | |
| |||
82 | 115 | | |
83 | 116 | | |
84 | 117 | | |
| 118 | + | |
85 | 119 | | |
86 | 120 | | |
87 | 121 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1799 | 1799 | | |
1800 | 1800 | | |
1801 | 1801 | | |
1802 | | - | |
| 1802 | + | |
1803 | 1803 | | |
1804 | 1804 | | |
1805 | 1805 | | |
| |||
Lines changed: 101 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
207 | 229 | | |
208 | 230 | | |
209 | 231 | | |
| |||
226 | 248 | | |
227 | 249 | | |
228 | 250 | | |
229 | | - | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
230 | 260 | | |
231 | 261 | | |
232 | 262 | | |
233 | 263 | | |
234 | | - | |
| 264 | + | |
235 | 265 | | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
236 | 271 | | |
237 | | - | |
| 272 | + | |
238 | 273 | | |
239 | | - | |
| 274 | + | |
240 | 275 | | |
241 | 276 | | |
242 | 277 | | |
| |||
273 | 308 | | |
274 | 309 | | |
275 | 310 | | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
276 | 321 | | |
277 | 322 | | |
278 | 323 | | |
| |||
305 | 350 | | |
306 | 351 | | |
307 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
308 | 360 | | |
309 | 361 | | |
310 | 362 | | |
| |||
326 | 378 | | |
327 | 379 | | |
328 | 380 | | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
329 | 387 | | |
330 | 388 | | |
331 | 389 | | |
| |||
338 | 396 | | |
339 | 397 | | |
340 | 398 | | |
341 | | - | |
| 399 | + | |
| 400 | + | |
342 | 401 | | |
343 | 402 | | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
344 | 411 | | |
345 | 412 | | |
346 | 413 | | |
| |||
366 | 433 | | |
367 | 434 | | |
368 | 435 | | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
369 | 448 | | |
370 | 449 | | |
371 | 450 | | |
372 | 451 | | |
373 | 452 | | |
374 | | - | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
375 | 457 | | |
376 | 458 | | |
377 | 459 | | |
| |||
382 | 464 | | |
383 | 465 | | |
384 | 466 | | |
385 | | - | |
386 | | - | |
| 467 | + | |
| 468 | + | |
387 | 469 | | |
388 | 470 | | |
389 | 471 | | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
390 | 483 | | |
391 | 484 | | |
392 | 485 | | |
| |||
0 commit comments