Optimistic send UI + bounded REST timeout for bad networks - #106
Merged
Conversation
On a bad network the send button appeared dead: Android had no optimistic message insertion (iOS does) and the shared OkHttp client used readTimeout(0), so a half-open connection could hang for minutes with zero feedback. - Split the network client: REST calls get a bounded 60s read timeout so a stuck request fails fast; the SSE stream keeps an infinite read timeout on its own client (it can be silent for minutes while the agent works). - Insert an optimistic user row (deterministic msg_<uuid> id) immediately on send, clear the composer, and show a spinner on the send button. - Reconcile in loadMessages/loadMoreMessages by id membership: keep pending rows the server has not echoed yet, prune confirmed ids. On failure, drop the row and restore the composer text/attachments. - Pass the client-chosen messageID through PromptRequest so the server echoes the same id back (pure id-membership reconciliation).
Address review findings on the optimistic send UI: - Handle the session.error SSE event. prompt_async acknowledges with 204 before the turn runs, so a failure that happens before the user message is persisted (agent missing, session deleted, crash) only surfaces through this event. Previously it was dropped, leaving a permanent ghost optimistic row and silently losing the user's text. Now, for the current session, drop the pending optimistic rows, recover their text into the composer, and set an error. Add parseSessionErrorReason to build a bounded display reason. - Guard the send-failure composer rollback by session ownership so a failure that lands after the user switched sessions no longer clobbers the other session's draft. - Reset pendingOptimisticMessageIds in resetRuntimeForHostSwitch so stale ids do not leak across a host switch. Adds tests for the session.error handling (drop+recover, and ignore for a different session), the cross-session rollback guard, the host-switch reset, and the parseSessionErrorReason edge cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a bad network, the Android send button appeared completely dead. Two compounding causes:
readTimeout(0)(infinite). A half-open connection could hang for minutes with no error and no UI change.Fix
OpenCodeRepository.kt): REST calls now use a bounded 60s read timeout (15s connect / 60s write) so a stuck request fails fast. The SSE stream keeps an infinite read timeout on its own client — it can legitimately be silent for minutes while the agent runs a long tool call, so it must not inherit the bounded REST timeout.MainViewModel.kt,MainViewModelSessionActions.kt): on send, immediately insert an optimistic user row carrying a deterministicmsg_<uuid>id, clear the composer, and show a spinner on the send button (ChatInputBar.kt/ChatScreen.kt).loadMessages/loadMoreMessagesmerge pending optimistic rows that the server has not echoed yet, and prune ids the server has confirmed. On send failure the row is dropped and the composer text/attachments are restored so the user can retry.PromptRequest.messageID(the opencode server schema supportsmessageID, and the server uses it as the user message id), so reconciliation is pure id membership.Review-driven hardening
Addressed findings from an independent code review:
session.errorSSE event (MainViewModelSyncActions.kt):prompt_asyncacknowledges with 204 before the turn runs, so a failure that happens before the user message is persisted (agent missing, session deleted, crash) only surfaces through this event. It was previously dropped, leaving a permanent ghost optimistic row and silently losing the user's text. Now, for the current session, the pending optimistic rows are dropped, their text is recovered into the composer, and an error is set.parseSessionErrorReasonbuilds a bounded display reason (mirrors the iOS handler).MainViewModelSessionActions.kt): the send-failure composer restore is now guarded by session ownership, so a failure that lands after the user switched sessions no longer clobbers the other session's draft.MainViewModel.kt):resetRuntimeForHostSwitchnow clearspendingOptimisticMessageIdsso stale ids don't leak across a host switch.Tests
sendMessagemocks/verifies to the new 6-arg signature.session.errorhandling: drop + text recovery, and ignore for a different session.pendingOptimisticMessageIds.OptimisticSendTest.ktcovering the pure helpers (makeServerId,buildOptimisticMessage,mergePendingOptimisticMessages,parseSessionErrorReason).testDebugUnitTestsuite is green (327 tests).