Skip to content

Commit 1b3425d

Browse files
fix(desktop): harden Notch chat and PTT reliability (#10125)
## Summary - preserve the user-resized Notch chat frame when macOS changes Spaces - keep streaming output from reclaiming the viewport after a reader scrolls away from the live edge - restore the canonical frame for non-chat Notch surfaces after a Space switch - retain completed and failed tool rows as durable chat progress in compact, collapsed-by-default groups whose step count updates while work continues, while keeping the structured agent card as the sole spawned-agent entrypoint - use a shared thin composer shell and transcript fade in regular and Notch chat, reduce the regular-chat header to one compact control row, and add a persistent Home action there - pass a deduplicated, compact skill catalog to chat, keep CLAUDE.md reference-only, and search overflow skills only when a specialized workflow is relevant - give active tools a 90-second no-progress watchdog that preserves the tool-stall terminal reason; use the 60-second generic watchdog only for a silent bridge with no active tool - correlate the desktop, agent, and chat route with an opaque request ID, and retry terminal journal projection once when needed - keep Push-to-Talk on its native voice path when screen capture is temporarily unavailable immediately after Screen Recording is granted - prevent long named-bundle titles from clipping the Screen Recording drag helper, and coalesce idle PTT context refreshes so streaming chat does not churn the warm voice socket ## Product invariants - INV-AGENT-* - INV-AUTH-1 - INV-CHAT-1 - INV-VOICE-1 - INV-INT-1 ## Verification - `desktop/macos/scripts/agent-logic-harness.sh` — passed (Swift lifecycle, agent runtime, and pi extension suites) - `xcrun swift test --package-path Desktop --filter ChatSkillCatalogTests` — passed (2 tests) - `xcrun swift test --package-path Desktop --filter AgentPillLifecycleTests/testFloatingAgentToolCallsUseCompactOneLinePresentation` — passed - `npm run build` and `node --experimental-strip-types scripts/generate-tool-surfaces.mjs --check` in `desktop/macos/agent` — passed - `npx vitest run tests/node-tools.test.ts tests/omi-tool-manifest.test.ts` — passed (20 tests) - `npx --yes tsx --test index.test.ts` in `desktop/macos/pi-mono-extension` — passed - `make preflight` and the bounded pre-push gate — passed - `git diff --check` — passed - rebuilt `/Applications/omi-tool-stall-reliability.app` from `ae2e79f`; health check confirmed development backend and agent protocol 2, and automation confirmed Chat's Home button returns to the dashboard - `xcrun swift test --package-path Desktop --filter ScreenRecordingPermissionPolicyTests` — passed (15 tests) - `xcrun swift test --package-path Desktop --filter RealtimeHubSessionHandoffPolicyTests` — passed (12 tests) - fast-rebuilt `/Applications/omi-tool-stall-reliability.app` from `f9d5a81`; health check confirmed the development backend and runtime protocol 2 Failure-Class: none
2 parents dfa13fd + f9d5a81 commit 1b3425d

53 files changed

Lines changed: 1311 additions & 344 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/scripts/product_file_line_count_ratchet_baseline/desktop-swift-mainwindow.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
{
22
"files": {
3-
"desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift": 1939,
3+
"desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift": 1926,
44
"desktop/macos/Desktop/Sources/MainWindow/Pages/AppsPage.swift": 3643,
55
"desktop/macos/Desktop/Sources/MainWindow/Pages/DashboardPage.swift": 4448,
66
"desktop/macos/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift": 3298,
77
"desktop/macos/Desktop/Sources/MainWindow/Pages/TasksPage.swift": 5518,
88
"desktop/macos/Desktop/Sources/MainWindow/SidebarView.swift": 1570
99
},
1010
"raise_justifications": {
11-
"desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift": "Syncs the local rating shadow to external message.rating changes so an un-rate tap is no longer swallowed after a server refresh.",
11+
"desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift": "Keeps all chat tool groups compact and collapsed while streamed steps increment, with an explicit expansion policy, fixed collapsed height, and regression coverage.",
1212
"desktop/macos/Desktop/Sources/MainWindow/Pages/AppsPage.swift": "Rebase reconciliation applies the pinned swift-format to the main-branch app-detail safety fixes; runtime behavior is unchanged.",
1313
"desktop/macos/Desktop/Sources/MainWindow/Pages/DashboardPage.swift": "Pinned swift-format normalizes line layout without changing this source's runtime behavior.",
1414
"desktop/macos/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift": "Bridge memory search now awaits the active initial or pagination projection before refreshing, preventing an immediate post-navigation marker search from using stale lifecycle capability state.",

.github/scripts/product_file_line_count_ratchet_baseline/desktop-swift-providers.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"files": {
3-
"desktop/macos/Desktop/Sources/Providers/ChatProvider.swift": 6197,
3+
"desktop/macos/Desktop/Sources/Providers/ChatProvider.swift": 6200,
44
"desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift": 2861
55
},
66
"raise_justifications": {
7-
"desktop/macos/Desktop/Sources/Providers/ChatProvider.swift": "Ambient turns without Screen Recording carry an honest screen-unavailable payload instead of silence.",
7+
"desktop/macos/Desktop/Sources/Providers/ChatProvider.swift": "The activity-aware watchdog must remain beside the send-generation, bridge-interrupt, and journal-terminalization boundary it atomically owns; extracting it would split that recovery invariant.",
88
"desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift": "Already-granted permissions no longer reopen System Settings: screen-recording and full-disk-access requests gate their Settings/drag-card opens behind the granted result."
99
},
1010
"threshold": 1500

desktop/macos/Backend-Rust/src/routes/chat/route.rs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,58 @@ pub(super) fn chat_metering_response(decision: &RateDecision) -> Option<Response
5858
/// cap, which covers all realistic floating-bar sessions. History trimming
5959
/// is tracked separately as the longer-term fix.
6060
const CHAT_COMPLETIONS_MAX_BODY_SIZE: usize = 16 * 1024 * 1024;
61+
62+
const OMI_REQUEST_ID_HEADER: &str = "x-omi-request-id";
63+
const RESPONSE_REQUEST_ID_HEADER: &str = "x-request-id";
64+
65+
fn inbound_request_id(headers: &HeaderMap) -> String {
66+
headers
67+
.get(OMI_REQUEST_ID_HEADER)
68+
.and_then(|value| value.to_str().ok())
69+
.filter(|value| {
70+
!value.is_empty()
71+
&& value.len() <= 128
72+
&& value
73+
.bytes()
74+
.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_'))
75+
})
76+
.map(ToOwned::to_owned)
77+
.unwrap_or_else(|| ulid::Ulid::new().to_string())
78+
}
79+
80+
fn attach_request_id(response: &mut Response, request_id: &str) {
81+
if let Ok(value) = request_id.parse() {
82+
response
83+
.headers_mut()
84+
.insert(RESPONSE_REQUEST_ID_HEADER, value);
85+
}
86+
}
87+
6188
async fn chat_completions(
6289
State(state): State<AppState>,
6390
user: PaywalledAuthUser,
6491
headers: HeaderMap,
6592
Json(req): Json<ChatCompletionRequest>,
93+
) -> Result<Response, StatusCode> {
94+
let request_id = inbound_request_id(&headers);
95+
tracing::info!(
96+
event = "chat_completion_request",
97+
request_id = %request_id,
98+
streaming = req.stream,
99+
"chat completion received"
100+
);
101+
let response = chat_completions_inner(state, user, headers, req).await;
102+
response.map(|mut response| {
103+
attach_request_id(&mut response, &request_id);
104+
response
105+
})
106+
}
107+
108+
async fn chat_completions_inner(
109+
state: AppState,
110+
user: PaywalledAuthUser,
111+
headers: HeaderMap,
112+
req: ChatCompletionRequest,
66113
) -> Result<Response, StatusCode> {
67114
let byok_stripped = user.byok_stripped;
68115
let user: AuthUser = user.into();
@@ -207,3 +254,44 @@ pub(crate) fn chat_completions_routes() -> Router<AppState> {
207254
.route("/v2/chat/completions", post(chat_completions))
208255
.layer(DefaultBodyLimit::max(CHAT_COMPLETIONS_MAX_BODY_SIZE))
209256
}
257+
258+
#[cfg(test)]
259+
mod tests {
260+
use axum::{
261+
body::Body,
262+
http::{HeaderMap, HeaderValue},
263+
response::Response,
264+
};
265+
266+
use super::{attach_request_id, inbound_request_id};
267+
268+
#[test]
269+
fn preserves_a_valid_opaque_request_id() {
270+
let mut headers = HeaderMap::new();
271+
headers.insert("x-omi-request-id", HeaderValue::from_static("req_01AB-cd"));
272+
273+
assert_eq!(inbound_request_id(&headers), "req_01AB-cd");
274+
}
275+
276+
#[test]
277+
fn replaces_invalid_request_ids_without_echoing_them() {
278+
let mut headers = HeaderMap::new();
279+
headers.insert(
280+
"x-omi-request-id",
281+
HeaderValue::from_static("request id with spaces"),
282+
);
283+
284+
let request_id = inbound_request_id(&headers);
285+
assert_ne!(request_id, "request id with spaces");
286+
assert!(request_id.parse::<ulid::Ulid>().is_ok());
287+
}
288+
289+
#[test]
290+
fn echoes_the_resolved_request_id_on_the_response() {
291+
let mut response = Response::new(Body::empty());
292+
293+
attach_request_id(&mut response, "req_01AB-cd");
294+
295+
assert_eq!(response.headers()["x-request-id"], "req_01AB-cd");
296+
}
297+
}

desktop/macos/Desktop/Sources/Chat/ChatTurnLifecycle.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ final class ChatJournalWriteCoordinator {
155155
return true
156156
}
157157

158+
/// Retry one idempotent terminal journal operation after transient local
159+
/// projection failure. The caller must first claim terminalization ownership;
160+
/// this helper never reopens that ownership or permits a streaming update.
161+
func retryTerminalization(_ operation: @MainActor () async -> Bool) async -> Bool {
162+
if await operation() { return true }
163+
return await operation()
164+
}
165+
158166
func cancelAll() {
159167
for task in updateTasks.values { task.cancel() }
160168
updateTasks.removeAll()

desktop/macos/Desktop/Sources/Chat/StallDetector.swift

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import Foundation
77
/// Two timers track independently:
88
/// - **Inter-event gap** — ms since the last event of any kind.
99
/// Captures "the bridge stopped streaming."
10-
/// - **Per-tool duration** — ms since each in-flight tool started.
11-
/// Captures "this specific tool is hanging."
10+
/// - **Per-tool no-progress gap** — ms since each in-flight tool last
11+
/// reported progress. Captures "this specific tool stopped moving."
1212
///
1313
/// The detector is pure logic: time is passed in as a parameter on
1414
/// every operation, so tests can drive it instantly without wall-clock
@@ -43,6 +43,11 @@ actor StallDetector {
4343
/// per-tool timer for `id` (typically the `toolUseId`).
4444
case toolStarted(id: String)
4545

46+
/// A `tool_activity` progress update for an in-flight tool. This refreshes
47+
/// only the tool's no-progress clock; it never changes the tool's original
48+
/// start time used by duration diagnostics.
49+
case toolProgress(id: String)
50+
4651
/// A `tool_activity` with status `completed` (or `failed` /
4752
/// `cancelled`) arrived. Clears the per-tool timer for `id` and
4853
/// emits a transition back to `.running` if the tool was promoted.
@@ -71,6 +76,7 @@ actor StallDetector {
7176

7277
/// In-flight tools keyed by `toolUseId`. Removed on completion.
7378
private var toolStartedAtMs: [String: Int] = [:]
79+
private var toolLastProgressAtMs: [String: Int] = [:]
7480
private var toolStates: [String: State] = [:]
7581

7682
// MARK: - Init
@@ -97,15 +103,21 @@ actor StallDetector {
97103
toolStates
98104
}
99105

100-
/// IDs of tools that have exceeded a hard execution budget. The caller owns
101-
/// the recovery action (for example, interrupting the bridge); the detector
102-
/// remains pure and does not perform side effects itself.
103-
func toolIdsExceeding(durationMs: Int, atMs: Int) -> [String] {
104-
toolStartedAtMs.compactMap { id, startedAt in
105-
atMs - startedAt >= durationMs ? id : nil
106+
/// IDs of tools that have gone too long without a progress update. The caller
107+
/// owns the recovery action (for example, interrupting the bridge); the
108+
/// detector remains pure and does not perform side effects itself.
109+
func toolIdsWithoutProgress(durationMs: Int, atMs: Int) -> [String] {
110+
toolLastProgressAtMs.compactMap { id, lastProgressAt in
111+
atMs - lastProgressAt >= durationMs ? id : nil
106112
}
107113
}
108114

115+
/// A generic bridge timeout may fire only after a full quiet interval with
116+
/// no tool in flight. Active tools have their own no-progress watchdog.
117+
func isSilentWithoutActiveTools(durationMs: Int, atMs: Int) -> Bool {
118+
toolStartedAtMs.isEmpty && atMs - lastEventAtMs >= durationMs
119+
}
120+
109121
// MARK: - Observation
110122

111123
/// Record an event at simulated time `atMs` and return any state
@@ -153,11 +165,18 @@ actor StallDetector {
153165
case .toolStarted(let id):
154166
if toolStartedAtMs[id] == nil {
155167
toolStartedAtMs[id] = atMs
168+
toolLastProgressAtMs[id] = atMs
156169
toolStates[id] = .running
157170
}
158171

172+
case .toolProgress(let id):
173+
if toolStartedAtMs[id] != nil {
174+
toolLastProgressAtMs[id] = atMs
175+
}
176+
159177
case .toolCompleted(let id):
160178
toolStartedAtMs.removeValue(forKey: id)
179+
toolLastProgressAtMs.removeValue(forKey: id)
161180
let old = toolStates.removeValue(forKey: id) ?? .running
162181
if old != .running {
163182
// Tool completed while promoted (slow/stalled → done) — UI
@@ -181,9 +200,9 @@ actor StallDetector {
181200
}
182201

183202
// Per-tool timers.
184-
for (toolId, startedAt) in toolStartedAtMs {
185-
let duration = atMs - startedAt
186-
let newToolState = promotedState(forElapsedMs: duration)
203+
for (toolId, lastProgressAt) in toolLastProgressAtMs {
204+
let noProgressGap = atMs - lastProgressAt
205+
let newToolState = promotedState(forElapsedMs: noProgressGap)
187206
let oldToolState = toolStates[toolId] ?? .running
188207
if newToolState != oldToolState {
189208
transitions.append(.tool(id: toolId, from: oldToolState, to: newToolState))

desktop/macos/Desktop/Sources/Chat/StallThresholds.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import Foundation
22

33
/// Time thresholds the `StallDetector` uses to promote inter-event gaps
4-
/// and per-tool durations between `.running`, `.slow`, and `.stalled`.
4+
/// and per-tool no-progress gaps between `.running`, `.slow`, and `.stalled`.
55
///
66
/// Values here are first-pass defaults intended to be tuned later
77
/// against real inter-event-gap and per-tool-duration distributions.
88
/// The struct is fully `Sendable` + `Equatable` so future tuning is a
99
/// trivial constants update with no behavioral churn.
1010
struct StallThresholds: Sendable, Equatable {
11-
/// A gap (inter-event or per-tool) reaching this length promotes to
11+
/// A gap (inter-event or per-tool no-progress) reaching this length promotes to
1212
/// `.slow`. The UI surfaces a "still working…" annotation.
1313
let slowGapMs: Int
1414

desktop/macos/Desktop/Sources/CloudConnectorGuidanceOverlay.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ final class CloudConnectorGuidanceOverlay {
223223
settingsWatchTask?.cancel()
224224
window?.close()
225225

226-
let cardSize = CGSize(width: 180, height: 164)
226+
let cardSize = Self.dragCardSize(appName: appName)
227227
dragCardSize = cardSize
228228
let dragTargetState = ScreenRecordingDragTargetState(frame: anchor)
229229
self.dragTargetState = dragTargetState
@@ -324,6 +324,14 @@ final class CloudConnectorGuidanceOverlay {
324324
return SpatialOverlayGeometry.clamped(proposed, to: visibleFrame, padding: 12)
325325
}
326326

327+
/// A named development bundle can have a much longer display name than the
328+
/// production app. Widen the helper rather than allowing its instruction to
329+
/// render outside the transparent panel and get clipped by AppKit.
330+
static func dragCardSize(appName: String) -> CGSize {
331+
let hasLongDisplayName = appName.count > 16
332+
return CGSize(width: hasLongDisplayName ? 240 : 180, height: hasLongDisplayName ? 180 : 164)
333+
}
334+
327335
static func dragCardInitialAlpha(reduceMotion: Bool) -> CGFloat {
328336
reduceMotion ? 1 : 0
329337
}
@@ -822,7 +830,8 @@ private struct ScreenRecordingDragCardView: View {
822830
.foregroundColor(OmiColors.textPrimary)
823831
.multilineTextAlignment(.center)
824832
.lineSpacing(-1)
825-
.fixedSize(horizontal: true, vertical: true)
833+
.fixedSize(horizontal: false, vertical: true)
834+
.frame(maxWidth: size.width - 20)
826835
.shadow(color: Color.black.opacity(0.65), radius: 3, y: 1)
827836
}
828837
}

desktop/macos/Desktop/Sources/FloatingControlBar/AIResponseView.swift

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ struct AIResponseView: View {
5454
currentContentView
5555
}
5656
.frame(maxWidth: .infinity, maxHeight: .infinity)
57+
.overlay(alignment: .bottom) {
58+
ChatComposerFade()
59+
}
5760

5861
if let shareFeedbackMessage, showShareFeedback {
5962
shareFeedbackBanner
@@ -204,6 +207,7 @@ struct AIResponseView: View {
204207
case .toolCalls(_, let calls):
205208
ToolCallsGroup(
206209
calls: calls,
210+
compact: true,
207211
onOpenAgent: onOpenAgent,
208212
onOpenAgentRef: onOpenAgentRef
209213
)
@@ -448,35 +452,38 @@ struct AIResponseView: View {
448452
.environment(\.colorScheme, .dark)
449453
}
450454

451-
HStack(spacing: OmiSpacing.xs) {
452-
Button(action: { shareLink() }) {
453-
Image(systemName: showShareFeedback ? "checkmark" : "arrowshape.turn.up.right")
455+
VStack(spacing: 0) {
456+
HStack(spacing: OmiSpacing.xs) {
457+
Button(action: { shareLink() }) {
458+
Image(systemName: showShareFeedback ? "checkmark" : "arrowshape.turn.up.right")
459+
.scaledFont(size: OmiType.body)
460+
.foregroundColor(showShareFeedback ? .green : .secondary)
461+
}
462+
.buttonStyle(.plain)
463+
.help("Copy share link")
464+
.disabled(isSharingLink)
465+
466+
TextField("Ask follow up...", text: $followUpText)
467+
.textFieldStyle(.plain)
454468
.scaledFont(size: OmiType.body)
455-
.foregroundColor(showShareFeedback ? .green : .secondary)
456-
}
457-
.buttonStyle(.plain)
458-
.help("Copy share link")
459-
.disabled(isSharingLink)
469+
.padding(.horizontal, OmiSpacing.sm)
470+
.padding(.vertical, OmiSpacing.xs)
471+
.background(Color.white.opacity(isDropTargeted ? 0.18 : 0.10))
472+
.cornerRadius(OmiChrome.elementRadius)
473+
.focused($isFollowUpFocused)
474+
.onSubmit {
475+
sendFollowUp()
476+
}
460477

461-
TextField("Ask follow up...", text: $followUpText)
462-
.textFieldStyle(.plain)
463-
.scaledFont(size: OmiType.body)
464-
.padding(.horizontal, OmiSpacing.sm)
465-
.padding(.vertical, OmiSpacing.xs)
466-
.background(Color.white.opacity(isDropTargeted ? 0.18 : 0.10))
467-
.cornerRadius(OmiChrome.elementRadius)
468-
.focused($isFollowUpFocused)
469-
.onSubmit {
470-
sendFollowUp()
478+
Button(action: { sendFollowUp() }) {
479+
Image(systemName: "arrow.up.circle.fill")
480+
.scaledFont(size: OmiType.heading)
481+
.foregroundColor(canSendFollowUp ? .white : .secondary)
471482
}
472-
473-
Button(action: { sendFollowUp() }) {
474-
Image(systemName: "arrow.up.circle.fill")
475-
.scaledFont(size: OmiType.heading)
476-
.foregroundColor(canSendFollowUp ? .white : .secondary)
483+
.disabled(!canSendFollowUp)
484+
.buttonStyle(.plain)
477485
}
478-
.disabled(!canSendFollowUp)
479-
.buttonStyle(.plain)
486+
.chatComposerShell(fill: OmiColors.backgroundSecondary.opacity(0.82))
480487
}
481488
}
482489
.onDrop(of: [UTType.fileURL], isTargeted: $isDropTargeted, perform: handleAttachmentDrop)

desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarWindow.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -779,15 +779,15 @@ class FloatingControlBarWindow: NSPanel, NSWindowDelegate {
779779
observePttHint()
780780
}
781781

782-
private func performSpacesTransitionGrowIn() {
782+
// Internal so the regression test can exercise the same workspace-transition
783+
// path that the NSWorkspace observer invokes.
784+
func performSpacesTransitionGrowIn() {
783785
updateNotchIslandState()
784786
guard notchModeEnabled, isVisible else { return }
785-
// The panel already lives on every Space (.canJoinAllSpaces), so switching
786-
// Spaces must NOT replay the reveal "pop" — doing so re-zoomed the island on
787-
// every desktop/app switch, which felt excessive. Keep it fully revealed and
788-
// only re-assert the resting frame if the current one has actually drifted
789-
// (e.g. the active screen's notch geometry changed).
787+
// Do not replay the reveal "pop" on Space changes; preserve chat size while
788+
// non-chat surfaces recover their canonical frame from this callback.
790789
state.notchRevealProgress = 1
790+
guard !state.showingAIConversation else { return }
791791
let targetFrame = defaultFrameForCurrentState()
792792
guard !Self.framesEquivalent(frame, targetFrame) else { return }
793793
resizeToFrame(targetFrame, makeResizable: styleMask.contains(.resizable), animated: false)

0 commit comments

Comments
 (0)