fix(ios): persist queued Watch replies#100372
Conversation
Route Apple Watch quick replies through the durable, gateway-scoped Watch message outbox with restart-safe routing and deduplication. Co-authored-by: NianJiuZst <[email protected]>
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 12:10 PM ET / 16:10 UTC. Summary PR surface: Docs +1, Other +436. Total +437 across 11 files. Reproducibility: yes. from source inspection. Current main accepts Watch quick replies into process-local arrays, so queued replies and dedupe state can be lost across suspension or termination; I did not run a physical Watch reproduction. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep the gateway-scoped durable Watch outbox approach, remove the direct changelog edit, finish exact-head validation, and require real Watch/WatchConnectivity proof or an explicit maintainer proof override before merge. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection. Current main accepts Watch quick replies into process-local arrays, so queued replies and dedupe state can be lost across suspension or termination; I did not run a physical Watch reproduction. Is this the best way to solve the issue? Mostly yes. Reusing one durable gateway-scoped Watch outbox is the right fix boundary, but the merge artifact still needs real behavior proof or an explicit override plus changelog cleanup. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 286d0b9fb340. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Docs +1, Other +436. Total +437 across 11 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
Bind shipped notification prompts without routing metadata to the current Gateway once, while keeping explicit and persisted Gateway owners fail-closed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 725615af2b
ℹ️ 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".
| - **Model aliases:** resolve provider-qualified aliases during session and chat-command model switches without collisions when providers share a display alias. (#100209) Thanks @sahilsatralkar. | ||
| - **TUI new-session hooks:** create `/new` sessions through the shared Gateway lifecycle so command and session hooks receive the completed parent transcript in both Gateway and embedded modes, while preventing rollover during an active turn. (#100241, #49918) Thanks @BingqingLyu. | ||
| - **TUI abort diagnostics:** show sanitized tool argument-validation summaries for aborted runs in both Gateway and local TUI modes without exposing raw model arguments. (#91002) Thanks @wsyjh8. | ||
| - **iOS Watch replies:** persist queued quick replies in the gateway-scoped chat outbox and submit them through idempotent chat delivery, preventing losses, duplicates, and cross-gateway sends after reconnects. (#100031) Thanks @NianJiuZst. |
There was a problem hiding this comment.
Remove the direct changelog entry
For normal fixes, the root AGENTS.md says CHANGELOG.md is release-only and generated from merged PRs/direct commits, so adding this line directly can leave the next release-generation pass with duplicate or stale release notes. Unless this commit is the release-generation commit, keep this context in the commit/PR body and let the changelog generator create the entry.
Useful? React with 👍 / 👎.
|
Land-ready review complete for What changed:
Proof:
Known gap: no physical Apple Watch round-trip was available. Native build and test lanes cover the changed Apple surfaces. |
|
Merged via squash.
|
* fix(ios): persist queued watch replies Route Apple Watch quick replies through the durable, gateway-scoped Watch message outbox with restart-safe routing and deduplication. Co-authored-by: NianJiuZst <[email protected]> * fix(ios): preserve pre-upgrade Watch replies Bind shipped notification prompts without routing metadata to the current Gateway once, while keeping explicit and persisted Gateway owners fail-closed. --------- Co-authored-by: NianJiuZst <[email protected]>
What Problem This Solves
Apple Watch quick replies were accepted in memory and could be lost when the iOS app was offline, suspended, or terminated before Gateway delivery. Delayed WatchConnectivity fallbacks could also duplicate delivery or route a reply after the active Gateway changed.
This supersedes #100031 after GitHub auto-closed the contributor branch during a maintainer ancestry repair. The implementation and contributor attribution are preserved here so the reviewed fix can land cleanly.
Why This Change Was Made
chat.sendidempotency keys.User Impact
Watch quick replies survive iPhone process loss and reconnects, avoid duplicate sends, and never guess across Gateways. Existing
watch.chat.command.queue.v1data remains decodable.Evidence
xcrun swiftc -parseon all changed iOS/watchOS Swift filesswiftformat --lint --config config/swiftformaton changed app sourcesnode --import tsx scripts/native-app-i18n.ts checkgit diff --checkios-build,macos-swift,macos-node, native i18n, and Node shardsOPENCLAW_TESTBOX=1 scripts/pr prepare-run 100372: passed exact-head hosted gatesKnown gap: no physical Apple Watch round-trip was available. Native build/test lanes cover the changed Apple surfaces.
Attribution
Original implementation by @NianJiuZst in #100031; replacement commit retains
Co-authored-bycredit.