Skip to content

[AIT-1141] fix(uts/objects): spec-side corrections from the LiveObjects deviations audit#506

Open
sacOO7 wants to merge 4 commits into
uts-liveobjectsfrom
fix/liveobjects-spec-compliance-audit
Open

[AIT-1141] fix(uts/objects): spec-side corrections from the LiveObjects deviations audit#506
sacOO7 wants to merge 4 commits into
uts-liveobjectsfrom
fix/liveobjects-spec-compliance-audit

Conversation

@sacOO7

@sacOO7 sacOO7 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

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-0 and the RTO20e1 test injected an unsolicited server DETACHED and then asserted AWAIT_STATE channel.state == DETACHED. A compliant SDK immediately re-attaches on an unsolicited DETACHED (RTL13a), so the DETACHED state is transient, the AWAIT_STATE is 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 asserted result.serials — but RTO15 defines an internal function, and the realtime channel.object exposes no public publish. The test now drives the publish through a public mutation and asserts the outbound OBJECT ProtocolMessage shape; the PublishResult.serials contract is observed through its only consumer, the neighbouring RTO20 apply-on-ACK tests. The RTO15h clause 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 in public_object_message.md updated. 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.md gains a conventions section: == null means the language-appropriate absent value (undefined in 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, null where it is indistinguishable from "omitted") make that test/row not applicable — sanctioned omissions rather than coverage gaps.
  • The RTLC12e1 invalid-amounts table's null row and the RTLMV4b key-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

  • RTO24c1 subscribe call flipped to listener-first (subscribe(listener, { depth: 1 })), matching every other with-options call after the argument-order convergence.
  • objects_faults.md unit-tier cross-reference updated to fails-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)

  • Nil operation payloads (COUNTER_INC without counterInc, MAP_SET without mapSet, MAP_REMOVE without mapRemove) 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-liveobjects predates main's OD2g/OD3g (json ObjectData) clauses — a merge from main is recommended before further UTS reconciliation.

…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.
@github-actions
github-actions Bot temporarily deployed to staging/pull/506 July 16, 2026 17:05 Inactive
@sacOO7
sacOO7 requested a review from Copilot July 16, 2026 17:06
@sacOO7 sacOO7 changed the title fix(uts/objects): spec-side corrections from the LiveObjects deviations audit [AIT-1141] fix(uts/objects): spec-side corrections from the LiveObjects deviations audit Jul 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread uts/objects/unit/realtime_object.md
Comment thread uts/objects/unit/realtime_object.md
Comment thread uts/objects/unit/realtime_object.md
@sacOO7
sacOO7 requested a review from ttypic July 16, 2026 17:21
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.
@github-actions
github-actions Bot temporarily deployed to staging/pull/506 July 16, 2026 17:29 Inactive
action: "MAP_CREATE",
objectId: "map:new@2000",
mapCreate: { semantics: "LWW", entries: { "key1": { data: { string: "val1" } } } }
mapCreate: { semantics: "lww", entries: { "key1": { data: { string: "val1" } } } }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it's in lowercase? Doesn't it simply represent enum in actual code?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — semantics is the ObjectsMapSemantics enum, wire-encoded as an integer (OMP2: "values in order from zero: LWW"), and the lowercase form here was ably-js's idiomatic rendering (ObjectsMapSemantics = 'lww' string-literal union) leaking into the pseudo-code as if it were the canonical contract.

Reverted to the symbolic "LWW" in b394399, and added an Enum values bullet to the uts/README.md pseudocode conventions so each SDK's derivation is sanctioned explicitly: the enum member itself in typed SDKs (MapSemantics.LWW in ably-java), the string-literal union value in ably-js ('lww'). The ably-js derived test keeps asserting its real public API and now cites that convention instead of claiming lowercase is spec-mandated.

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.
@github-actions
github-actions Bot temporarily deployed to staging/pull/506 July 19, 2026 14:26 Inactive
…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).
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.

3 participants