Add UTS integration tests for REST and realtime#2212
Add UTS integration tests for REST and realtime#2212paddybyers wants to merge 1 commit intouts-realtimefrom
Conversation
25 new test files covering REST integration (auth, publish, history, presence, pagination, push admin, revoke tokens, mutable messages, batch presence, time/stats) and realtime integration (auth, channels, connection, presence, mutable messages, delta decoding). Also updates deviations.md with integration test deviations: RSC10 (token renewal infinite loop) and RSH1b2 (push pagination missing Link headers). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis PR adds comprehensive UTS integration test suites for Realtime and REST client authentication, channels, connection, presence, messaging, and push admin functionality. It includes a shared sandbox infrastructure module for provisioning ephemeral test apps and establishing test clients, plus 17 test files that exercise end-to-end flows across both protocol implementations. ChangesIntegration Test Suite
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
test/uts/rest/integration/revoke_tokens.test.ts (2)
184-193: ⚡ Quick winAvoid relying on result ordering in mixed success/failure assertions.
Index-based assertions are brittle if the API response order changes. Match entries by
target(or by presence oferror) before asserting.🔍 Suggested change
- const success = revokeResult.results[0] as any; + const success = revokeResult.results.find((r: any) => r.target === `clientId:${clientId}`) as any; expect(success.target).to.equal('clientId:' + clientId); expect(success.issuedBefore).to.be.a('number'); expect(success.appliesAt).to.be.a('number'); - const failure = revokeResult.results[1] as any; + const failure = revokeResult.results.find((r: any) => r.target === 'invalidType:abc') as any; expect(failure.target).to.equal('invalidType:abc'); expect(failure.error).to.exist; expect(failure.error.statusCode).to.equal(400);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/uts/rest/integration/revoke_tokens.test.ts` around lines 184 - 193, The test currently assumes ordering in revokeResult.results by indexing into results[0] and results[1]; instead locate entries by their target or presence of error (e.g., find entry where result.target === 'clientId:' + clientId and where result.target === 'invalidType:abc' or result.error is present) and then run the same assertions on those found objects (check success.issuedBefore/appliesAt are numbers and failure.error.statusCode === 400) so the test no longer depends on array order.
59-61: ⚡ Quick winAdd explicit timeout to disconnection waits to avoid long hangs.
If disconnect never arrives, these waits consume the full 60s test timeout and hide the immediate failure point. Wrap each wait with a short
Promise.racetimeout.⏱️ Suggested change
- const disconnectedPromise = new Promise<any>((resolve) => { - realtimeClient.connection.once('disconnected', resolve); - }); + const disconnectedPromise = Promise.race([ + new Promise<any>((resolve) => { + realtimeClient.connection.once('disconnected', resolve); + }), + new Promise<never>((_, reject) => + setTimeout(() => reject(new Error('Timed out waiting for disconnected event')), 15000), + ), + ]);Also applies to: 171-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/uts/rest/integration/revoke_tokens.test.ts` around lines 59 - 61, The disconnection wait uses an unbounded promise (disconnectedPromise created via realtimeClient.connection.once('disconnected', resolve)) which can hang tests; change the pattern to race that promise against a short timeout (e.g., 1-2s) using Promise.race so the test fails fast if 'disconnected' never fires, and apply the same change to the other occurrence that creates a disconnection wait (the similar promise at the second instance around revoke flow). Ensure the timeout path rejects or resolves with an informative error so the test surface indicates a timeout rather than consuming the full suite timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/uts/realtime/integration/sandbox.ts`:
- Around line 57-64: The deleteSandboxApp function currently ignores the DELETE
response; update it to capture the fetch Response (e.g., const res = await
fetch(...)), check res.ok (or res.status) and on non-2xx throw an Error or
reject with a message that includes the app.appId, res.status, and the response
body/text to surface why deletion failed so teardown doesn't silently leak
sandboxes.
In `@test/uts/rest/integration/auth.test.ts`:
- Line 45: The failing tests call client.request with only (method, path);
client.request requires six args including a mandatory version parameter, so
update each invocation of client.request in this test (the calls in auth.test.ts
that currently pass only method and path) to include the required version
argument (use the project constant like API_VERSION / DEFAULT_API_VERSION if
available, otherwise add the appropriate string such as 'v1'), preserving the
existing params, body and headers arguments (or pass null/undefined for
params/body/headers if they are not needed) so each call matches the signature
client.request(method, path, version, params, body, headers).
In `@test/uts/rest/integration/history.test.ts`:
- Line 49: Tests call the method channel.history() with no arguments but the
implementation signature requires params: RestHistoryParams | null; update each
test call (the ones at the noted call sites in
test/uts/rest/integration/history.test.ts) to pass an explicit null (i.e.,
channel.history(null)) so the calls match the typed signature and resolve
TS2554; look for usages of channel.history in this file (including the
occurrences around the earlier listed call sites) and change them to pass null.
- Around line 48-53: The variable history returned from pollUntil has type
HistoryResult | null; before dereferencing history.items in the assertions
(after the pollUntil call), narrow it to non-null (e.g., const nonNullHistory =
history!;) or perform a runtime null check, then use nonNullHistory.items for
the subsequent expects; update all assertions that currently use history.items
or history.items[index] (lines referencing history after pollUntil) to use the
narrowed symbol (nonNullHistory) or guarded access so TypeScript strict null
checks are satisfied.
In `@test/uts/rest/integration/mutable_messages.test.ts`:
- Around line 145-158: The callbacks passed to pollUntil return null on failure
which makes the result typed as T | null, but the tests (variables like
updatedMsg, deletedMsg, versions, annotations, result) are dereferenced without
narrowing; update each test to first guard that the pollUntil return value is
not null (e.g., if (!updatedMsg) throw or expect(updatedMsg).to.not.be.null) and
then use a non-null assertion or proceed after the check so subsequent property
accesses (name, data, action, version, description, etc.) are type-safe; apply
this same null-check-and-narrow pattern for the callbacks and usages at the
pollUntil sites (updatedMsg, deletedMsg, versions, annotations, result).
In `@test/uts/rest/integration/pagination.test.ts`:
- Around line 105-117: The test dereferences possible null results from
page1.next()/page2.next() (which return Promise<PaginatedResult<T> | null) and
pushes an optional item.id into allIds; add null checks after each await (e.g.,
assert/expect that page2 and page3 are not null or throw if null) before
accessing .items, and when iterating items add a type guard for item.id (skip or
assert when undefined) before calling allIds.push(item.id) so you never push
undefined and avoid the type error; update the same null-check pattern for the
other next() usages mentioned (page1.next(), page2.next(), page3.next()) and the
loop that inspects item.id.
- Around line 53-56: The tests call channel.history() with no arguments causing
TS2554; update every call to channel.history() (including those inside pollUntil
callbacks) to pass an explicit options object — use {} where no params are
needed (e.g., replace channel.history() with channel.history({})), ensuring the
method signature RestHistoryParams | null is satisfied; search for occurrences
in this file (e.g., the calls at lines referenced around channel.history and
inside pollUntil) and apply the same change consistently.
In `@test/uts/rest/integration/push_admin.test.ts`:
- Around line 152-167: The PushRecipient union in
src/common/lib/types/devicedetails.ts is missing APNS and GCM variants causing
TS2339 when tests access deviceToken/registrationToken; add APNSRecipient
(transportType: 'apns', deviceToken: string) and GCMRecipient (transportType:
'gcm', registrationToken: string) interfaces and include them in the
PushRecipient union (alongside WebPushRecipient and PushChannelRecipient), and
update the ably.d.ts declaration to use the new PushRecipient union instead of
any so typings reflect the added variants; update references in
functions/methods that consume PushRecipient accordingly (e.g.,
client.push.admin.deviceRegistrations types).
In `@test/uts/rest/integration/revoke_tokens.test.ts`:
- Around line 143-145: The test currently asserts
expect(result.appliesAt).to.be.greaterThan(serverTimeThirtySecondsLater) which
can be flaky on exact boundaries; change this assertion to accept equality by
using the non-strict form (e.g.,
expect(result.appliesAt).to.be.at.least(serverTimeThirtySecondsLater) or the
equivalent gte/greaterThanOrEqual form) in the revoke_tokens.test.ts test where
serverTimeThirtySecondsLater and result.appliesAt are compared.
In `@test/uts/rest/integration/time_stats.test.ts`:
- Line 66: The test calls to client.stats() must be updated to match the typed
signature requiring a RequestParams argument (Record<string,string> | null);
change the calls to pass either null or a params object with string values
(e.g., { limit: "100" }) instead of omitting the argument or passing numeric
values; update both occurrences referenced (the call at client.stats() and the
other at line ~90) so they pass valid RequestParams strings to satisfy the
types.
---
Nitpick comments:
In `@test/uts/rest/integration/revoke_tokens.test.ts`:
- Around line 184-193: The test currently assumes ordering in
revokeResult.results by indexing into results[0] and results[1]; instead locate
entries by their target or presence of error (e.g., find entry where
result.target === 'clientId:' + clientId and where result.target ===
'invalidType:abc' or result.error is present) and then run the same assertions
on those found objects (check success.issuedBefore/appliesAt are numbers and
failure.error.statusCode === 400) so the test no longer depends on array order.
- Around line 59-61: The disconnection wait uses an unbounded promise
(disconnectedPromise created via realtimeClient.connection.once('disconnected',
resolve)) which can hang tests; change the pattern to race that promise against
a short timeout (e.g., 1-2s) using Promise.race so the test fails fast if
'disconnected' never fires, and apply the same change to the other occurrence
that creates a disconnection wait (the similar promise at the second instance
around revoke flow). Ensure the timeout path rejects or resolves with an
informative error so the test surface indicates a timeout rather than consuming
the full suite timeout.
🪄 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: CHILL
Plan: Pro
Run ID: cb8bcd61-47a5-46b8-815a-786bf86b4021
📒 Files selected for processing (25)
test/uts/realtime/integration/auth/auth.test.tstest/uts/realtime/integration/auth/token_renewal.test.tstest/uts/realtime/integration/auth/token_request.test.tstest/uts/realtime/integration/channels/channel_attach.test.tstest/uts/realtime/integration/channels/channel_history.test.tstest/uts/realtime/integration/channels/channel_publish.test.tstest/uts/realtime/integration/channels/channel_subscribe.test.tstest/uts/realtime/integration/connection/connection_failures.test.tstest/uts/realtime/integration/connection/connection_lifecycle.test.tstest/uts/realtime/integration/delta_decoding.test.tstest/uts/realtime/integration/mutable_messages.test.tstest/uts/realtime/integration/presence/presence_lifecycle.test.tstest/uts/realtime/integration/presence/presence_sync.test.tstest/uts/realtime/integration/sandbox.tstest/uts/rest/integration/auth.test.tstest/uts/rest/integration/batch_presence.test.tstest/uts/rest/integration/history.test.tstest/uts/rest/integration/mutable_messages.test.tstest/uts/rest/integration/pagination.test.tstest/uts/rest/integration/presence.test.tstest/uts/rest/integration/publish.test.tstest/uts/rest/integration/push_admin.test.tstest/uts/rest/integration/revoke_tokens.test.tstest/uts/rest/integration/sandbox.tstest/uts/rest/integration/time_stats.test.ts
| async function deleteSandboxApp(app: SandboxApp): Promise<void> { | ||
| const url = `https://${SANDBOX_REST_HOST}/apps/${app.appId}`; | ||
| const credentials = Buffer.from(app.keys[0].keyStr).toString('base64'); | ||
| await fetch(url, { | ||
| method: 'DELETE', | ||
| headers: { Authorization: `Basic ${credentials}` }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Check DELETE response status during sandbox teardown.
deleteSandboxApp ignores non-2xx responses. That can silently leak sandbox apps and make test cleanup nondeterministic.
Proposed fix
async function deleteSandboxApp(app: SandboxApp): Promise<void> {
const url = `https://${SANDBOX_REST_HOST}/apps/${app.appId}`;
const credentials = Buffer.from(app.keys[0].keyStr).toString('base64');
- await fetch(url, {
+ const response = await fetch(url, {
method: 'DELETE',
headers: { Authorization: `Basic ${credentials}` },
});
+ if (!response.ok) {
+ const body = await response.text();
+ throw new Error(`Sandbox app deletion failed (${response.status}): ${body}`);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/realtime/integration/sandbox.ts` around lines 57 - 64, The
deleteSandboxApp function currently ignores the DELETE response; update it to
capture the fetch Response (e.g., const res = await fetch(...)), check res.ok
(or res.status) and on non-2xx throw an Error or reject with a message that
includes the app.appId, res.status, and the response body/text to surface why
deletion failed so teardown doesn't silently leak sandboxes.
| endpoint: SANDBOX_ENDPOINT, | ||
| }); | ||
|
|
||
| const result = await client.request('GET', '/channels/' + channelName); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify expected Rest.request signature and affected callsites.
rg -n "request\\s*\\(" ably.d.ts -n -C3
rg -n "client\\.request\\(" test/uts/rest/integration/auth.test.ts -n -C1Repository: ably/ably-js
Length of output: 648
🏁 Script executed:
# Find ably.d.ts and inspect Rest.request signature
fd "ably\\.d\\.ts" --type f
# Also search for request method in TypeScript definitions
rg "request\s*\(" --type ts -A 5 | head -50Repository: ably/ably-js
Length of output: 4078
🏁 Script executed:
# Get file size and inspect specific lines in the test file
wc -l test/uts/rest/integration/auth.test.ts
# Check line 101 specifically
sed -n '99,103p' test/uts/rest/integration/auth.test.tsRepository: ably/ably-js
Length of output: 241
🏁 Script executed:
# Read ably.d.ts to find Rest.request signature
cat ably.d.ts | grep -A 10 "class Rest\|interface Rest\|request\s*(" | head -80Repository: ably/ably-js
Length of output: 2128
🏁 Script executed:
# Get full context around the mentioned lines in auth.test.ts
sed -n '40,50p' test/uts/rest/integration/auth.test.ts
sed -n '98,105p' test/uts/rest/integration/auth.test.ts
sed -n '128,138p' test/uts/rest/integration/auth.test.ts
sed -n '160,170p' test/uts/rest/integration/auth.test.ts
sed -n '182,192p' test/uts/rest/integration/auth.test.ts
sed -n '232,242p' test/uts/rest/integration/auth.test.tsRepository: ably/ably-js
Length of output: 1750
🏁 Script executed:
# Search for the request method signature in Rest class
rg "request\s*\(" ably.d.ts -A 8 | grep -A 8 "request"Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Also check src/fragments/ably.d.ts
rg "request\s*\(" src/fragments/ably.d.ts -A 8 | head -60Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Search for "request" keyword in ably.d.ts files
rg "request" ably.d.ts -B 2 -A 5 | head -100Repository: ably/ably-js
Length of output: 5748
🏁 Script executed:
# Also search in src/fragments
rg "request" src/fragments/ably.d.ts -B 2 -A 5 | head -100Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Search for method declarations in type definitions
rg "^\s+(request\s*\(|request\s*:)" ably.d.ts -A 3Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Search in fragments as well
rg "^\s+(request\s*\(|request\s*:)" src/fragments/ably.d.ts -A 3Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Look for TypeScript config and build logs
fd "tsconfig\|package.json" --type f | head -5
# Check if there are any build error files
fd "\.log$" --type f | head -10Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Try simpler pattern to find Rest class and its methods
rg "class Rest\|interface Rest" ably.d.ts -A 30 | head -100Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Get file size first
wc -l ably.d.ts src/fragments/ably.d.ts
# Search for "Rest" class/interface definition
grep -n "class Rest\|interface Rest" ably.d.tsRepository: ably/ably-js
Length of output: 348
🏁 Script executed:
# Look for request method more broadly
grep -n "request" ably.d.ts | grep -v "^\s*\*" | head -20Repository: ably/ably-js
Length of output: 5013
🏁 Script executed:
# Read RestClient interface to find request method signature
sed -n '1803,1900p' ably.d.tsRepository: ably/ably-js
Length of output: 6723
🏁 Script executed:
# Read Rest class definition
sed -n '3568,3650p' ably.d.tsRepository: ably/ably-js
Length of output: 2660
client.request calls are missing the required version parameter (build blocker).
The request method signature requires 6 parameters: method, path, version, params, body, and headers. The version parameter is mandatory (not optional). However, at lines 45, 72, 101, 132, 165, 186, and 235, request() is invoked with only 2 arguments (method and path), missing the required version parameter. This will cause TypeScript error TS2554 at compile time.
🧰 Tools
🪛 GitHub Actions: API Reference
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🪛 GitHub Actions: Bundle size report
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🪛 GitHub Actions: Lint
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🪛 GitHub Actions: Spec coverage report
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🪛 GitHub Actions: Test (react hooks)
[error] 45-45: TypeScript (TS2554): Expected 6 arguments, but got 2.
🪛 GitHub Actions: Test browser
[error] 45-235: Multiple TypeScript errors in auth tests, including TS2554 (expected 6 args but got 2; expected 2 args but got 1) at lines 45, 72, 101, 123, 132, 165, 186, 235.
🪛 GitHub Actions: Test NodeJS
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🪛 GitHub Actions: Test NPM package
[error] 45-45: TS2554: Expected 6 arguments, but got 2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/auth.test.ts` at line 45, The failing tests call
client.request with only (method, path); client.request requires six args
including a mandatory version parameter, so update each invocation of
client.request in this test (the calls in auth.test.ts that currently pass only
method and path) to include the required version argument (use the project
constant like API_VERSION / DEFAULT_API_VERSION if available, otherwise add the
appropriate string such as 'v1'), preserving the existing params, body and
headers arguments (or pass null/undefined for params/body/headers if they are
not needed) so each call matches the signature client.request(method, path,
version, params, body, headers).
| const history = await pollUntil(async () => { | ||
| const result = await channel.history(); | ||
| return result.items.length === 3 ? result : null; | ||
| }, { interval: 500, timeout: 10000 }); | ||
|
|
||
| expect(history.items).to.have.length(3); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n test/uts/rest/integration/history.test.ts | head -80Repository: ably/ably-js
Length of output: 2829
🏁 Script executed:
rg -n "function pollUntil|const pollUntil|export.*pollUntil" test/uts/rest/integration/Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
fd -n sandbox.ts test/uts/rest/integration/Repository: ably/ably-js
Length of output: 287
🏁 Script executed:
fd sandbox.ts test/uts/rest/integration/Repository: ably/ably-js
Length of output: 93
🏁 Script executed:
cat -n test/uts/rest/integration/sandbox.tsRepository: ably/ably-js
Length of output: 871
🏁 Script executed:
cat -n test/realtime/integration/sandbox.ts | grep -A 20 "pollUntil"Repository: ably/ably-js
Length of output: 125
🏁 Script executed:
fd sandbox.ts test/realtime/Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
rg -l "function pollUntil|const pollUntil.*=" --type tsRepository: ably/ably-js
Length of output: 97
🏁 Script executed:
cat -n test/uts/realtime/integration/sandbox.ts | grep -A 25 "pollUntil"Repository: ably/ably-js
Length of output: 987
🏁 Script executed:
cat -n test/uts/rest/integration/history.test.ts | sed -n '48,70p'Repository: ably/ably-js
Length of output: 1033
🏁 Script executed:
rg -n "tsconfig|strict" --type json test/uts/rest/integration/ | head -5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
cat -n tsconfig.json | grep -A 5 "strict"Repository: ably/ably-js
Length of output: 277
Narrow history before dereferencing to fix strict null checking error.
The pollUntil callback returns result | null, making the inferred return type Promise<HistoryResult | null>. With strict: true in tsconfig.json, TypeScript requires narrowing before accessing history.items at lines 53, 56–63, and 66.
Add a null assertion after the pollUntil call:
const history = await pollUntil(async () => {
const result = await channel.history();
return result.items.length === 3 ? result : null;
}, { interval: 500, timeout: 10000 });
const nonNullHistory = history!;
expect(nonNullHistory.items).to.have.length(3);
expect(nonNullHistory.items[0].name).to.equal('event3');
// ... update remaining assertions🧰 Tools
🪛 GitHub Actions: API Reference
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
[error] 53-53: TS18047: 'history' is possibly 'null'.
🪛 GitHub Actions: Bundle size report
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
[error] 53-53: TS18047: 'history' is possibly 'null'.
🪛 GitHub Actions: Lint
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
[error] 53-53: TS18047: 'history' is possibly 'null'.
🪛 GitHub Actions: Spec coverage report
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
[error] 53-53: TS18047: 'history' is possibly 'null'.
🪛 GitHub Actions: Test (react hooks)
[error] 49-49: TypeScript (TS2554): Expected 1 arguments, but got 0.
[error] 53-53: TypeScript (TS18047): 'history' is possibly 'null'.
🪛 GitHub Actions: Test NodeJS
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test NPM package
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
[error] 53-53: TS18047: 'history' is possibly 'null'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/history.test.ts` around lines 48 - 53, The variable
history returned from pollUntil has type HistoryResult | null; before
dereferencing history.items in the assertions (after the pollUntil call), narrow
it to non-null (e.g., const nonNullHistory = history!;) or perform a runtime
null check, then use nonNullHistory.items for the subsequent expects; update all
assertions that currently use history.items or history.items[index] (lines
referencing history after pollUntil) to use the narrowed symbol (nonNullHistory)
or guarded access so TypeScript strict null checks are satisfied.
|
|
||
| // Poll until messages appear in history | ||
| const history = await pollUntil(async () => { | ||
| const result = await channel.history(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the test file exists and examine the relevant lines
fd -t f history.test.tsRepository: ably/ably-js
Length of output: 311
🏁 Script executed:
# Read the test file to see the actual code at the specified lines
wc -l test/uts/rest/integration/history.test.tsRepository: ably/ably-js
Length of output: 102
🏁 Script executed:
# Get the context around the specified lines
sed -n '40,60p' test/uts/rest/integration/history.test.ts
sed -n '85,105p' test/uts/rest/integration/history.test.ts
sed -n '115,135p' test/uts/rest/integration/history.test.ts
sed -n '160,180p' test/uts/rest/integration/history.test.ts
sed -n '200,220p' test/uts/rest/integration/history.test.tsRepository: ably/ably-js
Length of output: 3270
🏁 Script executed:
# Find the channel type and history method definition
# First, let's search for imports and variable declarations
head -50 test/uts/rest/integration/history.test.tsRepository: ably/ably-js
Length of output: 1355
🏁 Script executed:
# Search for the history method definition in the codebase
fd -t f -e ts -e js | xargs rg "history\s*\(" -B 2 -A 5 --max-count=20Repository: ably/ably-js
Length of output: 50368
🏁 Script executed:
# Find the main source files (not test files)
fd -t f -e ts -e js 'src' | head -20Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Search for the history method definition in source code
fd -t f -e ts 'src' | xargs rg "history\s*\(" -B 3 -A 8 | head -100Repository: ably/ably-js
Length of output: 7742
🏁 Script executed:
# Look for type definitions related to history
fd -t f -e ts -e d.ts | xargs rg "history.*\(.*\)" -B 2 -A 5 | grep -E "(history|interface|type|class)" | head -40Repository: ably/ably-js
Length of output: 3474
🏁 Script executed:
# Look for source files in the repository structure
find . -type f -name "*.ts" -not -path "*/node_modules/*" -not -path "*/test/*" | head -30Repository: ably/ably-js
Length of output: 1156
🏁 Script executed:
# Search for Channel class definition
rg "class.*Channel" -t ts --max-count=10Repository: ably/ably-js
Length of output: 1181
🏁 Script executed:
# Search for the history method signature more broadly
rg "history\s*\(.*\)" -t ts --max-count=20 | grep -v test | grep -v "\.test\."Repository: ably/ably-js
Length of output: 2451
🏁 Script executed:
# Check the actual lines in the test file
sed -n '45,55p' test/uts/rest/integration/history.test.ts
sed -n '87,97p' test/uts/rest/integration/history.test.ts
sed -n '118,128p' test/uts/rest/integration/history.test.ts
sed -n '162,172p' test/uts/rest/integration/history.test.ts
sed -n '205,215p' test/uts/rest/integration/history.test.tsRepository: ably/ably-js
Length of output: 1913
🏁 Script executed:
# Check the RestHistoryParams type definition
rg "RestHistoryParams" -t ts -A 5 -B 2 | head -30Repository: ably/ably-js
Length of output: 1628
Pass explicit null to history() to satisfy the typed method signature.
These call sites currently fail with TS2554 ("Expected 1 arguments, but got 0"). The implementation signature requires a parameter (params: RestHistoryParams | null) while the integration tests call channel.history() without arguments.
Proposed fix
- const result = await channel.history();
+ const result = await channel.history(null);
@@
- const result = await channel.history();
+ const result = await channel.history(null);
@@
- const result = await channel.history();
+ const result = await channel.history(null);
@@
- const result = await channel.history();
+ const result = await channel.history(null);
@@
- const history = await channel.history();
+ const history = await channel.history(null);Also applies to: 91-91, 122-122, 166-166, 209-209
🧰 Tools
🪛 GitHub Actions: API Reference
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Bundle size report
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Lint
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Spec coverage report
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test (react hooks)
[error] 49-49: TypeScript (TS2554): Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test browser
[error] 49-209: Multiple TypeScript errors in history tests: TS2554 (expected 1 argument but got 0) and TS18047 ('history' is possibly 'null') at lines 49, 53, 56, 57, 59, 60, 62, 63, 66, 91, 122, 166, 209.
🪛 GitHub Actions: Test NodeJS
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test NPM package
[error] 49-49: TS2554: Expected 1 arguments, but got 0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/history.test.ts` at line 49, Tests call the method
channel.history() with no arguments but the implementation signature requires
params: RestHistoryParams | null; update each test call (the ones at the noted
call sites in test/uts/rest/integration/history.test.ts) to pass an explicit
null (i.e., channel.history(null)) so the calls match the typed signature and
resolve TS2554; look for usages of channel.history in this file (including the
occurrences around the earlier listed call sites) and change them to pass null.
| const updatedMsg = await pollUntil( | ||
| async () => { | ||
| const msg = await channel.getMessage(serial); | ||
| if (msg.action === 'message.update') return msg; | ||
| return null; | ||
| }, | ||
| { interval: 500, timeout: 10000 }, | ||
| ); | ||
|
|
||
| expect(updatedMsg.name).to.equal('updated'); | ||
| expect(updatedMsg.data).to.equal('updated-data'); | ||
| expect(updatedMsg.action).to.equal('message.update'); | ||
| expect(updatedMsg.version).to.be.an('object'); | ||
| expect(updatedMsg.version!.description).to.equal('edited content'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all nullable pollUntil assignments are guarded before use.
rg -n "const (updatedMsg|deletedMsg|versions|annotations|result) = await pollUntil" test/uts/rest/integration/mutable_messages.test.ts -n -C2
rg -n "updatedMsg\\.|deletedMsg\\.|versions\\.|annotations\\.|result\\." test/uts/rest/integration/mutable_messages.test.ts -n -C1Repository: ably/ably-js
Length of output: 3782
🏁 Script executed:
# Find the pollUntil function definition
rg -n "function pollUntil|const pollUntil|export.*pollUntil" --type ts --type tsxRepository: ably/ably-js
Length of output: 84
🏁 Script executed:
# Check the return type and signature of pollUntil
rg -A 10 "pollUntil.*=" test/uts/rest/integration/mutable_messages.test.ts | head -20Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Search for pollUntil in test utilities
fd -name "*.ts" -o -name "*.tsx" | xargs rg -l "pollUntil" | grep -E "(setup|util|helper|common)" | head -5Repository: ably/ably-js
Length of output: 287
🏁 Script executed:
# Search for pollUntil definition across all TypeScript files
rg "pollUntil" test/uts/ -l | head -10Repository: ably/ably-js
Length of output: 538
🏁 Script executed:
# Look for import of pollUntil in the test file
head -50 test/uts/rest/integration/mutable_messages.test.ts | rg -n "import|pollUntil"Repository: ably/ably-js
Length of output: 116
🏁 Script executed:
# Search for function definition of pollUntil
rg -B2 -A8 "^(export )?function pollUntil|const pollUntil = "Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Get the full import statement for pollUntil
sed -n '9,20p' test/uts/rest/integration/mutable_messages.test.tsRepository: ably/ably-js
Length of output: 283
🏁 Script executed:
# Find pollUntil definition in sandbox.ts
rg -B2 -A10 "pollUntil" test/uts/rest/integration/sandbox.tsRepository: ably/ably-js
Length of output: 306
🏁 Script executed:
# Find pollUntil definition in realtime sandbox
rg -B2 -A12 "^export.*function pollUntil|^function pollUntil|^export const pollUntil" test/uts/realtime/integration/sandbox.tsRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Broader search for pollUntil in the sandbox file
rg -A15 "pollUntil" test/uts/realtime/integration/sandbox.ts | head -40Repository: ably/ably-js
Length of output: 712
🏁 Script executed:
# Check if there are type definitions or signatures
rg -B5 -A15 "pollUntil.*=" test/uts/realtime/integration/sandbox.tsRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Check for TS18047 or type errors in the codebase
rg "TS18047|error TS" --type tsRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Check if there's a TypeScript build or lint configuration
cat tsconfig.json | head -40Repository: ably/ably-js
Length of output: 542
🏁 Script executed:
# Look for CI configuration that might report the errors
find . -maxdepth 2 -name "*.yml" -o -name "*.yaml" | head -5Repository: ably/ably-js
Length of output: 107
🏁 Script executed:
# Check the GitHub workflow to see what TypeScript checks are run
cat .github/workflows/*.yml 2>/dev/null | grep -A10 -B5 "typescript\|tsc\|typecheck" | head -60Repository: ably/ably-js
Length of output: 908
🏁 Script executed:
# List available workflows
ls -la .github/workflows/ 2>/dev/nullRepository: ably/ably-js
Length of output: 807
🏁 Script executed:
# Check if there are any recent build or lint configuration files
find . -maxdepth 3 -name "package.json" -exec grep -l "tsc\|typecheck" {} \;Repository: ably/ably-js
Length of output: 1546
🏁 Script executed:
# Check the lint and test scripts in package.json
rg "\"lint\"|\"typecheck\"|\"tsc\"" package.json -A2Repository: ably/ably-js
Length of output: 146
🏁 Script executed:
# Get the actual npm scripts section
jq '.scripts' package.jsonRepository: ably/ably-js
Length of output: 2064
🏁 Script executed:
# Check if there's any TypeScript type checking for test files in the build process
rg "tsc.*test|typecheck" Gruntfile.js 2>/dev/null || echo "No matches or file not found"Repository: ably/ably-js
Length of output: 113
🏁 Script executed:
# Look for how the test files are type-checked, if at all
cat .eslintrc* 2>/dev/null | grep -A5 -B5 "typescript\|parser" || echo "No eslint config found"Repository: ably/ably-js
Length of output: 1374
🏁 Script executed:
# Now let's verify the actual type issue by checking what TypeScript infers for pollUntil return type
# when the callback can return T | null
cat > /tmp/test_types.ts << 'EOF'
// Simulating the pollUntil function signature
async function pollUntil<T>(
fn: () => Promise<T> | T,
opts?: { interval?: number; timeout?: number },
): Promise<T> {
return null as any;
}
// Test case: callback can return string | null
async function testCase() {
const result = await pollUntil(
async () => {
const x: string | null = Math.random() > 0.5 ? 'hello' : null;
return x;
}
);
// What type is result here?
const x: typeof result = result;
}
EOF
cat /tmp/test_types.tsRepository: ably/ably-js
Length of output: 546
🏁 Script executed:
# Verify the actual code to see if the callbacks in the test return T | null
sed -n '145,152p' test/uts/rest/integration/mutable_messages.test.tsRepository: ably/ably-js
Length of output: 305
🏁 Script executed:
# Check what the Gruntfile does for typecheck
grep -B10 -A10 "typecheck" Gruntfile.js 2>/dev/null | head -40Repository: ably/ably-js
Length of output: 796
🏁 Script executed:
# Look for CI configuration that runs typecheck on tests
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "typecheck\|tsc.*test" 2>/dev/nullRepository: ably/ably-js
Length of output: 40
🏁 Script executed:
# Check if the test files are included in the main tsconfig
rg "test/" tsconfig.jsonRepository: ably/ably-js
Length of output: 199
🏁 Script executed:
# Check the actual current state of all 5 problem locations
echo "=== LINE 145-160 (updatedMsg) ==="
sed -n '145,160p' test/uts/rest/integration/mutable_messages.test.ts
echo -e "\n=== LINE 187-200 (deletedMsg) ==="
sed -n '187,200p' test/uts/rest/integration/mutable_messages.test.ts
echo -e "\n=== LINE 228-245 (versions) ==="
sed -n '228,245p' test/uts/rest/integration/mutable_messages.test.tsRepository: ably/ably-js
Length of output: 1579
🏁 Script executed:
# Check lines 300-315 (annotations)
echo "=== LINE 300-315 (annotations) ==="
sed -n '300,315p' test/uts/rest/integration/mutable_messages.test.ts
echo -e "\n=== LINE 357-370 (result) ==="
sed -n '357,370p' test/uts/rest/integration/mutable_messages.test.tsRepository: ably/ably-js
Length of output: 1075
🏁 Script executed:
# Run TypeScript compiler on the test file to see actual errors
cd test/uts/rest/integration && npx tsc --noEmit mutable_messages.test.ts 2>&1 | head -50 || trueRepository: ably/ably-js
Length of output: 7715
🏁 Script executed:
# Check if there's a way to run tsc on just the test files from root
npx tsc --noEmit test/uts/rest/integration/mutable_messages.test.ts 2>&1 | head -50 || trueRepository: ably/ably-js
Length of output: 7139
🏁 Script executed:
# Try checking what the actual inferred type of updatedMsg is with strict null checks
cat > /tmp/check_inference.ts << 'EOF'
interface Message {
action: string;
name?: string;
data?: string;
version?: { description?: string };
}
async function pollUntil<T>(
fn: () => Promise<T> | T,
opts?: { interval?: number; timeout?: number }
): Promise<T> {
const interval = opts?.interval || 500;
const timeout = opts?.timeout || 10000;
const start = Date.now();
while (true) {
const result = await fn();
if (result) return result;
if (Date.now() - start > timeout) {
throw new Error(`pollUntil timed out after ${timeout}ms`);
}
await new Promise((r) => setTimeout(r, interval));
}
}
async function test() {
// Simulating the actual code from the test file
const updatedMsg = await pollUntil(
async () => {
const msg: Message = { action: 'message.update' };
if (msg.action === 'message.update') return msg;
return null;
}
);
// This line should produce an error if msg could be null
console.log(updatedMsg.name); // <-- Type error here?
}
EOF
npx tsc --strict --noEmit /tmp/check_inference.ts 2>&1 || trueRepository: ably/ably-js
Length of output: 144
🏁 Script executed:
# Verify if tests are actually type-checked in the main CI pipeline
cat .github/workflows/check.ymlRepository: ably/ably-js
Length of output: 1258
🏁 Script executed:
# Check if the test-node workflow runs type checking
cat .github/workflows/test-node.ymlRepository: ably/ably-js
Length of output: 1369
🏁 Script executed:
# Check what "npm run test:uts" actually does and if it type-checks
grep -A5 "test:uts" package.jsonRepository: ably/ably-js
Length of output: 470
Nullability from pollUntil must be narrowed before dereference.
At lines 145, 187, 228, 300, and 357, the callbacks passed to pollUntil() explicitly return null on condition failure, making the return type T | null. The subsequent property dereferences without null checks will trigger TypeScript TS18047 errors.
Suggested fix pattern
const updatedMsg = await pollUntil(
async () => {
const msg = await channel.getMessage(serial);
if (msg.action === 'message.update') return msg;
return null;
},
{ interval: 500, timeout: 10000 },
);
+expect(updatedMsg, 'Timed out waiting for message.update').to.not.be.null;
-expect(updatedMsg.name).to.equal('updated');
+expect(updatedMsg!.name).to.equal('updated');Apply the same guard+non-null assertion pattern for deletedMsg, versions, annotations, and result.
Also applies to lines 187–196, 228–243, 300–312, 357–368.
🧰 Tools
🪛 GitHub Actions: API Reference
[error] 154-154: TS18047: 'updatedMsg' is possibly 'null'.
[error] 155-155: TS18047: 'updatedMsg' is possibly 'null'.
[error] 156-156: TS18047: 'updatedMsg' is possibly 'null'.
[error] 157-157: TS18047: 'updatedMsg' is possibly 'null'.
[error] 158-158: TS18047: 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Bundle size report
[error] 154-154: TS18047: 'updatedMsg' is possibly 'null'.
[error] 155-155: TS18047: 'updatedMsg' is possibly 'null'.
[error] 156-156: TS18047: 'updatedMsg' is possibly 'null'.
[error] 157-157: TS18047: 'updatedMsg' is possibly 'null'.
[error] 158-158: TS18047: 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Lint
[error] 154-154: TS18047: 'updatedMsg' is possibly 'null'.
[error] 155-155: TS18047: 'updatedMsg' is possibly 'null'.
[error] 156-156: TS18047: 'updatedMsg' is possibly 'null'.
[error] 157-157: TS18047: 'updatedMsg' is possibly 'null'.
[error] 158-158: TS18047: 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Spec coverage report
[error] 154-154: TS18047: 'updatedMsg' is possibly 'null'.
[error] 155-155: TS18047: 'updatedMsg' is possibly 'null'.
[error] 156-156: TS18047: 'updatedMsg' is possibly 'null'.
[error] 157-157: TS18047: 'updatedMsg' is possibly 'null'.
[error] 158-158: TS18047: 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Test (react hooks)
[error] 154-154: TypeScript (TS18047): 'updatedMsg' is possibly 'null'.
[error] 155-155: TypeScript (TS18047): 'updatedMsg' is possibly 'null'.
[error] 156-156: TypeScript (TS18047): 'updatedMsg' is possibly 'null'.
[error] 157-157: TypeScript (TS18047): 'updatedMsg' is possibly 'null'.
[error] 158-158: TypeScript (TS18047): 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Test NodeJS
[error] 154-158: TS18047: 'updatedMsg' is possibly 'null'.
🪛 GitHub Actions: Test NPM package
[error] 154-154: TS18047: 'updatedMsg' is possibly 'null'.
[error] 155-155: TS18047: 'updatedMsg' is possibly 'null'.
[error] 156-156: TS18047: 'updatedMsg' is possibly 'null'.
[error] 157-157: TS18047: 'updatedMsg' is possibly 'null'.
[error] 158-158: TS18047: 'updatedMsg' is possibly 'null'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/mutable_messages.test.ts` around lines 145 - 158,
The callbacks passed to pollUntil return null on failure which makes the result
typed as T | null, but the tests (variables like updatedMsg, deletedMsg,
versions, annotations, result) are dereferenced without narrowing; update each
test to first guard that the pollUntil return value is not null (e.g., if
(!updatedMsg) throw or expect(updatedMsg).to.not.be.null) and then use a
non-null assertion or proceed after the check so subsequent property accesses
(name, data, action, version, description, etc.) are type-safe; apply this same
null-check-and-narrow pattern for the callbacks and usages at the pollUntil
sites (updatedMsg, deletedMsg, versions, annotations, result).
| await pollUntil( | ||
| async () => { | ||
| const result = await channel.history(); | ||
| return result.items.length === 15 ? result : null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify zero-arg history() calls in this test file
rg -nP 'channel\.history\(\)' test/uts/rest/integration/pagination.test.ts
# Inspect history()/pagination-related signatures in client code
rg -nP '\bhistory\s*\(' src/common/lib/client -g '*.ts' -C2Repository: ably/ably-js
Length of output: 3835
🏁 Script executed:
rg -nP 'type\s+RestHistoryParams|interface\s+RestHistoryParams' src/Repository: ably/ably-js
Length of output: 138
🏁 Script executed:
rg -nP 'RestHistoryParams' src/common/lib/types/index.ts -A 10 | head -30Repository: ably/ably-js
Length of output: 126
🏁 Script executed:
sed -n '12,40p' src/common/lib/client/restchannelmixin.tsRepository: ably/ably-js
Length of output: 976
🏁 Script executed:
rg -nP 'channel\.history\(' test/uts/rest/integration/pagination.test.ts | head -20Repository: ably/ably-js
Length of output: 605
Pass an explicit options object to channel.history(...) calls to fix TS2554 error blocking CI.
Lines 55, 98, 148, 192, and 246 call history() with zero arguments, but the method requires a parameter of type RestHistoryParams | null. Passing an empty object {} resolves the TypeScript error and aligns with the pattern used elsewhere in the test file.
Suggested patch
- const result = await channel.history();
+ const result = await channel.history({});
@@
- const result = await channel.history();
+ const result = await channel.history({});
@@
- const result = await channel.history();
+ const result = await channel.history({});
@@
- const result = await channel.history();
+ const result = await channel.history({});
@@
- const result = await channel.history();
+ const result = await channel.history({});🧰 Tools
🪛 GitHub Actions: API Reference
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Bundle size report
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Lint
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Spec coverage report
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test (react hooks)
[error] 55-55: TypeScript (TS2554): Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test NodeJS
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test NPM package
[error] 55-55: TS2554: Expected 1 arguments, but got 0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/pagination.test.ts` around lines 53 - 56, The tests
call channel.history() with no arguments causing TS2554; update every call to
channel.history() (including those inside pollUntil callbacks) to pass an
explicit options object — use {} where no params are needed (e.g., replace
channel.history() with channel.history({})), ensuring the method signature
RestHistoryParams | null is satisfied; search for occurrences in this file
(e.g., the calls at lines referenced around channel.history and inside
pollUntil) and apply the same change consistently.
| const page2 = await page1.next(); | ||
| const page3 = await page2.next(); | ||
|
|
||
| expect(page1.items.length).to.equal(5); | ||
| expect(page2.items.length).to.equal(5); | ||
| expect(page3.items.length).to.equal(2); | ||
|
|
||
| // Verify no duplicate messages across pages | ||
| const allIds: string[] = []; | ||
| for (const page of [page1, page2, page3]) { | ||
| for (const item of page.items) { | ||
| expect(allIds).to.not.include(item.id); | ||
| allIds.push(item.id); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify next() nullable contract
rg -nP 'async\s+next\(\):\s*Promise<[^>]+\|\s*null>' src/common/lib/client/paginatedresource.ts -C2
# Verify call sites that dereference next() directly in this file
rg -nP 'await\s+page\d*\.next\(\)|page\s*=\s*await\s+page\.next\(\)' test/uts/rest/integration/pagination.test.ts -C2
# Verify message id typing in declarations
rg -nP '\bid\??\s*:\s*string' ably.d.ts -C2Repository: ably/ably-js
Length of output: 2111
🏁 Script executed:
# Find the PaginatedResource class and next() method definition
fd "paginatedresource" --type fRepository: ably/ably-js
Length of output: 99
🏁 Script executed:
# Search for next() method in common lib
rg -n "next\(\)" src/common/lib/client/ -A 3 --type tsRepository: ably/ably-js
Length of output: 375
🏁 Script executed:
# Get the exact type definition for the Page/PaginatedResult returned by next()
rg -n "interface.*Page|class.*Page|type.*Page" ably.d.ts -A 5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
# Read the test file to see actual context at the problematic lines
cat -n test/uts/rest/integration/pagination.test.ts | sed -n '100,120p'Repository: ably/ably-js
Length of output: 788
🏁 Script executed:
# Read another problematic section
cat -n test/uts/rest/integration/pagination.test.ts | sed -n '150,160p'Repository: ably/ably-js
Length of output: 443
🏁 Script executed:
# Read the third section with the while loop
cat -n test/uts/rest/integration/pagination.test.ts | sed -n '195,215p'Repository: ably/ably-js
Length of output: 681
🏁 Script executed:
# Find Message interface and id field definition
rg -n "interface Message" ably.d.ts -A 20Repository: ably/ably-js
Length of output: 4666
🏁 Script executed:
# Verify the complete Message interface to confirm id is optional
rg -n "interface Message" ably.d.ts -A 50 | head -60Repository: ably/ably-js
Length of output: 2368
Add null checks for next() calls and type guards for optional id field.
The next() method returns Promise<PaginatedResult<T> | null>, but lines 105-106 and 154-156 dereference the result without null checking. Additionally, line 117 pushes item.id (which is optional: id?: string) to the array without narrowing, causing a type error.
Suggested fix
const page2 = await page1.next();
+ if (!page2) throw new Error('Expected page2 to exist');
const page3 = await page2.next();
+ if (!page3) throw new Error('Expected page3 to exist');
const allIds: string[] = [];
for (const page of [page1, page2, page3]) {
for (const item of page.items) {
+ if (!item.id) continue;
expect(allIds).to.not.include(item.id);
allIds.push(item.id);
}
}Apply the same null check pattern to lines 154-156:
const page2 = await page1.next();
+ if (!page2) throw new Error('Expected page2 to exist');
const firstPage = await page2.first();Also applies to: 154-156
🧰 Tools
🪛 GitHub Actions: API Reference
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: Bundle size report
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string' (undefined not allowed).
🪛 GitHub Actions: Lint
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string' (undefined not allowed).
🪛 GitHub Actions: Spec coverage report
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: Test (react hooks)
[error] 106-106: TypeScript (TS18047): 'page2' is possibly 'null'.
[error] 109-109: TypeScript (TS18047): 'page2' is possibly 'null'.
[error] 110-110: TypeScript (TS18047): 'page3' is possibly 'null'.
[error] 115-115: TypeScript (TS18047): 'page' is possibly 'null'.
[error] 117-117: TypeScript (TS2345): Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: Test NodeJS
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: Test NPM package
[error] 106-106: TS18047: 'page2' is possibly 'null'.
[error] 109-109: TS18047: 'page2' is possibly 'null'.
[error] 110-110: TS18047: 'page3' is possibly 'null'.
[error] 115-115: TS18047: 'page' is possibly 'null'.
[error] 117-117: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/pagination.test.ts` around lines 105 - 117, The
test dereferences possible null results from page1.next()/page2.next() (which
return Promise<PaginatedResult<T> | null) and pushes an optional item.id into
allIds; add null checks after each await (e.g., assert/expect that page2 and
page3 are not null or throw if null) before accessing .items, and when iterating
items add a type guard for item.id (skip or assert when undefined) before
calling allIds.push(item.id) so you never push undefined and avoid the type
error; update the same null-check pattern for the other next() usages mentioned
(page1.next(), page2.next(), page3.next()) and the loop that inspects item.id.
| const updated = await client.push.admin.deviceRegistrations.save({ | ||
| id: deviceId, | ||
| platform: 'ios', | ||
| formFactor: 'phone', | ||
| push: { | ||
| recipient: { transportType: 'apns', deviceToken: 'token-v2' }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(updated.id).to.equal(deviceId); | ||
| expect(updated.push!.recipient!.deviceToken).to.equal('token-v2'); | ||
|
|
||
| // Verify via get | ||
| const retrieved = await client.push.admin.deviceRegistrations.get(deviceId); | ||
| expect(retrieved.push!.recipient!.deviceToken).to.equal('token-v2'); | ||
| } finally { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the failing property access sites
rg -nP 'recipient!\.deviceToken|recipient\?\.deviceToken' test/uts/rest/integration/push_admin.test.ts -C2
# Inspect push recipient type declarations
rg -nP 'type\s+PushRecipient|interface\s+.*PushRecipient|deviceToken|registrationToken|transportType' ably.d.ts -C2Repository: ably/ably-js
Length of output: 478
🏁 Script executed:
#!/bin/bash
# Find and inspect PushRecipient type definition
rg -n 'type\s+PushRecipient|interface\s+PushRecipient' --type ts --type tsxRepository: ably/ably-js
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Look for deviceRegistrations related type definitions
rg -n 'deviceRegistrations|DeviceRegistration' --type ts --type tsx -A 5 | head -100Repository: ably/ably-js
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check if ably.d.ts exists and inspect PushRecipient definition
find . -name "*.d.ts" -type f | head -20Repository: ably/ably-js
Length of output: 797
🏁 Script executed:
#!/bin/bash
# Search for PushRecipient type definition
rg -n 'PushRecipient' ably.d.ts push.d.ts -A 10Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check the test file to understand context and imports
head -30 test/uts/rest/integration/push_admin.test.tsRepository: ably/ably-js
Length of output: 705
🏁 Script executed:
#!/bin/bash
# Look at the full test method to understand the context
sed -n '140,175p' test/uts/rest/integration/push_admin.test.tsRepository: ably/ably-js
Length of output: 1145
🏁 Script executed:
#!/bin/bash
# Search in push.d.ts for PushRecipient
cat push.d.ts | head -200Repository: ably/ably-js
Length of output: 1368
🏁 Script executed:
#!/bin/bash
# Search for PushRecipient in ably.d.ts more carefully
rg -n 'PushRecipient' ably.d.ts -A 15Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Look for recipient type definition in broader context
rg -n 'recipient' push.d.ts -B 2 -A 2Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Search for deviceToken in type definitions
rg -n 'deviceToken' ably.d.ts -B 5 -A 5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Search for recipient in ably.d.ts
rg -n 'recipient' ably.d.ts -B 3 -A 3Repository: ably/ably-js
Length of output: 862
🏁 Script executed:
#!/bin/bash
# Look for APNS or FCM related types
rg -n 'APNS|FCM|GCM' ably.d.ts -B 2 -A 5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Search for deviceRegistrations and its type definition
rg -n 'deviceRegistrations' ably.d.ts -B 5 -A 10Repository: ably/ably-js
Length of output: 860
🏁 Script executed:
#!/bin/bash
# Search for DeviceRegistration interface/type
rg -n 'DeviceRegistration|interface.*Registration' ably.d.ts -B 2 -A 10Repository: ably/ably-js
Length of output: 4070
🏁 Script executed:
#!/bin/bash
# Look at the full context around line 1320
sed -n '1300,1350p' ably.d.tsRepository: ably/ably-js
Length of output: 1296
🏁 Script executed:
#!/bin/bash
# Search for DeviceDetails interface
rg -n 'interface DeviceDetails|export.*DeviceDetails' ably.d.ts -B 2 -A 20Repository: ably/ably-js
Length of output: 2222
🏁 Script executed:
#!/bin/bash
# Check the actual TypeScript compilation errors by looking at CI logs or tsconfig
cat tsconfig.jsonRepository: ably/ably-js
Length of output: 542
🏁 Script executed:
#!/bin/bash
# Check if there's a build or test output showing the actual error
find . -name "*.log" -o -name "tsconfig*.json" | head -10Repository: ably/ably-js
Length of output: 466
🏁 Script executed:
#!/bin/bash
# Get the full DeviceDetails interface definition
sed -n '1259,1310p' ably.d.tsRepository: ably/ably-js
Length of output: 1483
🏁 Script executed:
#!/bin/bash
# Search specifically for push property in DeviceDetails context
rg -n 'export interface DeviceDetails' ably.d.ts -A 60 | head -70Repository: ably/ably-js
Length of output: 2058
🏁 Script executed:
#!/bin/bash
# Confirm DevicePushDetails.recipient type one more time
sed -n '1313,1330p' ably.d.tsRepository: ably/ably-js
Length of output: 513
🏁 Script executed:
#!/bin/bash
# Check if there have been recent changes to type definitions
git log -n 5 --oneline ably.d.tsRepository: ably/ably-js
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Try to compile the test file and see if there are actual TypeScript errors
npx tsc --noEmit test/uts/rest/integration/push_admin.test.ts 2>&1 | head -50Repository: ably/ably-js
Length of output: 7139
🏁 Script executed:
#!/bin/bash
# Search for any PushRecipient or similar union types that might be the intended type
rg -n 'type.*Recipient|interface.*Recipient' ably.d.ts -B 2 -A 5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Look for APNS or FCM recipient types
rg -n 'APNSRecipient|FCMRecipient|GCMRecipient' ably.d.ts -B 2 -A 5Repository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Search in TypeScript source files (not .d.ts) for potential type definitions
find src -name "*.ts" -type f | xargs grep -l "recipient" | head -5Repository: ably/ably-js
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Check the source file for device details type definitions
cat src/common/lib/types/devicedetails.tsRepository: ably/ably-js
Length of output: 3880
🏁 Script executed:
#!/bin/bash
# Also check push.ts for recipient-related types
cat src/common/lib/client/push.ts | head -100Repository: ably/ably-js
Length of output: 3426
🏁 Script executed:
#!/bin/bash
# Check if there's an APNS recipient type defined anywhere
rg -n 'apns|APNS' src/ -iRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check the full devicedetails.ts file to see all recipient types
cat src/common/lib/types/devicedetails.ts | grep -A 20 "interface.*Recipient"Repository: ably/ably-js
Length of output: 638
🏁 Script executed:
#!/bin/bash
# Look at the test to see what recipient type it's actually trying to use
sed -n '145,175p' test/uts/rest/integration/push_admin.test.tsRepository: ably/ably-js
Length of output: 1009
🏁 Script executed:
#!/bin/bash
# Search for APNSRecipient or any APNS-related type definitions
find . -name "*.ts" -o -name "*.d.ts" | xargs grep -l "APNSRecipient" 2>/dev/nullRepository: ably/ably-js
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the git diff or recent changes to devicedetails.ts to see if APNSRecipient was removed
git log --oneline -n 20 src/common/lib/types/devicedetails.tsRepository: ably/ably-js
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for transportType: 'apns' in source code
rg -n "transportType.*apns|apns.*transportType" -iRepository: ably/ably-js
Length of output: 2527
🏁 Script executed:
#!/bin/bash
# Search the commit that added the test to understand the intended APNS type
git show 2055b397 --stat | head -20Repository: ably/ably-js
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Check if APNSRecipient or similar type should exist
git show 2055b397 -- src/common/lib/types/devicedetails.ts | head -150Repository: ably/ably-js
Length of output: 4777
🏁 Script executed:
#!/bin/bash
# Check what transportType values the test expects to work
rg -n "transportType" test/uts/rest/integration/push_admin.test.tsRepository: ably/ably-js
Length of output: 776
🏁 Script executed:
#!/bin/bash
# Check what properties the test expects on APNS and GCM recipients
rg -n "transportType.*apns|transportType.*gcm" test/uts/rest/integration/push_admin.test.ts -A 1 -B 1 -iRepository: ably/ably-js
Length of output: 1139
🏁 Script executed:
#!/bin/bash
# Check if there are APNS or GCM recipient types defined elsewhere
find . -name "*.ts" -o -name "*.d.ts" | xargs grep -l "APNSRecipient\|GCMRecipient\|deviceToken" 2>/dev/null | grep -v node_modulesRepository: ably/ably-js
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Check the .d.ts exports to see if APNS/GCM types are exported
grep -n "APNSRecipient\|GCMRecipient\|registrationToken" ably.d.tsRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Let's verify what the union type currently includes and what properties it has
cat src/common/lib/types/devicedetails.ts | grep -A 50 "type PushRecipient"Repository: ably/ably-js
Length of output: 1779
🏁 Script executed:
#!/bin/bash
# Check if APNSRecipient or GCMRecipient should be added to the union
grep -r "interface.*APNSRecipient\|interface.*GCMRecipient" . --include="*.ts" --include="*.d.ts" 2>/dev/null | grep -v node_modulesRepository: ably/ably-js
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check what the spec says about recipient types by looking at the test description
head -20 test/uts/rest/integration/push_admin.test.tsRepository: ably/ably-js
Length of output: 503
Add missing APNS and GCM recipient types to the PushRecipient union.
Lines 162 and 166 access deviceToken on a recipient that is typed as PushRecipient, a union of WebPushRecipient | PushChannelRecipient. However, the test uses APNS and GCM recipients with deviceToken and registrationToken properties respectively. These types are missing from the union definition in src/common/lib/types/devicedetails.ts, causing TS2339 errors. Add APNSRecipient and GCMRecipient interfaces with their respective properties to the union:
interface APNSRecipient {
transportType: 'apns';
deviceToken: string;
}
interface GCMRecipient {
transportType: 'gcm';
registrationToken: string;
}
type PushRecipient = WebPushRecipient | PushChannelRecipient | APNSRecipient | GCMRecipient;Also update the ably.d.ts declaration to reflect the union type instead of any.
🧰 Tools
🪛 GitHub Actions: API Reference
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on 'WebPushRecipient').
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on 'WebPushRecipient').
🪛 GitHub Actions: Bundle size report
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
🪛 GitHub Actions: Lint
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
🪛 GitHub Actions: Spec coverage report
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
🪛 GitHub Actions: Test (react hooks)
[error] 162-162: TypeScript (TS2339): Property 'deviceToken' does not exist on type 'PushRecipient' (missing on WebPushRecipient).
[error] 166-166: TypeScript (TS2339): Property 'deviceToken' does not exist on type 'PushRecipient' (missing on WebPushRecipient).
🪛 GitHub Actions: Test browser
[error] 162-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (WebPushRecipient).
🪛 GitHub Actions: Test NodeJS
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (not on WebPushRecipient).
🪛 GitHub Actions: Test NPM package
[error] 162-162: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (missing on 'WebPushRecipient').
[error] 166-166: TS2339: Property 'deviceToken' does not exist on type 'PushRecipient' (missing on 'WebPushRecipient').
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/push_admin.test.ts` around lines 152 - 167, The
PushRecipient union in src/common/lib/types/devicedetails.ts is missing APNS and
GCM variants causing TS2339 when tests access deviceToken/registrationToken; add
APNSRecipient (transportType: 'apns', deviceToken: string) and GCMRecipient
(transportType: 'gcm', registrationToken: string) interfaces and include them in
the PushRecipient union (alongside WebPushRecipient and PushChannelRecipient),
and update the ably.d.ts declaration to use the new PushRecipient union instead
of any so typings reflect the added variants; update references in
functions/methods that consume PushRecipient accordingly (e.g.,
client.push.admin.deviceRegistrations types).
| const serverTimeThirtySecondsLater = serverTime + 30 * 1000; | ||
| expect(result.appliesAt).to.be.greaterThan(serverTimeThirtySecondsLater); | ||
| }); |
There was a problem hiding this comment.
Use non-strict boundary for appliesAt threshold to prevent flakiness.
greaterThan can fail on exact boundary values; this assertion should accept equality.
✅ Suggested change
- expect(result.appliesAt).to.be.greaterThan(serverTimeThirtySecondsLater);
+ expect(result.appliesAt).to.be.at.least(serverTimeThirtySecondsLater);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/revoke_tokens.test.ts` around lines 143 - 145, The
test currently asserts
expect(result.appliesAt).to.be.greaterThan(serverTimeThirtySecondsLater) which
can be flaky on exact boundaries; change this assertion to accept equality by
using the non-strict form (e.g.,
expect(result.appliesAt).to.be.at.least(serverTimeThirtySecondsLater) or the
equivalent gte/greaterThanOrEqual form) in the revoke_tokens.test.ts test where
serverTimeThirtySecondsLater and result.appliesAt are compared.
| endpoint: SANDBOX_ENDPOINT, | ||
| }); | ||
|
|
||
| const result = await client.stats(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "time_stats.test.ts" -type fRepository: ably/ably-js
Length of output: 103
🏁 Script executed:
cat -n ./test/uts/rest/integration/time_stats.test.ts | head -100Repository: ably/ably-js
Length of output: 3482
🏁 Script executed:
find . -name "*.ts" -not -path "./node_modules/*" -type f | xargs rg -l "stats.*:" | head -20Repository: ably/ably-js
Length of output: 233
🏁 Script executed:
rg "stats\s*\(" ./src/common/lib/client/rest.ts -A 5 -B 2Repository: ably/ably-js
Length of output: 431
🏁 Script executed:
rg "interface RequestParams|type RequestParams" -A 10Repository: ably/ably-js
Length of output: 682
Align stats calls with the typed API signature (current code does not compile).
Line 66 and Line 90 cause TS2554/TS2322 errors in CI. The stats method requires a params argument of type RequestParams (which is Record<string, string> | null), and all values including limit must be strings, not numbers.
Proposed fix
- const result = await client.stats();
+ const result = await client.stats({});
@@
- limit: 5,
+ limit: '5',
direction: 'forwards',
unit: 'hour',🧰 Tools
🪛 GitHub Actions: API Reference
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Bundle size report
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Lint
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Spec coverage report
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test (react hooks)
[error] 66-66: TypeScript (TS2554): Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test browser
[error] 66-90: Multiple TypeScript errors: TS2554 (expected 1 argument but got 0) at line 66; TS2322 (number not assignable to string) at line 90.
🪛 GitHub Actions: Test NodeJS
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🪛 GitHub Actions: Test NPM package
[error] 66-66: TS2554: Expected 1 arguments, but got 0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/uts/rest/integration/time_stats.test.ts` at line 66, The test calls to
client.stats() must be updated to match the typed signature requiring a
RequestParams argument (Record<string,string> | null); change the calls to pass
either null or a params object with string values (e.g., { limit: "100" })
instead of omitting the argument or passing numeric values; update both
occurrences referenced (the call at client.stats() and the other at line ~90) so
they pass valid RequestParams strings to satisfy the types.
1 participant
Summary
Test plan
npm run test:uts -- --grep 'uts/rest/integration' --timeout 60000— 74 passing, 2 pendingnpm run test:uts -- --grep 'uts/realtime/integration' --timeout 60000— 42 passingnpm run test:uts -- --timeout 30000— no regressions🤖 Generated with Claude Code
Summary by CodeRabbit