Skip to content

Only update attachSerial for non-resumed attaches (RTL15c)#2276

Open
SimonWoolf wants to merge 1 commit into
mainfrom
only-update-attachserial-for-non-resumed-attaches
Open

Only update attachSerial for non-resumed attaches (RTL15c)#2276
SimonWoolf wants to merge 1 commit into
mainfrom
only-update-attachserial-for-non-resumed-attaches

Conversation

@SimonWoolf

@SimonWoolf SimonWoolf commented Jul 24, 2026

Copy link
Copy Markdown
Member

Implements ably/specification#503 (spec 6.1.1).

attachSerial is used as the fromSerial bound for untilAttach history, which returns history contiguous with the realtime message stream. A resumed ATTACHED (resumed=true) indicates a brief disconnection that did not break message continuity, so it must not move the point up to which untilAttach history is contiguous.

Previously the SDK updated attachSerial on every ATTACHED regardless of the RESUMED flag; now it only does so when resumed is false (RTL15a → RTL15c).

Changes

  • realtimechannel.ts: guard the attachSerial update on !resumed in the ATTACHED handler. channelSerial still updates unconditionally per RTL15b.
  • UTS ports (test/uts/.../channel_properties.test.ts): renamed the two RTL15a tests to RTL15c, and added a new test asserting attachSerial is left unchanged by a resumed ATTACHED (while channelSerial still updates, confirming the message was processed).

attachSerial is not part of the public ably.d.ts (experimental/unstable per RTL15), so there's no public docstring to update.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed channel attachment handling so resumed connections preserve the existing attachment serial.
    • Ensured channel serials continue updating correctly after resumed attachments.
  • Tests

    • Added coverage for resumed and non-resumed attachment scenarios to verify serial behavior.

Implements ably/specification#503 (spec 6.1.1).

attachSerial is used as the fromSerial bound for untilAttach history,
which returns history contiguous with the realtime message stream. A
resumed ATTACHED (resumed=true) indicates a brief disconnection that did
not break message continuity, so it must not move the point up to which
untilAttach history is contiguous. Previously the SDK updated
attachSerial on every ATTACHED regardless of the RESUMED flag; now it
only does so when resumed is false.

RTL15a is superseded by RTL15c. Update the ported UTS channel_properties
tests accordingly, including a new test asserting attachSerial is left
unchanged by a resumed ATTACHED (while channelSerial still updates per
RTL15b, confirming the message was processed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@SimonWoolf
SimonWoolf requested a review from ttypic July 24, 2026 15:30
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a563b7f2-72a5-425c-b7db-89d3cd922982

📥 Commits

Reviewing files that changed from the base of the PR and between 004e99c and 1d9a372.

📒 Files selected for processing (2)
  • src/common/lib/client/realtimechannel.ts
  • test/uts/realtime/unit/channels/channel_properties.test.ts

Walkthrough

ATTACHED handling now preserves attachSerial for resumed attaches while continuing to update channelSerial. RTL15c tests cover non-resumed updates and resumed-message behavior.

Changes

Attach serial resume handling

Layer / File(s) Summary
Conditional attachSerial update
src/common/lib/client/realtimechannel.ts
processMessage() updates attachSerial only for non-resumed ATTACHED messages.
RTL15c attach serial coverage
test/uts/realtime/unit/channels/channel_properties.test.ts
Updates existing attach serial scenarios and adds coverage confirming resumed attaches update channelSerial without changing attachSerial.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: ttypic, paddybyers, sacoo7

Poem

I’m a bunny guarding serials tight,
Resumed attaches hop through the night.
channelSerial moves with cheer,
attachSerial stays right here.
RTL15c makes the behavior clear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: attachSerial is now updated only for non-resumed attaches in RTL15c.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch only-update-attachserial-for-non-resumed-attaches

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant