docs: Signal-cache flush scheduling and flush_pending_signal_state (PR #1022) - #402
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
| Filename | Overview |
|---|---|
| advanced/signal-protocol.mdx | Explains send and receive flush scheduling, recovery behavior, generation scoping, and explicit durability settling. |
| api/client.mdx | Documents flush_pending_signal_state(), its result semantics, usage, and narrowed deadlock warning. |
| api/send.mdx | Documents pre-wire outbound persistence and the related SendError::Internal behavior. |
Reviews (7): Last reviewed commit: "docs: narrow flush_pending_signal_state ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@advanced/signal-protocol.mdx`:
- Around line 1138-1148: Rewrite the documentation section around the Signal
cache flush behavior using concise sentences with one idea per sentence, active
voice, and direct second-person phrasing. Split long sentences and replace
passive constructions such as retry descriptions with explicit actors, while
preserving the existing send/receive durability rules, scheduler generation
behavior, synchronous recovery paths, and flush_pending_signal_state() usage
restrictions.
In `@api/client.mdx`:
- Around line 1458-1470: Update the `flush_pending_signal_state` documentation
to format the heading as a code reference and rewrite the prose using concise,
second-person sentences. Split compound sentences into single ideas while
preserving the existing behavior, timing caveats, failure handling, and warning
about calling it only from a control task.
In `@api/send.mdx`:
- Line 1259: Rewrite the Internal error description in the send_message
documentation as separate concise sentences, keeping the catch-all meaning and
the note about failures to durably persist the outbound Signal ratchet advance
before stanza transmission.
- Around line 46-48: Rewrite the Note in api/send.mdx using concise,
active-voice sentences: state that the system synchronously persists the
outbound Signal ratchet advance before transmitting DMs, group, or status
stanzas; explain that this prevents outbound counter, message-key, and IV reuse;
state that send_message returns Err when persistence fails; and retain the
existing flush-scheduling reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 70509f4f-5ba8-45e7-b2f1-e6e4e8ef8ab0
📒 Files selected for processing (3)
advanced/signal-protocol.mdxapi/client.mdxapi/send.mdx
| <Note> | ||
| For DMs, group, and status sends, the outbound Signal ratchet advance is persisted to the backend **synchronously, before the stanza is transmitted** — reusing an outbound counter would reuse its message key and IV, so the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that couldn't be saved. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model. | ||
| </Note> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use concise sentences and active voice.
The note merges multiple ideas into a single long sentence and uses passive voice ("is persisted"). As per coding guidelines, documentation must use concise sentences (one idea per sentence) and active voice.
📝 Proposed rewrite
<Note>
-For DMs, group, and status sends, the outbound Signal ratchet advance is persisted to the backend **synchronously, before the stanza is transmitted** — reusing an outbound counter would reuse its message key and IV, so the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that couldn't be saved. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model.
+For DMs, group, and status sends, the client persists the outbound Signal ratchet advance to the backend **synchronously, before the stanza is transmitted**. Reusing an outbound counter would reuse its message key and IV. Therefore, the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that it could not save. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model.
</Note>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Note> | |
| For DMs, group, and status sends, the outbound Signal ratchet advance is persisted to the backend **synchronously, before the stanza is transmitted** — reusing an outbound counter would reuse its message key and IV, so the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that couldn't be saved. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model. | |
| </Note> | |
| <Note> | |
| For DMs, group, and status sends, the client persists the outbound Signal ratchet advance to the backend **synchronously, before the stanza is transmitted**. Reusing an outbound counter would reuse its message key and IV. Therefore, the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that it could not save. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model. | |
| </Note> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/send.mdx` around lines 46 - 48, Rewrite the Note in api/send.mdx using
concise, active-voice sentences: state that the system synchronously persists
the outbound Signal ratchet advance before transmitting DMs, group, or status
stanzas; explain that this prevents outbound counter, message-key, and IV reuse;
state that send_message returns Err when persistence fails; and retain the
existing flush-scheduling reference.
Source: Coding guidelines
| - `InvalidRequest` — the send request was malformed (e.g., invalid JID, bad message shape) | ||
| - `Client` — underlying transport/connection error | ||
| - `Internal` — catch-all for errors not yet assigned a typed variant | ||
| - `Internal` — catch-all for errors not yet assigned a typed variant. Includes a failure to durably persist the outbound Signal ratchet advance before the stanza was sent — see the durability note under [`send_message`](#send_message). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use concise sentences.
The description merges two separate directives with an em dash. As per coding guidelines, documentation must use concise sentences with one idea per sentence.
📝 Proposed rewrite
-- `Internal` — catch-all for errors not yet assigned a typed variant. Includes a failure to durably persist the outbound Signal ratchet advance before the stanza was sent — see the durability note under [`send_message`](`#send_message`).
+- `Internal` — catch-all for errors not yet assigned a typed variant. It includes a failure to durably persist the outbound Signal ratchet advance before the stanza was sent. See the durability note under [`send_message`](`#send_message`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `Internal` — catch-all for errors not yet assigned a typed variant. Includes a failure to durably persist the outbound Signal ratchet advance before the stanza was sent — see the durability note under [`send_message`](#send_message). | |
| - `Internal` — catch-all for errors not yet assigned a typed variant. It includes a failure to durably persist the outbound Signal ratchet advance before the stanza was sent. See the durability note under [`send_message`](`#send_message`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/send.mdx` at line 1259, Rewrite the Internal error description in the
send_message documentation as separate concise sentences, keeping the catch-all
meaning and the note about failures to durably persist the outbound Signal
ratchet advance before stanza transmission.
Source: Coding guidelines
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Auto-approved: Documentation-only PR describing existing durability model and new API method; no behavioral changes or tradeoffs to evaluate.
Re-trigger cubic
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Auto-approved: This PR updates three documentation files with notes on Signal cache flush scheduling and a new API method. The diff is purely informational; no behavior, configuration, or code is changed. The documentation clarifies existing implementation details and is clearly beneficial for users.
Re-trigger cubic
2 participants
Summary
Documents the durability-model changes from whatsapp-rust#1022 ("perf(signal): coalesce receive flushes and persist outbound state pre-wire"), which changed when the in-memory Signal cache reaches persistent storage:
Errif that persistence write fails (previously this was logged only, not propagated).Client::flush_pending_signal_state(), lets callers force a deterministic durability settle (e.g. before reading persisted state directly, or ahead of a non-graceful shutdown), with a documented deadlock precondition (never call from inside an event handler orInboundDurabilityHook).The offline drain, retry-receipt recovery, identity-change recovery, and teardown all keep their own synchronous flushes and are unaffected — confirmed by inspecting the diff, which does not touch
commit_batch.rs(the file governingInboundDurabilityHookbatch-commit ordering).advanced/inbound-durability.mdxwas therefore left unchanged.Changes
api/client.mdx— adds aflush_pending_signal_statereference entry under Protocol Operations (signature, durability semantics, deadlock warning, example).advanced/signal-protocol.mdx— adds a new "Flush scheduling: send vs. receive" subsection under Storage Integration explaining the synchronous-send / coalesced-receive durability model and the generation-scoped scheduler.api/send.mdx— adds a note undersend_messagethat outbound Signal state is now persisted synchronously pre-wire and that persistence failures now surface asSendError::Internal, plus updates theSendError::Internalvariant description.No changelog entries were added or modified per standing instructions (changelog is human-maintained only).
Test plan
/api/client#flush_pending_signal_state,/advanced/signal-protocol#flush-scheduling-send-vs-receiveapi/send.mdxandapi/client.mdxto the new signal-protocol section render correctly🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Documents the updated Signal cache durability model and the new
Client::flush_pending_signal_state()API. Sends persist ratchet advances synchronously and abort on failure; live receives coalesce with retries and generation scoping; the new API forces a deterministic settle and may wait on locks or storage.advanced/signal-protocol.mdx: adds “Flush scheduling: send vs. receive” with coalesced receive flushes, retry behavior, and generation‑scoped scheduler; clarifies unaffected offline drain/recovery paths and reconnect behavior; links to the new API.api/client.mdx: addsflush_pending_signal_state()reference with example; clarifies success guarantee vs. no hard time bound; narrows the deadlock warning to permit‑holding callers (InboundDurabilityHookand synchronous inlineEventHandler), explicitly noting ordinaryBotclosures are safe.api/send.mdx: notes synchronous pre‑wire durability for sends and that persistence failures surface asSendError::Internal; restores the missing “Error Types” heading and expands theInternalvariant description.Written for commit c0f54e0. Summary will update on new commits.
Summary by CodeRabbit