Skip to content

fix(voice): prevent floating controls cleanup panic - #218

Merged
johnmatthewtennant merged 1 commit into
mainfrom
jtennant/fix-floating-voice-controls-panic
Aug 26, 2026
Merged

fix(voice): prevent floating controls cleanup panic#218
johnmatthewtennant merged 1 commit into
mainfrom
jtennant/fix-floating-voice-controls-panic

Conversation

@johnmatthewtennant

Copy link
Copy Markdown
Contributor

Summary

Floating voice-controls cleanup could ask Wry for the WKWebView URL after WebKit had already cleared it. Wry 0.55.1 unwraps that missing URL and panics, closing Berd while a voice call is stopping or its controls are being replaced.

Track the controls window's lifecycle revision in native voice state and use that state for matching and cleanup. The floating controls retain the same replacement and stale-window safety without reading a teardown-sensitive WebKit property.

Reviewer-reproducible examples

  1. Start a native voice conversation so the floating controls window appears.
  2. End the conversation or close its owner window while the controls are being dismissed.
  3. Confirm Berd dismisses the matching controls without a Wry url_from_webview panic.
  4. Start another conversation and confirm cleanup from the prior lifecycle does not destroy the replacement controls.

@johnmatthewtennant
johnmatthewtennant marked this pull request as ready for review August 25, 2026 23:56
@johnmatthewtennant
johnmatthewtennant requested a review from a team August 25, 2026 23:56

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Automated code review

APPROVE. Fresh static review of exact three-dot range a1ae169...041b4d5 found no publishable issues. The change replaces teardown-sensitive webview URL reads with lifecycle-bound native revision state, validates registration against the active session and revision, conditionally clears only the captured revision, and preserves replacement-window race protection. Final self-check covered the floating-controls install, replacement, readiness timeout, terminal dismissal, stale cleanup during main-window close, and replacement-lifecycle race flows; accessibility, i18n, navigation/consent guards, async/error/never-completes/lifecycle/race behavior, test honesty, project laws and design-system rules, duplicate overlap, and blocking-evidence requirements. No unsupported or overlapping findings remain. Supplied GitHub evidence was inspected: all listed check runs at the exact head SHA completed successfully; the legacy combined-status endpoint is pending with no status contexts, so required checks still independently govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

@johnmatthewtennant
johnmatthewtennant merged commit 7992c30 into main Aug 26, 2026
9 checks passed
@johnmatthewtennant
johnmatthewtennant deleted the jtennant/fix-floating-voice-controls-panic branch August 26, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants