feat(ios): durable offline command outbox for chat sends#100331
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b134bfa3e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.outbox != nil, self.attachments.isEmpty, | ||
| !self.hasRestoredOutboxMessages | ||
| || self.outboxStatesByMessageID.values.contains(where: { !$0.isFailed }) |
There was a problem hiding this comment.
Reset the outbox restore gate per session
When a user has already restored one session, hasRestoredOutboxMessages remains true across applySessionSwitch; if they switch to another session that has durable queued commands and submit text before that session's async restore finishes, this condition can bypass the outbox and send the new text live ahead of the older queued row, violating the FIFO guarantee for that session. Reset the restore/mapping state when changing sessions so the gate assumes a backlog until the new session's rows are adopted.
Useful? React with 👍 / 👎.
|
Codex review: stale review; fresh review needed. Summary Next step Review history (3 earlier review cycles)
|
4d38e7f to
7551c63
Compare
7551c63 to
ec1ee2b
Compare
5b134bf to
f0c7687
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0c7687184
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sessionKey: sessionKey, | ||
| transport: self.appModel.makeChatTransport(), | ||
| transcriptCache: offlineStore, | ||
| outbox: offlineStore, |
There was a problem hiding this comment.
Keep the composer enabled for offline outbox sends
When the operator gateway is actually disconnected (gatewayConnected == false), the surrounding ChatProTab still passes that value as isComposerEnabled, and OpenClawChatComposer disables editing and sendDraftIfEnabled before OpenClawChatViewModel.send() can enqueue anything. That means the outbox wired here only helps stale-healthy transport failures; in the documented dead-zone/disconnected case users cannot type or tap Send to create a durable queued row. Enable the text composer when an offline store/outbox exists (while still keeping attachment sends online-only) so disconnected text can reach this outbox path.
Useful? React with 👍 / 👎.
f0c7687 to
36c882d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36c882dda1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private func recoverInterruptedOutboxSendsIfNeeded() async { | ||
| guard let outbox, !self.hasRecoveredInterruptedOutboxSends else { return } | ||
| // Burn the once-per-launch gate only when the store was reachable: | ||
| // with Complete file protection the database is legitimately | ||
| // unavailable while the device is locked, and skipping recovery then | ||
| // would leave crashed 'sending' rows stuck forever after unlock. | ||
| if await outbox.recoverInterruptedSends() { |
There was a problem hiding this comment.
Keep recovery from requeueing active flushes
When the iOS chat model is rebuilt on reconnect, NodeAppModel reuses the same offline store but this recovery gate is per view model, so the new model's restore can run recoverInterruptedSends() while the old model still has a command marked sending and is awaiting sendMessage. That flips an actively in-flight row back to queued, allowing the new model to flush/delete it as a duplicate (for example after an in_flight ack) before the original send returns; if that original send later errors, the durable queued command has been lost. Make interrupted-send recovery store/process-scoped, or otherwise ensure it only runs on true cold-start recovery rather than during same-process view-model replacement.
Useful? React with 👍 / 👎.
36c882d to
1ea0410
Compare
Text messages sent while the gateway is unreachable queue in a durable
per-gateway outbox (new outbox_commands table in the per-gateway chat
cache SQLite store, schema v2) instead of failing. Queued bubbles render
with visible Queued/Sending/Not sent states and flush strictly in
createdAt order once transport health recovers; each command's client
UUID rides as the send idempotency key, so at-least-once delivery plus
gateway dedupe keeps the transcript exact.
Contract summary:
- Bounds: 50 queued commands per gateway; refused enqueues keep the
draft. Queued rows older than 48h expire to failed("expired") rather
than silently sending stale commands; tap-to-retry refreshes
createdAt so an expired row can resend as new intent.
- Failure taxonomy: transport-level failures keep rows queued without
burning retry attempts (backoff ladder, then health drops so the
reconnect machinery owns pacing); gateway rejections burn attempts
and fail terminally after 3, with context-menu retry/delete.
- Deletes are tombstoned synchronously and rechecked after the claim
await so an active flush can never send a removed command; Delete is
hidden while a bubble is already in flight; offline enqueue is
guarded against double submit during the health probe.
- Post-reconnect live sends route behind draining outbox rows (FIFO),
and a cold open assumes a backlog until restore adopts durable rows.
- Crash safety: 'sending' rows revert to 'queued' at startup; flushed
turns are spliced into the session's cached transcript before their
outbox row is deleted, and stale history snapshots cannot evict a
just-flushed turn until a snapshot confirms it.
- Per-gateway scoping and purge ride the transcript cache: one SQLite
file per gateway; reset/forget drops the queue with the cache.
Fixture/unpaired transports get no outbox.
Part of #46664
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ea0410eec
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| nonisolated static let maxOutboxSendAttempts = 3 | ||
| @ObservationIgnored | ||
| var pendingCacheWriteTask: Task<Void, Never>? | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
Resolve conflict markers before merging
This file still contains unresolved merge-conflict markers (<<<<<<<, =======, >>>>>>>) in the committed diff, so Swift cannot parse ChatViewModel.swift and the shared OpenClawKit/iOS targets will fail to build until the conflict is resolved. The same marker pattern appears in multiple touched blocks of this file, so this is not just a stray comment.
Useful? React with 👍 / 👎.
1ea0410 to
6497b8c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6497b8cf8b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard self.isCurrentSession(session) else { return } | ||
| self.presentOutboxCommands(commands.filter { $0.sessionKey == session.key }) | ||
| // The FIFO send gate assumes a backlog until this point. | ||
| self.hasRestoredOutboxMessages = true |
There was a problem hiding this comment.
Keep the restore gate closed when the store is unreadable
When this restore runs while the iOS protected SQLite file is unavailable, OpenClawChatSQLiteTranscriptCache.loadCommands() returns [] because handle() returns nil, but this line still marks hasRestoredOutboxMessages true. If the app/view model was created before protected data became available, the first unlocked send can then take the live path with no adopted outboxStatesByMessageID, bypassing older durable rows that were merely unreadable rather than absent; keep the gate closed unless the outbox read actually succeeded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45d05538d2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.pendingLocalUserEchoMessageIDsByRunID.removeAll() | ||
| self.runMessageScopesByRunID.removeAll() | ||
| self.provisionalFinalMessagesByID.removeAll() | ||
| self.recentlySentOutboxUserKeys.removeAll() |
There was a problem hiding this comment.
Preserve sent-outbox keys across session switches
When an outbox flush succeeds but gateway history is still lagging, recentlySentOutboxUserKeys is the only guard that keeps the just-spliced user turn from being evicted by a stale history snapshot. Clearing the whole set on every session switch means a user who switches away/back (or switches to a background session whose queued command just flushed) before history contains that idempotency key will pre-paint the cached sent turn, then the next stale live history response will replace it and overwrite the cache without the turn, making the acknowledged send disappear until a later fresh history load. Keep this protection scoped per session instead of dropping it during switches.
Useful? React with 👍 / 👎.
…0331) * feat(ios): durable offline command outbox for chat sends Text messages sent while the gateway is unreachable queue in a durable per-gateway outbox (new outbox_commands table in the per-gateway chat cache SQLite store, schema v2) instead of failing. Queued bubbles render with visible Queued/Sending/Not sent states and flush strictly in createdAt order once transport health recovers; each command's client UUID rides as the send idempotency key, so at-least-once delivery plus gateway dedupe keeps the transcript exact. Contract summary: - Bounds: 50 queued commands per gateway; refused enqueues keep the draft. Queued rows older than 48h expire to failed("expired") rather than silently sending stale commands; tap-to-retry refreshes createdAt so an expired row can resend as new intent. - Failure taxonomy: transport-level failures keep rows queued without burning retry attempts (backoff ladder, then health drops so the reconnect machinery owns pacing); gateway rejections burn attempts and fail terminally after 3, with context-menu retry/delete. - Deletes are tombstoned synchronously and rechecked after the claim await so an active flush can never send a removed command; Delete is hidden while a bubble is already in flight; offline enqueue is guarded against double submit during the health probe. - Post-reconnect live sends route behind draining outbox rows (FIFO), and a cold open assumes a backlog until restore adopts durable rows. - Crash safety: 'sending' rows revert to 'queued' at startup; flushed turns are spliced into the session's cached transcript before their outbox row is deleted, and stale history snapshots cannot evict a just-flushed turn until a snapshot confirms it. - Per-gateway scoping and purge ride the transcript cache: one SQLite file per gateway; reset/forget drops the queue with the cache. Fixture/unpaired transports get no outbox. Part of openclaw#46664 * chore(ios): sync native i18n inventory * style(chat-ui): satisfy strict SwiftFormat lint (doc comment, scope blank line)
…0331) * feat(ios): durable offline command outbox for chat sends Text messages sent while the gateway is unreachable queue in a durable per-gateway outbox (new outbox_commands table in the per-gateway chat cache SQLite store, schema v2) instead of failing. Queued bubbles render with visible Queued/Sending/Not sent states and flush strictly in createdAt order once transport health recovers; each command's client UUID rides as the send idempotency key, so at-least-once delivery plus gateway dedupe keeps the transcript exact. Contract summary: - Bounds: 50 queued commands per gateway; refused enqueues keep the draft. Queued rows older than 48h expire to failed("expired") rather than silently sending stale commands; tap-to-retry refreshes createdAt so an expired row can resend as new intent. - Failure taxonomy: transport-level failures keep rows queued without burning retry attempts (backoff ladder, then health drops so the reconnect machinery owns pacing); gateway rejections burn attempts and fail terminally after 3, with context-menu retry/delete. - Deletes are tombstoned synchronously and rechecked after the claim await so an active flush can never send a removed command; Delete is hidden while a bubble is already in flight; offline enqueue is guarded against double submit during the health probe. - Post-reconnect live sends route behind draining outbox rows (FIFO), and a cold open assumes a backlog until restore adopts durable rows. - Crash safety: 'sending' rows revert to 'queued' at startup; flushed turns are spliced into the session's cached transcript before their outbox row is deleted, and stale history snapshots cannot evict a just-flushed turn until a snapshot confirms it. - Per-gateway scoping and purge ride the transcript cache: one SQLite file per gateway; reset/forget drops the queue with the cache. Fixture/unpaired transports get no outbox. Part of openclaw#46664 * chore(ios): sync native i18n inventory * style(chat-ui): satisfy strict SwiftFormat lint (doc comment, scope blank line)
Stacked on the iOS chat offline cache branch (#100219,
feat/ios-chat-offline-cache); lands after it.Part of #46664
What Problem This Solves
Text sent while the gateway is unreachable is refused and lost. Users who think of things for their agent while offline (transit, dead zones, gateway restarts) have no way to hand them off — the thought has to survive in their head until connectivity returns.
Why This Change Was Made
Adds a durable offline command outbox to the shared chat pipeline, wired up for iOS. It extends the existing transcript-cache SQLite database (same DB, new table) rather than adding a store, and uses the existing health transition as the flush trigger rather than a second reachability source. Each queued command's UUID rides as the
chat.sendidempotency key, so flushing is at-least-once with gateway-side dedup. Conservative contract: text-only v1, 50 queued per gateway, 48h expiry to a visible "expired" state instead of silently firing stale commands, 3 attempts for gateway rejections with visible failure + tap-to-retry, transport-level failures never burn attempts (they climb an in-memory backoff ladder, then drop health so the reconnect machinery owns pacing),sendingrows recover toqueuedafter a crash, and rows purge with the cache on gateway resets.Ordering is a first-class guarantee: the outbox is the single ordering authority for a session's turns. New live sends route behind queued/sending rows (including immediately after a cold open, before durable rows are re-adopted), the flush is strictly createdAt-ordered and single-flight, and a just-flushed turn stays visible and cached until a history snapshot confirms its idempotency key — a lagging gateway snapshot cannot evict it. Deliberately session-scoped: other sessions' queued rows are separate conversations with no ordering contract against a new send (documented inline).
User Impact
Evidence
cd apps/shared/OpenClawKit && swift test --filter "ChatViewModelOutboxTests|ChatTranscriptCacheStoreTests"— full outbox suite green (16 VM scenarios + store round-trip/claim tests), including deterministic tests for: enqueue-offline persistence across VM recreation, ordered flush with idempotency keys, ack-removes-row without duplicate bubbles, rejection→failed-after-3 with tap retry, transport-failure backoff ladder then health drop, 48h expiry boundary, MAX_QUEUED refusal + draft preservation, crash recovery, delete-vs-flush races (snapshot window AND claim-await window via a held-delete/claim-hook harness), double-submit during the health probe, post-reconnect live sends queueing behind draining rows, cold-open sends queueing behind not-yet-adopted durable rows, and stale-history-after-ack cache preservation.swift test --filter "ChatViewModelTests|ChatViewModelTranscriptCacheTests"— 111+ existing tests green.Follow-ups noted: voice-capture queue unification (
WatchReplyCoordinatoroverlap documented in code), macOS wiring rides the stacked macOS cache PR, and attachment sends remain online-only v1 (outside the ordering guarantee, documented inline).Update (post-#100219 landing): rebuilt as a single squashed commit on main, re-expressing the outbox on the landed cache rewrite (per-gateway SHA256-named DB files under
chat-cache/, retired/async-handle store API, main's onboarding purge redesign). All seven review-hardened behaviors preserved with their regression tests, plus four more fixes from this pass's review rounds: session-switch FIFO-gate reset, model-patch ordering during flush, delete-durability window (bubble persists until the durable row is confirmed gone — no resurrect-after-kill), and crash-recovery gating under a locked store. Validation:swift testacross the five affected suites — 150 tests green (three consecutive runs); final Codex autoreview clean (0.84).