Skip to content

iOS: composer send waits for the reconnect instead of failing - #1

Open
thingnoy wants to merge 8 commits into
mainfrom
fix/ios-composer-send-waits-for-reconnect
Open

thingnoy wants to merge 8 commits into
mainfrom
fix/ios-composer-send-waits-for-reconnect

Conversation

@thingnoy

Copy link
Copy Markdown
Owner

Summary

Sending from the iOS composer right after coming back to the app often failed with "Couldn't send. Check the connection and try again." Foreground recovery retires the dead RPC client before it redials, and submitComposer failed at once on remoteClient == nil. A paste that died with connectionClosed also never started recovery, so later sends kept failing.

Now a send that lands during recovery stays in .sending and waits (bounded by reconnectAttemptDeadlineNanoseconds, 30s) for the replacement client, then goes through it. connectionClosed on the current client starts dead-connection recovery (RecoveryTrigger.requestConnectionClosed), the same path transportWriteTimedOut takes. With no recovery in flight the send fails at once, as before.

The wait fails the send, keeping the draft and attachments, when it ends without a client, after a sign-out or account switch, or with a client for a different Mac than the one being redialed (foregroundOrRecoveryMacKey, captured before the wait). submitComposer and submitComposerInput share that one check, so a timed-out wait never falls into a second wait in the text submit. This addresses the review on manaflow-ai#19038.

Testing

Focused command, Xcode 26.3 toolchain (Xcode 27 beta 4's Swift 6.4 fails to build the swift-collections 1.7 dependency):

swift test --package-path Packages/iOS/CmuxMobileShell --no-parallel --filter ComposerSendReconnectWaitTests
  • 715bdeadcb (tests only): fails 4 of 5.
  • bd7a7d4581 (review cases added on top of the first fix): fails 2 of 7, sendDuringRecoveryFailsWhenAnotherMacsClientArrives (both entry points) and sendFailsAfterTheFirstWaitWithoutWaitingAgain.
  • 9b5956ed70: passes.

Observed / Not run

Observed at 9b5956ed70:

  • The 5 composer/reconnect suites pass: 152 tests.
  • The full CmuxMobileShell package passes: 1541 tests in 157 suites.
  • python3 scripts/verify-local.py passes 4/4: swift-syntax, localization-defaults, package-groups, feature-flags.

Not run:

  • No tagged iOS build, simulator or device. The live reconnect path is untested.
  • sendFailsAfterTheFirstWaitWithoutWaitingAgain uses a 450 ms sleep against a 300 ms wait deadline to land inside where a second wait would run, so it depends on the clock.
  • Read only, not tested: the connect path sets foregroundMacDeviceID and activeMacInstanceTag with no await after installing the client, so a woken send sees the new Mac identity.
  • Soak coverage is not updated; the unit tests above cover the send-side wait.

Changelog

Fixed: Sending from the iOS composer right after returning to the app waits for the reconnect instead of failing with "Couldn't send"

Proof

Logic change with no UI; covered by the unit tests above. No device clip yet.

🤖 Generated with Claude Code

A composer send that arrives while foreground recovery has retired the
dead client fails at once with "Couldn't send", and a terminal.paste that
dies with connectionClosed only marks the Mac unavailable instead of
starting recovery. These tests pin the intended behavior: wait for the
replacement client (bounded by the reconnect-attempt deadline), fail at
once when nothing is dialing, and start dead-connection recovery on
connectionClosed.

swift test --package-path Packages/iOS/CmuxMobileShell --no-parallel \
  --filter ComposerSendReconnectWaitTests
Fails 4 of 5 (11 issues); sendWithoutClientAndNoRecoveryFailsImmediately
passes because that is today's behavior and stays.
Returning to the app after it sat in the background and sending right
away often showed "Couldn't send. Check the connection and try again."
Foreground recovery retires the dead RPC client before redialing, and
submitComposer failed instantly on remoteClient == nil. A paste that died
with connectionClosed also never started recovery, so later sends kept
failing until a network change or liveness probe did.

submitComposer() and submitComposerInput() now share one wait: while
isRecoveringConnection is true and no client is installed, the send stays
in .sending and parks on a continuation that remoteClient.didSet resumes
when the replacement lands, isRecoveringConnection.didSet resumes when
recovery stops, and a timer resumes at the runtime's
reconnectAttemptDeadlineNanoseconds (30s). The send then goes through the
new client or settles .failed with the draft and attachments kept. With no
recovery in flight the send fails at once as before.

handleMacAvailabilityFailureIfCurrent routes connectionClosed on the
current client and generation into recoverDeadConnection with the new
RecoveryTrigger.requestConnectionClosed (diagnostic code 12), the same
path transportWriteTimedOut already takes; the owner's redialing guard
and the expectedClient check keep a running recovery from being started
twice.

swift test --package-path Packages/iOS/CmuxMobileShell --no-parallel \
  --filter 'ComposerSendReconnectWaitTests|ComposerSubmitRoutingTests|ComposerSubmitDraftReconcileTests|MobileTaskComposerSubmitTests|ReconnectRouteSelectionTests'
151 tests in 5 suites passed.
A sign-out, account switch, or Mac switch that lands while a send is
parked must not let the identity snapshot adopt the new session. The
deadline task no longer wakes waiters when it is cancelled.

Adds the CHANGELOG line under Unreleased.
Merge-main commit by scripts/merge-main.sh.
Merged by scripts/merge-main.sh: origin/main at f4ff8bc.

Merge-main-previous-head: 51c0e13
Merge-main-base: f4ff8bc
A send whose bounded wait ends with no client waits a second time inside
submitComposerInput, and a client that lands then gets the text without
the sign-in and workspace checks. A client from a different Mac that
installs during the wait also gets the text. The replacement-client test
now reconnects the same Mac, since another Mac must not receive the send.

swift test --package-path Packages/iOS/CmuxMobileShell --no-parallel \
  --filter ComposerSendReconnectWaitTests
Fails 2 of 7 (10 issues).
…returns

awaitRemoteClientDuringConnectionRecovery now returns whether the send
may proceed. After a wait it requires a client, the same signInGeneration,
and a foregroundMacKey equal to the Mac being redialed
(foregroundOrRecoveryMacKey, captured before the wait). submitComposer and
submitComposerInput both fail the send when it returns false, so a timed-out
wait no longer falls into a second 30s wait in the text submit, and a Mac
switch during the wait can no longer route the text to another Mac's
terminal with the same id.

The connect path sets foregroundMacDeviceID and activeMacInstanceTag in the
same synchronous run as installing the client, so a woken waiter sees the
new Mac identity.

swift test --package-path Packages/iOS/CmuxMobileShell --no-parallel \
  --filter 'ComposerSendReconnectWaitTests|ComposerSubmitRoutingTests|ComposerSubmitDraftReconcileTests|MobileTaskComposerSubmitTests|ReconnectRouteSelectionTests'
152 tests in 5 suites passed (red before: 2 of 7 in ComposerSendReconnectWaitTests).

This branch has not been deployed

No deployments
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.

1 participant