[AIT-1141] fix(uts/objects): spec-side corrections from the LiveObjects deviations audit#506
Merged
Merged
Conversation
…ns audit - RTO25b/RTO26b/RTO20e1 unit tests reach DETACHED via client-side channel.detach(): an unsolicited server DETACHED triggers an immediate re-attach (RTL13a) in a compliant SDK, so the channel never observably stays DETACHED and AWAIT_STATE == DETACHED is a race. The shared helper mocks now answer the outbound DETACH with DETACHED. - RTO20e1 covers both reachable states as separate tests: fails-on-channel-detached-0 (client-side detach) and a new fails-on-channel-failed-0 (channel ERROR, matching the proxy tier per #501). SUSPENDED is connection-level and out of unit-mock scope. - RTO15 test asserts observable wire behaviour only: realtime channel.object exposes no public publish (RTO15 is internal), so the result.serials assertion was unrunnable; the serials contract is observed via the neighbouring RTO20 apply-on-ACK tests. The RTO15h clause itself is unchanged. - public_object_message semantics assertions use "lww" - the shipped public type (ObjectsMapSemantics = 'lww') is lowercase; "LWW" could never match. - RTO24c1 subscribe call fixed to listener-first, matching every other with-options subscribe call. - Pseudocode conventions added to uts/README.md (null/undefined mapping, property-vs-call syntax, language-inapplicable inputs) plus language-applicability notes on the RTLC12e1 null row and the RTLMV4b key-type test. - objects_faults.md unit-tier cross-reference updated to the fails-on-channel-failed-0 test it describes.
sacOO7
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the Objects UTS specs and helpers to align unit/proxy-tier tests and pseudocode conventions with shipped LiveObjects behavior (treating the reference SDK behavior as authoritative), and to remove spec-side assertions/premises that are not reachable for compliant SDKs.
Changes:
- Updates unit-tier channel-fault tests to use client-side detaches and introduces an ERROR→FAILED variant for RTO20e1.
- Adjusts the RTO15 unit test to assert observable wire behavior via a public mutation, rather than asserting an internal publish API surface.
- Documents language-mapping/applicability conventions in UTS docs and adds explicit “not applicable” notes to specific value-type tables.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uts/README.md | Adds pseudocode conventions for absent values, property access, and language-inapplicable inputs. |
| uts/objects/unit/value_types.md | Adds applicability note for non-string map key validation in JS/TS. |
| uts/objects/unit/realtime_object.md | Restructures several unit tests (RTO15, RTO20e1 variants, detach-related tests) and adjusts a subscribe call signature. |
| uts/objects/unit/public_object_message.md | Changes asserted public map semantics string from "LWW" to "lww". |
| uts/objects/unit/internal_live_counter_api.md | Adds applicability note for the null invalid-amount table row and annotates the row. |
| uts/objects/integration/proxy/objects_faults.md | Updates the unit-tier cross-reference text for the ERROR-based sequence. |
| uts/objects/helpers/standard_test_pool.md | Extends synced-channel helpers to respond to client DETACH with DETACHED. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sacOO7
added a commit
to ably/ably-js
that referenced
this pull request
Jul 16, 2026
… preconditions Review feedback on the companion spec PR (ably/specification#506): - RTO15 asserts the state entry is the encoded COUNTER_INC for the driven mutation (RTO15e3), not just that one entry exists. - Both RTO20e1 tests assert the increment is still parked in the RTO20e wait before the channel state transition, closing the false-positive where an SDK that rejects early with 92008 (without implementing the wait) would pass.
…preconditions Review feedback: - RTO15 asserts the state entry is the encoded ObjectMessage for the driven mutation (RTO15e3), not just that one entry exists. - Both RTO20e1 tests assert 'inc_future IS NOT complete' before the channel state transition (matching the RTO20e waits-for-synced pattern), pinning the parked-in-wait precondition and closing the false-positive where an SDK that rejects early with 92008 would pass.
ttypic
reviewed
Jul 17, 2026
Review feedback (ttypic): semantics is the ObjectsMapSemantics enum, wire-encoded as an integer (OMP2) - the lowercase form asserted here previously was ably-js's idiomatic string-literal rendering leaking into the pseudo-code. The assertions revert to the symbolic "LWW", and a new 'Enum values' bullet in the uts/README.md pseudocode conventions defines how SDKs derive their idiomatic public rendering from it (enum member in typed SDKs, string-literal union in ably-js). Also documents the provision_objects_via_rest response contract: the helper returns the created/updated objectIds flattened in request order, so derived tests can target follow-up operations by objectId.
…sync The bulk enterClient test counted only ENTER events on the observing client: if that client's connection drops mid-test, members it missed arrive via the presence re-sync as PRESENT events rather than ENTER, so an ENTER-only count undercounts forever and the poll burns its full timeout - the mechanism behind the intermittent CI failure of the derived test. Members are now counted by clientId from both ENTER and PRESENT events, asserted as a set size. Derived ably-js test updated in lock-step (ably/ably-js@f5be7694).
…from test specs - add reference pseudocode definitions for poll_until and the new poll_until_success to writing-test-specs.md: poll_until_success treats any error raised by the condition as "keep polling" and raises the most recent error on timeout so failures stay diagnosable; document both, plus flush_async(), in the README pseudocode conventions - use poll_until_success for mutable_messages store reads (not-found until a just-written message is visible) and objects_faults recovery polls (value reads raise while the channel is transiently DETACHED by fault injection) - fix value-form poll call sites that returned a boolean while their result was assigned and dereferenced (12 sites across mutable_messages, history, publish, presence and channel_history specs) - replace unit-tier real-time WAITs with deterministic steps: flush_async() (RTC1b, RTL5k), fake timers + ADVANCE_TIME (RSC15f x2), and an explicit second authorize() for RSA10e - RTO10/RTO10b1: stamp GC tombstones with now() so ADVANCE_TIME is what makes them eligible; RTO10b1 previously passed even when objectsGCGracePeriod was ignored - drop redundant default timeouts from objects integration polls
…mer guidance - rename the flush_async() pseudocode convention to process_pending_events(): intent-named like the other conventions (poll_until, AWAIT_STATE) rather than leaking the JavaScript event-loop mechanism; define it cross-language with flushAsync()/setImmediate (ably-js) and runCurrent() (kotlinx) as renderings - move the fake-time/poll-deadline trap out of the integration-timeouts section into "No real timers in unit tests" where readers will look for it; present the two remedies as an ordered list, cite ably-js's RTO10/RTO10b1 backdating as the worked example of the preferred remedy (correctly attributed as the derived rendering of the spec's ADVANCE_TIME), and note when a synchronous stub window is exempt (RTLM19) - state that mainstream fake-timer tools (Jest, sinon) mock Date by default, which is what makes the trap easy to hit - add negative-assertion guidance to writing-test-specs (settle pending events via process_pending_events(), never a fixed WAIT) and bridge the derived-test flushAsync() bullet back to the convention - drop "harness" phrasing in favour of the docs' established helper/suite terminology
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
Spec-side corrections produced by a comprehensive audit that reconciled ably-js LiveObjects against the objects spec (
objects-features.md+uts/objects), treating the reference implementation as the source of truth for behaviours that have already shipped. Every UTS unit test body was cross-checked against its derived implementation; the changes below fix spec tests that were unrunnable for a compliant SDK, assertions that contradicted the shipped public API, and pseudocode gaps that forced SDKs to document avoidable "deviations".Companion implementation PR: ably/ably-js#2263 (source fixes + derived tests, kept in lock-step with the test IDs and bodies changed here).
Changes
1. Unsolicited-server-DETACHED tests were unrunnable — switched to client-side detach
RTO25b/access-throws-detached-0,RTO26b/write-throws-detached-0and the RTO20e1 test injected an unsolicited serverDETACHEDand then assertedAWAIT_STATE channel.state == DETACHED. A compliant SDK immediately re-attaches on an unsolicited DETACHED (RTL13a), so the DETACHED state is transient, theAWAIT_STATEis a race, and — since every mock answers the re-ATTACH with ATTACHED — the precondition under test evaporates. This is the same reasoning that moved the proxy tier to an ERROR-based sequence in #501, and the spec's own RTPO19b test already used the correct pattern.All three now reach DETACHED via a client-side
AWAIT channel.detach()(solicited detach does not trigger RTL13a), and the shared helper mocks (setup_synced_channel,setup_synced_channel_no_ack) gain the DETACH → DETACHED responder the tests rely on.2. RTO20e1 now covers both reachable states
The single DETACHED-injection test is replaced by a pair:
objects/unit/RTO20e1/fails-on-channel-detached-0— client-side detach while the operation waits for SYNCED.objects/unit/RTO20e1/fails-on-channel-failed-0(new) — channel ERROR → FAILED, the same sequence as the proxy-tier test.SUSPENDED is a connection-level state and is noted as out of scope for a channel-level unit mock.
3. RTO15 test asserts observable behaviour only
The test obtained
result = AWAIT channel.object.publish([...])and assertedresult.serials— but RTO15 defines an internal function, and the realtimechannel.objectexposes no publicpublish. The test now drives the publish through a public mutation and asserts the outbound OBJECT ProtocolMessage shape; thePublishResult.serialscontract is observed through its only consumer, the neighbouring RTO20 apply-on-ACK tests. TheRTO15hclause itself is unchanged.4. Public map semantics asserted as
"lww"The shipped public type is lowercase (
ObjectsMapSemantics = 'lww' | 'unknown'in ably-js); asserting"LWW"could never match the public API. Five assertions inpublic_object_message.mdupdated. Reviewer note: please confirm ably-java's public rendering agrees — if it shipped uppercase, this becomes a cross-SDK reconciliation instead.5. Pseudocode conventions and language-applicability notes
uts/README.mdgains a conventions section:== nullmeans the language-appropriate absent value (undefinedin JS); member access written as a call (instance.id()) is satisfied by a property where the features spec defines one (RTINS3); inputs that cannot be constructed in a language (non-string map keys in JS,nullwhere it is indistinguishable from "omitted") make that test/row not applicable — sanctioned omissions rather than coverage gaps.RTLC12e1invalid-amounts table'snullrow and theRTLMV4bkey-type test carry explicit applicability notes referencing those conventions.This lets SDKs retire boilerplate "deviation" records for what are really language-mapping conventions.
6. Small fixes
RTO24c1subscribe call flipped to listener-first (subscribe(listener, { depth: 1 })), matching every other with-options call after the argument-order convergence.objects_faults.mdunit-tier cross-reference updated tofails-on-channel-failed-0, the test whose ERROR-based sequence it describes.Verification
The derived ably-js tests for every changed body pass spec-verbatim (full UTS unit suite: 1351 passing) — see ably/ably-js#2263 for the implementation side, including the two runtime bugs the same audit surfaced (sub-16-character object-id nonces, and one malformed inbound operation discarding its siblings).
Follow-ups (out of scope here)
COUNTER_INCwithoutcounterInc,MAP_SETwithoutmapSet,MAP_REMOVEwithoutmapRemove) are spec-silent; ably-js now warns-and-skips them consistently with RTLC7d3/RTLM15d4. Proposing explicit sub-clauses is left as a follow-up.uts-liveobjectspredatesmain'sOD2g/OD3g(jsonObjectData) clauses — a merge frommainis recommended before further UTS reconciliation.