revert: isolate composer appkit bridge behind explicit events (#24401)

[TirmanSidhu]

Summary

Reverts #24401 (refactor: isolate composer appkit bridge behind explicit events)

🤖 Generated with Claude Code

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 438a3d001e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Avoid re-requesting first responder on every render

updateNSView now schedules a first-responder update on every render while the editor is mounted, instead of only when focus intent changes. Because this executes via DispatchQueue.main.async, queued shouldFocus: true work from normal typing can run after the user clicks another control (before the blur binding propagates), and it can immediately make the composer first responder again. That makes blur unreliable and can steal subsequent keystrokes; this call should be gated to real focus transitions.

Useful? React with 👍 / 👎.

[devin-ai-integration]

Devin Review found 2 potential issues.

View 3 additional findings in Devin Review.

[devin-ai-integration]

🟡 Unconditional scheduleFirstResponderUpdate on every updateNSView can steal focus

The old code gated first-responder changes behind a one-shot pendingRequestFocus command that was only set when the SwiftUI layer explicitly wanted to change focus. The new code calls scheduleFirstResponderUpdate(shouldFocus: isFocused) unconditionally on every updateNSView call (lines 236-242). This changes the semantics from "request focus when asked" to "continuously enforce focus state on every render."

This creates a focus-stealing race: when the user clicks away, the focus loss propagates asynchronously (resignFirstResponder → scheduleFocusBindingUpdate(false) → DispatchQueue.main.async { parent.isFocused = false }). Before that async fires, if any unrelated state change triggers updateNSView, it reads stale isFocused = true and enqueues a makeFirstResponder(textView) call. Although the FIFO ordering of DispatchQueue.main.async ensures the binding-update async fires first, writing to parent.isFocused (a @Binding) does not guarantee a synchronous updateNSView — SwiftUI defers layout to the next render cycle. So pendingFirstResponderValue remains true from the stale render, and the subsequent async block passes its guard (pendingFirstResponderValue == shouldFocus) and calls window.makeFirstResponder(textView), stealing focus.

Example scenario: user clicks "Stop generation" button

1. Mouse down → textView.resignFirstResponder → scheduleFocusBindingUpdate(false) → enqueue ASYNC_A
2. Button action → isAssistantBusy changes → isEditable changes → SwiftUI calls updateNSView synchronously
3. updateNSView reads isFocused = true (stale) → scheduleFirstResponderUpdate(true) → pendingFirstResponderValue = true → enqueue ASYNC_B
4. ASYNC_A fires → parent.isFocused = false → SwiftUI DEFERS the resulting updateNSView
5. ASYNC_B fires → pendingFirstResponderValue == true → passes guard → makeFirstResponder(textView) → focus stolen

Prompt for agents

The scheduleFirstResponderUpdate call in updateNSView (ComposerTextEditor.swift lines 236-242) is called unconditionally on every render pass, unlike the old code which gated it behind a one-shot pendingRequestFocus command. This creates a race where stale isFocused values can cause focus-stealing.

To fix this, avoid calling scheduleFirstResponderUpdate on every updateNSView. Instead, only call it when the isFocused value has actually changed since the last call. The coordinator could track the last-requested focus value (e.g. lastRequestedFocus: Bool?) and only schedule a first-responder update when the value differs:

  if coordinator.lastRequestedFocus != isFocused {
      coordinator.lastRequestedFocus = isFocused
      coordinator.scheduleFirstResponderUpdate(in: window, textView: textView, shouldFocus: isFocused)
  }

This restores the one-shot semantics of the old pendingRequestFocus command pattern while keeping the binding-based architecture.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 composerController.focusChanged() is never called — focusIntent is stale

The ComposerController class documents itself as accepting explicit events including focusChanged (ComposerController.swift:10), and focusChanged(_ focused:) at ComposerController.swift:98-100 sets focusIntent. In the old code, the bridge's focusChanged event handler called composerController.focusChanged(focused) whenever the text view gained/lost focus, keeping focusIntent in sync.

In the new code, composerController.focusChanged() is never called from ComposerView. The only caller is tests (ComposerControllerTests.swift:433,436,443). This means focusIntent doesn't track actual focus state — it's only set by interactionEnabledChanged() (which overwrites it unconditionally). The sole external reader (ComposerView.swift:167) reads it immediately after interactionEnabledChanged() overwrites it, so no current code path sees a stale value. However, focusIntent is now dead state that violates the controller's documented API contract. If future code reads focusIntent expecting it to reflect reality, it will get incorrect results.

---

Was this helpful? React with 👍 or 👎 to provide feedback.