Repository navigation
fix: harden remote control and polish image and companion actions - #3896
Merged
Merged
Conversation
Add a provider-neutral delivery helper: replies are chunked, each chunk is retried with bounded backoff that honours retry_after, and a final failure reports the unsent remainder so callers can resume from the failed chunk. Telegram, Feishu (now chunked like the others) and WeChat text sends use it. Telegram getUpdates and send responses with ok:false become classified errors (401, 409, 429 with retry_after) instead of an empty batch. Feishu gains a bounded, TTL'd event-id dedup set and read-idle tracking on the WebSocket. Bot strings for the new notices are added in all bot languages.
Local bot turns now watch the tracker snapshot as the account-routed path polls the device: a tool waiting for approval reaches the chat with the same approve/deny prompt (answered through the host interaction owner), and a still-blocked question or approval is re-presented every four minutes, re-sent when its earlier delivery failed. A host interaction no longer expires with the menu TTL, and a settled local turn retires its own prompts. The one-hour watcher cap is gone on both paths: after an hour the chat is told the turn is still running and the result is delivered when it settles; a local watcher also detects a settlement whose event was lost. A remote host that fails three consecutive polls produces one offline notice, polls back off to 30s, and recovery is announced.
…tal polls A final reply whose parts still fail after the provider retries is held in memory (bounded, 24h) and resent with the chat's next message; the chat is told it was not delivered and that the answer is in the session history (/resume). Interaction prompts that fail to send are marked for re-send. Telegram polling now backs off on errors (honouring retry_after) instead of re-polling immediately. A revoked token (401) or a persistent 409 conflict stops the listener with one clear error log; on a conflict the paired chat is told another consumer owns the token.
… sockets Events are dispatched once per event id (message id for older envelopes) through a bounded, TTL'd in-memory set, so an event Feishu redelivers after a lost ack does not run a command or turn twice. A failed ack now reconnects instead of being ignored. Each ping tick checks the socket: nothing received for two ping intervals, a failed ping, or a ping write over 10s replaces the connection, so a socket left half-open by host sleep no longer stays silent. Undelivered final replies are held and resent like Telegram's.
…e the cursor An expired iLink session (-14) now ends polling with one error log instead of looping silently; the existing login_required bot status reports it until the user signs in again, and the proactive worker pauses meanwhile. The getupdates cursor is persisted only after the whole batch was admitted to the per-peer workers (or the pairing message handled), so a stop or crash mid-batch replays messages instead of skipping them. Undelivered final replies of the bot's own turns are held and resent with the peer's next message, which also brings a fresh context_token.
An unknown capability snapshot made sendMessage fall through to the legacy non-idempotent send_message. The send now probes first and fails before submitting when the probe fails; the composer shows a host check and blocks Send while the probe is in flight.
… re-probe capabilities The host stream reader ignores wakes while its retry timer is pending and backs off to 30 s, and an unsupported host was final. The session synchronizer now restarts a stalled or waiting reader at once on Relay reconnects, page resume, browser online, host-online transitions and hints for the session, shows a catching-up notice until the next catch-up, and re-probes capabilities after a host restart, a host coming back online, or before repeating an unsupported answer.
… error A failed catalog subscribe was never retried, so the session list stayed stale until a foreground refresh. It now retries with backoff, retries at once on Relay reconnects, resume, browser online and host-online changes, replaces a reader whose reads keep failing, and clears the banner it set once the catalog catches up.
Chat actions and the host queue showed raw transport text such as 'Error: Relay returned HTTP 503'. Failures now go through the shared RelayFailure classifier plus explicit not-submitted, desktop-unavailable and unknown-outcome cases; a host's own answer keeps its text. Stream read failures use the same copy inside the catching-up notice, which clears once the reader catches up.
…ering retry A failed question answer had no catch and gave no feedback, Approve/Deny with a lost reply reported failure and re-enabled the buttons, and a question was rendered both by the interaction mailbox and the transcript. An unknown outcome now shows a check, re-reads the host (stream and mailbox), and only offers the actions again, marked as not confirmed, while the request is still pending; definite failures are stated inline in translated copy. The mailbox owns the questions it lists and the transcript defers to it.
Every failure showed one 'Reconnecting' banner offering 'Pair again' after a single missed ping. Connection health now distinguishes an offline browser, a reconnecting Relay, a desktop the Relay lists as offline (with its last seen time) and a listed desktop that does not answer; it needs two missed pings before blaming the desktop, pings at once after reconnects, resume, network return and the desktop coming back, and offers 'Pair again' only for the unanswered-desktop case.
Image sends were one large POST with no retry, and the composer showed the same wait for the upload and for the desktop. Bulk RPC parameters are now uploaded by mobile-web with a bounded retry of the same bytes; the RPC carries the reference, which a call that provably never left this client reuses once instead of uploading again. The composer shows 'Uploading images' while the upload runs and 'Waiting for the desktop' afterwards.
…pload surface ChatPage now reads capability probe state and payload upload activity from the session manager; the layout fixture's stand-in manager provides both.
Without the one-hour cap, a watcher for a turn whose session was deleted or moved off this host probed it every 30 seconds forever. Two consecutive probes that cannot serve the session now end the watcher as cancelled.
…to notFound A slow or offline target held the single observer lock, so every other job and every explicit refresh waited behind several SSH or device timeouts. Status reads now run in one serial lane per target; index reconciliation stays single-flight, explicit refreshes go straight to their lane, and only a failed status read backs a target off. A target that answers without the job is recorded as notFound (typed marker or the legacy text) instead of unreachable. The store keeps transient last-reachable, answered-prompt and stop state.
…t health Pending prompts restored from disk or kept while the target is unreachable are no longer actionable until a status read confirms them, and an answered prompt leaves the list at once (restored if delivery fails). Navigation ranks an unreachable target above approval. Stop requests retry transport failures and record stopping or unconfirmed. The dispatch target chip shows unreachable with the last update time, not delivered, stopping, stop not confirmed with a retry, and prompts awaiting the target.
is_connected only read a liveness flag that russh clears after up to six 30-second keepalives, so the status stayed green for minutes after a sleep or network change. When the last confirmation is older than 20 seconds (wall clock, so a sleep counts) or an operation timed out, is_connected now opens and closes one channel with a 5-second bound; a silent transport is marked disconnected so the next operation reconnects, while a refused channel still counts as an answer. The provider re-verifies immediately on visibility, online and operation-timeout events and exposes the connections being re-verified.
The target CLI now reports a missing job directory or commit marker with a [dispatch_job_not_found] marker followed by the legacy text, so older controllers keep matching it. The controller maps both the marker and the legacy text from older targets (SSH and device) to DispatchJobNotFoundOnTarget, whose message starts with the marker; other failures pass through unchanged.
A controller catching up after a long disconnect read the surviving suffix when the host had already evicted records past its cursor, silently skipping them. A forward read below the highest evicted live sequence now gives the stream a fresh epoch, like a slow backward reader crossing evicted history, so the subscriber announces a gap and reloads the latest page. The Rust subscriber also treats a truncated page whose oldest sequence is past its cursor as a restart, covering hosts that predate the fence.
The 15 s first-page timeout started only after subscribeSession resolved, so a host that never answered the subscribe request parked the session in hydration forever. The deadline now covers the subscribe request; on expiry the open fails with the existing retryable error and a late grant is unsubscribed when it arrives.
…eleting them A peer send whose start reply was lost (outcome_unknown) deleted the optimistic turn, reset the session, toasted the raw error and restored the composer, so a resend ran the prompt twice when the host had already accepted it. The turn now stays with a "Confirming with device" status while the session is re-read from the host. A turn the host has is kept; otherwise (or when the read-back fails) it is removed and the message is retained in the pending queue as failed under the same turn id, so an explicit retry is deduplicated by the host. Copy is translated. A dispatch first submit that fails after the request was sent keeps the optimistic job turn as "Checking whether the target received this" and lets the dispatch observer settle it from the job state. A job still submitting after a short grace failed before anything was sent; a typed not-delivered state from a newer observer is handled the same way.
…as lost A permission or AskUserQuestion answer whose reply was lost, or that the host refused because the interaction had already settled, showed a failure and every retry failed again. Such failures now show a confirming state and re-read the host: pending permissions for the answered requests, or the session interaction mailbox for a question. A settled interaction leaves its card (or shows the submitted answer); only one the host still waits for is reported as failed.
A failed live read keeps the rendered records, so a device session looked current while nothing new arrived. When the session history is failed but content is shown on a device surface, an inline notice says the controller is reconnecting to that device and updates are paused, with a Retry that re-reads the session from the host. The next ready event clears it.
…rns that ran - Unacknowledged outbox submits never resolved on their own; only accepted records were reconciled. With the new opt-in resolveUnconfirmed option a refresh asks the owner of the same epoch for each pending submit's receipt: a receipt marks it accepted, none records a definite notSubmitted outcome, so the UI can offer "Not sent - Retry" instead of "unconfirmed". A retry clears the flag and still reuses the Turn id. - After a host restart, accepted records of messages that had already run became "delivery unknown". An injected hasTurn lookup now drops records whose Turn exists in the transcript before anything is marked unaccepted. Both are optional constructor options; the class API and stored record shape stay backward compatible (notSubmitted is additive), and mobile web keeps its current behaviour. The web UI client opts in.
…overy The host queue panel showed raw error text (such as too_late: or queue_scope_expired:) and kept a failed operation's error after the queue had recovered. Known codes now map to translated copy with a generic fallback, the panel's own failures keep their copy, and a failure is retired once a newer snapshot arrives. Records the host confirmed it never received are labelled "Not sent" instead of "unconfirmed".
A rollback whose reply was lost (outcome_unknown) surfaced as a raw failure and was never reconciled, so the controller kept showing a history the host may already have rewritten. The session is now marked as requiring reconciliation, re-read from its device (the host replica on a device surface) and reloaded, and the user is told the result is unknown and the session was reloaded from the device. An edit-and-rerun stops there instead of submitting against an unknown history; if the reload fails the session stays blocked until it is reopened.
…ain failure Only a lost reply or a transport failure on the way to the target leaves the dispatch first submit uncertain. A refusal before sending (validation, preflight) keeps its own error, removes the optimistic turn and returns the draft to the composer as before.
…er's not-found fact The dispatch observer reports a job the target never received as notFound reachability while the job state stays submitting/submission_unknown. The uncertain-submit confirmation only read the job state, so such a turn kept showing "Checking..." forever. Read the delivery status first.
…to the session job The dispatch driver now reports a stop the target never confirmed in translated copy; cancelSessionTask only logged it, so the user could not tell whether the job stopped. The composer also passes its own job id to the target picker so the transport chip describes that job.
A relay blip, routing reconnect or route stop on a Desktop host used to cancel every permission request projected to peer controllers once the last delivery target was cleared. Presence loss is not a controller decision: route loss now only clears delivery targets, the Runtime keeps the request, and a re-attaching controller restores it from the interaction mailbox. Only an explicit final peer_control_detach still fails the tracked requests closed, matching the CLI host.
The lightbox copy button and context menu read their labels and toasts
through i18nService.t during render. Use useI18n('components') instead,
and the design-system duplicate and check-line icons in place of the
lucide Copy and Check glyphs. Tests that open an image lightbox under an
i18n mock now provide useI18n as well.
Markdown can show remote http(s) images. Copying one needs a CORS read that most hosts refuse, and the user only saw a generic failure. - Offer the copy button and context menu only when the page can read the pixels back: data, blob, same-origin and Tauri asset URLs. Origins are compared by URL instead of a string prefix. - copyImageToClipboard rejects a cross-origin source up front with CrossOriginImageError, and the toast names that reason.
The companion context menu hard-coded 24px for the menu row and the small control height, overriding every density. Set both to the menu's own compact row token (overlay menu heading height) and only under compact density, so other densities keep their row and hit-target heights.
…tion The lightbox copy menu pulls in modules that read more of deviceSurface than the test mocked, so the whole file failed to load since the copy action landed. Keep the real module and override only the surface scope, and let the i18n mock answer label lookups that pass no values.
Bounding every peer read to 25 s attempts inside a 60 s deadline made slow but legitimate reads (model catalogs, usage reports, statistics, MCP resources, skill market, SSH directories, large trees) always fail as "host not responding". Reads now keep the shared 120 s attempt by default. Only an explicit hot-path allowlist of cheap session-list, workspace and config hydrate reads keeps the 25 s attempt that also ends at the 60 s deadline. A silent host still fails with the typed peer_host_not_responding error after one attempt, and unsent or reply-lost retries stay inside the 60 s window.
…oller A route loss clears controller delivery targets but keeps pending permission requests for a re-attaching controller. A later explicit peer_control_detach from that controller found nothing to remove and never drained them, so an unattended turn waited forever. Any detach that leaves no controller attached now fails tracked requests closed, whether or not the caller was still in the set.
The 15 s presence grace was measured from the last roster refresh that listed the device online. A roster that stayed unchanged for longer had already used up the grace, so a device that briefly left presence lost its sidebar section at once. The grace now starts when the device is first seen offline and restarts only after it comes back online.
When peer_control_detach failed on transport during a disconnect, the controller logged it and never tried again, so the host kept the controller attached and its pending approvals waiting. The detach is now retried in the background with capped backoff, at once when the host is seen online again, and gives up after a bounded number of attempts. A reconnect to the same device retires the retry and orders its attach after any detach already in flight. The disconnect itself still completes without waiting for these retries.
A webview reload discards the HostInvoke bridge listener without running its cleanup, so the readiness flag stayed true and controller requests were emitted into a page with no listener until they timed out. The existing page-load hook now clears readiness when the main webview starts loading; the new page reports ready again once its bridge is registered.
An older controller treats peer_host_ui_not_ready as a final error, so a request that reached a new Desktop host while its webview was starting or reloading failed outright instead of running moments later. Dispatch now waits up to 10 s for the HostInvoke bridge to register before refusing. Newer controllers, which retry the refusal, simply get their answer sooner.
Both Peer hosts cached every settled start_dialog_turn outcome for 15 minutes, failures included, so an explicit Retry after a transient failure (an unavailable model, a lost event stream) replayed the old error instead of running again. The Desktop bridge cache and the CLI host now coalesce retries of an attempt still in flight as before, but keep a settled outcome only when the submission was accepted. The CLI host also keeps a failure that happened after the Runtime admitted the Turn, because a retry could only meet the duplicate Turn id refusal.
…etry Update the transport invariants for the hot-path read allowlist, the Desktop host's short readiness wait, accepted-only submission replay and background detach retries.
…s with the host behavior
A drained queue item whose start reply was lost keeps its FIFO position and is removed once the host confirms the Turn, instead of a failed copy being appended behind later messages. A retry the host refuses as an already admitted Turn id is treated as delivered, and a confirmed draft first message materializes the draft so the next send does not reuse its Turn id.
Records the owner already lists are accepted without a get, receipt reads for records without a receipt back off, and submit/retry no longer wait behind the receipt reads of other outbox records.
account_subscribe_session can use the whole relay RPC budget on large sessions or slow relays. The 15 s deadline now starts after the grant, and the grant has its own ceiling, so a slow grant is adopted instead of being discarded on every retry.
A re-read keeps the replica, so turns a rollback removed on the host stayed visible. The unknown-outcome path now starts a full relay resync (fresh replica, pruned at ready) before reopening, and a reader retired by a re-read no longer marks its replacement's history as failed.
A refusal of an already settled permission request or question no longer counts as an answer: the rejection reason stays in the composer and the question card says it was already resolved instead of showing it submitted. Lost replies keep their existing unknown-outcome handling.
Every poll publishes a new snapshot object, so the panel cleared its error on the next poll. It now retires an error only after the queue epoch or revision changes.
Stream pages now carry the host's eviction watermark. A fencing host reports exactly what it evicted, so the subscriber no longer infers eviction from the sticky truncated flag and an oldest sequence past its cursor, which resynced needlessly after control-event eviction. Hosts without the field keep the existing fallback.
A live read that fails and recovers within a few seconds no longer flashes the notice or shifts the transcript; it appears once the failure lasts four seconds.
…oval fails When a local approval answer was refused because the host no longer waited on it, the next queued prompt was installed but the chat only received the failure text. A queued prompt is never sent when it is queued, so the user saw no buttons for it. Render the next prompt's menu with the failure as its lead, the same way a successful answer advances the queue.
…l block it The remote watcher reset its poll version to 0 on every poll once any prompt was older than the re-presentation window. An answered prompt kept its old timestamp, so for the rest of an uncapped turn each 2 s poll re-downloaded the full session history. Track presented tools separately from the ones still blocking the turn: only a still-blocked, due prompt forces a full read and is re-presented, while every presented id is still retired when the turn ends.
…s early Host interactions no longer expire with the menu TTL; the turn watcher retires them when it ends. A remote watch that ended on a refused poll, an expired token or an HTTP 403 returned no tool ids, so its approvals and questions stayed pending forever and captured the chat's next messages. Return every presented tool id on those exits as well.
Remote turns have no wall-clock cap, so a flaky link could post an offline and a reconnected notice for every outage of one long turn. Tell the chat about the first outage and its recovery only; later outages are still logged, and poll backoff still restarts with each outage.
A final reply the provider refused permanently (content rejected, HTTP 403) was held and resent with every later message, failing the same way each time, while restoring it refreshed its hold time and skipped the byte bound. UndeliveredText now carries whether a later attempt may succeed. Only such failures are held; unclassified errors such as a stale WeChat context token still are. A permanently refused reply is dropped with a localized notice, and restored replies keep their original hold time and the per-chat byte budget.
When two reads showed the session could no longer be served by this host, the local watcher ended with "Task cancelled" although nothing cancelled the turn. End it with a neutral, localized notice that the session is no longer available on this host.
…sage Feishu, Telegram and WeChat resent held replies inline before handling the message that triggered the resend, so under provider 429 backoff a button tap could wait minutes. Run the resend in a background task fenced to the bot identity, and let only one resend per chat run at a time so held parts keep their order.
… run A bot message dropped from the session queue (for example after a failed desktop turn cleared it) left its local watcher waiting forever: it read the persisted history every 30 s and after an hour claimed the task was still running. Skip the read while another turn visibly runs ahead, and end the watch with a localized notice once two reads confirm the turn is neither persisted as settled nor queued or running in the scheduler. The still-running notice now requires the turn to have been seen running.
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.
Summary
Robustness of every remote-control surface under latency, packet loss, disconnect and reconnect, with matching UI states. Follows #3893. A read-only audit of five areas found 44 issues; each was re-verified in code before it was fixed. The commits are grouped by area.
Type and Areas
Type: bug fix / UX
Areas: Rust core (IM bots, dispatch), services (IM bot transports, host stream, SSH), desktop and CLI peer hosts, web UI (FlowChat, dispatch, SSH, Peer Device), mobile web, shared dialog queue
Motivation / Impact
Image previews and companion actions
IM bots (Telegram / Feishu / WeChat)
retry_afterhonoured) and resume from the failed chunk. A reply that still cannot be delivered is held and re-sent with the next message.ok:falsepolls back off; 401/409 stop the listener.Dispatch / SSH
Peer chat data plane
Peer Device transport
Mobile web
Upgrade compatibility: all wire additions are optional and capability-gated. Older hosts and controllers keep their previous paths (covered by tests for both directions).
Review follow-ups (second pass)
A second review of this PR across six areas found 37 regressions or gaps. Each was re-verified in code, then fixed with tests:
dispatch_submithas no client deadline, and a client timeout never settles a submit as not delivered; a stop retry stops once a follow-up is accepted; "connecting" shows only for re-checks longer than 500 ms.020eb1eae, carried in this branch):useI18n.Second-pass verification: all 21 Frontend Build CI steps pass locally. After forcing a rebuild of every changed Rust file: core bot 125, dispatch 58, host_queue 23; services remote_connect 270, SSH 101; runtime 12; cli peer_host 131, dispatch store 34; desktop peer_host_invoke 28, remote_connect_api 22, clipboard 17.
rustfmt --checkis clean on every changed Rust file.Verification
CI follow-ups: restored the SessionDriver import boundary, fixed declaration ordering, registered recovery Appearance contracts, and updated mobile fixture subscriptions. The focused recovery/Appearance suites passed 197 tests; mobile acknowledgement/target-generation fixtures passed 9 tests.
Additional image/companion changes: 91 focused tests, Web TypeScript/lint, i18n, Appearance/theme audits and generated MiniApp style drift passed. Design-system build and public UI tests passed. Its aggregate check still finds two existing tool-catalog mismatches (GenerativeUI and GenOfficeFile); the catalog inputs are unchanged from upstream main. The local
check:webaggregate is also blocked by an existing Dialog typography token violation. Native clipboard interaction has not been exercised.web-ui: the 546 test files under flow_chat, features/dispatch, features/ssh-remote, infrastructure/peer-device, the peer adapter and NavPanel — 6359 passed;
pnpm run type-check:webpassedmobile-web: type-check, test:host-stream 39, test:host-queue 24, test:ui-components 19, test:interaction-mailbox 3, test:images 10 (the area branch also ran the browser suites)
Gates:
i18n:audit(0 warnings),check:repo-hygiene,capabilities:check,check:core-boundaries,theme:color-audit:all,git diff --checkRust:
openbitfun-product-domains: 70 passed, 1 failed (generative_ui::tests::collection_schema_restricts_generation_but_keeps_saved_apps_readable). It fails identically on a clean upstream main; it is unrelated to this PR.Remote scenarios: exercised only with simulated fixtures (fake relay / hosts / IM APIs, loopback SSH). Not run against live Relay, Peer Device, IM platforms or Detached Dispatch targets.
Reviewer Notes
AI-assisted, lightly tested (focused unit/integration fixtures, no live remote run). Behaviour changes worth noting:
RpcPayload.ts; the two must stay in sync.peer_host_bridge_ready) is added to the product operation registry.Checklist