Skip to content

feat: Node.js SDK update for version 23.0.0#145

Merged
abnegate merged 9 commits intomainfrom
dev
Mar 26, 2026
Merged

feat: Node.js SDK update for version 23.0.0#145
abnegate merged 9 commits intomainfrom
dev

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Mar 24, 2026

This PR contains updates to the Node.js SDK for version 23.0.0.

* Updated `DocumentsDB` docs and `DatabasesIndexType` usage; bumped API version badge to 1.9.0
@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

Warning

Rate limit exceeded

@ChiragAgg5k has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d0f3f440-a9b2-48cb-9ca9-3b28b89a2887

📥 Commits

Reviewing files that changed from the base of the PR and between 566f848 and b9a5684.

📒 Files selected for processing (1)
  • CHANGELOG.md

Walkthrough

This pull request bumps the SDK version from 22.1.3 to 23.0.0 and updates API version support from 1.8.x to 1.9.x. It introduces breaking changes including a type change for $sequence from int to string in Row and Document models, and reorganizes index type enums (IndexTypeDatabasesIndexType for Databases, TablesDBIndexType for TablesDB). The specification parameter is replaced with separate buildSpecification, runtimeSpecification, and deploymentRetention fields in Functions and Sites services. Two new services are added: Project for managing project variables and Webhooks for managing webhooks. The Client is extended with impersonation methods (setImpersonateUserId, setImpersonateUserEmail, setImpersonateUserPhone). Five health queue-related methods are removed. Models are extended with new webhook types and additional fields for User, Site, and Function. A comprehensive Jest test suite is added covering all services.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: a Node.js SDK version bump to 23.0.0, which aligns with the extensive updates across version numbers, enums, services, and models throughout the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ChiragAgg5k ChiragAgg5k changed the title feat: Node.js SDK update for version 22.1.3 feat: Node.js SDK update for version 23.0.0 Mar 25, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🧹 Nitpick comments (21)
src/services/project.ts (1)

1-1: Remove unused import UploadProgress.

The UploadProgress type is imported but never used in this file.

🧹 Proposed fix
-import { AppwriteException, Client, type Payload, UploadProgress } from '../client';
+import { AppwriteException, Client, type Payload } from '../client';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/project.ts` at line 1, The import line in this module currently
includes an unused symbol UploadProgress; remove UploadProgress from the import
list (leaving AppwriteException, Client, and Payload) and verify there are no
remaining references to UploadProgress in this file so the import stays clean.
test/role.test.js (1)

10-13: Minor: Inconsistent semicolon usage.

Lines 10-13 are missing semicolons at the end of statements, while earlier lines (4-9) include them. Consider adding semicolons for consistency.

🔧 Proposed fix
-    test('team without role', () => expect(Role.team('custom')).toEqual('team:custom'))
-    test('team with role', () => expect(Role.team('custom', 'owner')).toEqual('team:custom/owner'))
-    test('member', () => expect(Role.member('custom')).toEqual('member:custom'))
-    test('label', () => expect(Role.label('admin')).toEqual('label:admin'))
+    test('team without role', () => expect(Role.team('custom')).toEqual('team:custom'));
+    test('team with role', () => expect(Role.team('custom', 'owner')).toEqual('team:custom/owner'));
+    test('member', () => expect(Role.member('custom')).toEqual('member:custom'));
+    test('label', () => expect(Role.label('admin')).toEqual('label:admin'));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/role.test.js` around lines 10 - 13, Add missing semicolons to the test
statements using Role.team, Role.member, and Role.label to match the existing
style: terminate the calls in test('team without role'), test('team with role'),
test('member'), and test('label') with semicolons so all test lines consistently
end with semicolons.
package.json (1)

51-51: Consider using exact version for jest.

The jest dependency uses a caret range (^29.7.0) while other devDependencies use exact versions (e.g., "tsup": "7.2.0", "typescript": "5.4.2"). For consistent builds, consider pinning to an exact version.

🔧 Proposed fix
-    "jest": "^29.7.0"
+    "jest": "29.7.0"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 51, The jest dependency in package.json is using a
caret range ("jest": "^29.7.0") while other devDependencies are pinned; update
the "jest" entry to an exact version (e.g., "jest": "29.7.0") to ensure
consistent installs and reproducible builds, keeping the rest of the
devDependencies (like "tsup" and "typescript") unchanged.
docs/examples/databases/upsert-documents.md (1)

7-7: Use a named placeholder instead of an empty session string.

At Line 7, consider setSession('<YOUR_SESSION_ID>') for consistency with other examples and to avoid implying empty-string is a valid value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/databases/upsert-documents.md` at line 7, The example uses an
empty string in the setSession call which implies an empty session is valid;
replace .setSession('') with a named placeholder like
.setSession('<YOUR_SESSION_ID>') in the snippet (locate the setSession call) so
examples are consistent and clearly indicate the developer must supply a session
ID.
test/services/webhooks.test.js (3)

45-50: Consider using a valid placeholder URL for the test.

The create() method is called with an empty string '' for the URL parameter (line 47), while other string parameters use descriptive placeholders like '<WEBHOOK_ID>' and '<NAME>'. Using a placeholder like 'https://example.com/webhook' would be more consistent with the mock response data and other test patterns.

Suggested fix
         const response = await webhooks.create(
             '<WEBHOOK_ID>',
-            '',
+            'https://example.com/webhook',
             '<NAME>',
             [],
         );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/webhooks.test.js` around lines 45 - 50, The test calls
webhooks.create with an empty URL string; replace the second argument in the
webhooks.create call with a valid placeholder URL (e.g.,
"https://example.com/webhook") so the test input aligns with other descriptive
placeholders and the mock response data; update the webhooks.create invocation
in test/services/webhooks.test.js accordingly.

102-107: Consider using a valid placeholder URL for the update test as well.

Similar to the create() test, the update() method passes an empty string for the URL parameter.

Suggested fix
         const response = await webhooks.update(
             '<WEBHOOK_ID>',
             '<NAME>',
-            '',
+            'https://example.com/webhook',
             [],
         );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/webhooks.test.js` around lines 102 - 107, The update test calls
webhooks.update('<WEBHOOK_ID>', '<NAME>', '', []) with an empty string for the
URL; change it to use a valid placeholder URL (e.g.
'https://example.com/webhook' or the same placeholder used in the create() test)
so webhooks.update receives a proper URL parameter and the test mirrors
create()'s behavior.

2-2: Unused import.

Same as project.test.js - InputFile is not used.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/webhooks.test.js` at line 2, Remove the unused require of
InputFile in webhooks.test.js: the symbol InputFile is imported via const {
InputFile } = require("../../dist/inputFile"); but never used, so either remove
that import line or replace it with the correct dependency actually needed by
the tests; ensure no other references to InputFile remain in the file (search
for InputFile) before committing the removal.
test/services/project.test.js (1)

1-6: Remove unused InputFile import.

InputFile is imported but not used in this test file. This pattern is repeated across all new test files in this PR.

Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Project } = require("../../dist/services/project");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/project.test.js` around lines 1 - 6, Remove the unused
InputFile import from the test file: delete the `const { InputFile } =
require("../../dist/inputFile");` line in test/services/project.test.js (and
similarly remove that import from the other new test files in this PR) so only
required symbols like `Client` and `Project` remain; ensure no other references
to InputFile exist before committing.
test/services/graphql.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/graphql.test.js` at line 2, The import of InputFile is unused
in test/services/graphql.test.js; remove the line "const { InputFile } =
require(\"../../dist/inputFile\");" (or otherwise use InputFile in tests) so
there are no stale/unused imports referencing InputFile.
test/services/activities.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/activities.test.js` at line 2, The test file imports InputFile
but never uses it; remove the unused import statement referencing InputFile from
the require call (or alternatively use InputFile in the test if intended) to
eliminate the dead import and satisfy linting—look for the require(...) that
assigns InputFile and delete that binding or add meaningful usage of InputFile
in the tests (e.g., constructing an InputFile instance where appropriate).
test/services/teams.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/teams.test.js` at line 2, Remove the unused import of InputFile
from the test file: delete or comment out the require line that imports
InputFile (const { InputFile } = require("../../dist/inputFile");) in
test/services/teams.test.js so only used dependencies remain; if that import was
intended for future tests, either use it in an assertion or add a TODO comment
linking the upcoming test instead of leaving it unused.
test/services/locale.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/locale.test.js` at line 2, The test file imports InputFile but
never uses it; remove the unused import statement (the require of InputFile)
from the top of the test so there are no unused symbols; search for the const {
InputFile } declaration in the test and delete that line, keeping other imports
intact.
test/services/tokens.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/tokens.test.js` at line 2, The test imports InputFile but never
uses it; remove the unused import statement "const { InputFile } =
require(\"../../dist/inputFile\");" from the test (or, if InputFile was intended
to be used, replace the unused import by actually instantiating or referencing
InputFile in the test code) so there are no unused symbols left.
test/services/avatars.test.js (1)

2-2: Unused import.

InputFile is not used in this test file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/avatars.test.js` at line 2, The import of InputFile in the test
(the require line "const { InputFile } = require(\"../../dist/inputFile\");") is
unused; remove the InputFile import (or the entire require statement if nothing
else is imported from that module) so the test file no longer contains the
unused symbol InputFile and lints cleanly.
test/services/account.test.js (1)

2-2: Unused import.

InputFile is imported but never used in this test file.

🔧 Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Account } = require("../../dist/services/account");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/account.test.js` at line 2, The import of InputFile in
test/services/account.test.js is unused; remove the import statement "const {
InputFile } = require(\"../../dist/inputFile\");" (or the specific InputFile
binding) so the file no longer imports InputFile unnecessarily and the test file
only contains required modules.
test/services/messaging.test.js (1)

2-2: Unused import.

InputFile is imported but never used in this test file.

🔧 Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Messaging } = require("../../dist/services/messaging");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/messaging.test.js` at line 2, The import of InputFile in the
test file is unused; either remove the line "const { InputFile } =
require(\"../../dist/inputFile\");" or use InputFile in the tests if it was
intended, e.g., reference the InputFile symbol in the test cases that need it;
update test/services/messaging.test.js to eliminate the unused import to satisfy
linter and remove dead code.
test/services/backups.test.js (1)

2-2: Unused import.

InputFile is imported but never used in this test file.

🔧 Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Backups } = require("../../dist/services/backups");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/backups.test.js` at line 2, Remove the unused import of
InputFile: locate the top-level require for InputFile (const { InputFile } =
require("../../dist/inputFile");) in the test file and delete it, or if the test
should exercise InputFile instead, replace the unused import by actually
instantiating/using InputFile in the relevant test cases (e.g., create a new
InputFile(...) or call a method on InputFile). Ensure no other references remain
to InputFile after the change.
test/services/health.test.js (1)

2-2: Unused import.

InputFile is imported but never used in this test file.

🔧 Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Health } = require("../../dist/services/health");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/health.test.js` at line 2, The test imports InputFile but never
uses it; remove the unused require line for InputFile (const { InputFile } =
require(...)) from the test file or, if the test should exercise InputFile
behavior, replace the unused import by actually instantiating/using InputFile in
the relevant test cases (e.g., create an InputFile instance and assert expected
behavior). Ensure no other references to InputFile remain after removal to avoid
lint/test failures.
test/services/databases.test.js (1)

2-2: Unused import.

InputFile is imported but never used in this test file.

🔧 Suggested fix
 const { Client } = require("../../dist/client");
-const { InputFile } = require("../../dist/inputFile");
 const { Databases } = require("../../dist/services/databases");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/databases.test.js` at line 2, Remove the unused import
InputFile from the test file: delete the require line that imports InputFile
(const { InputFile } = require(...)) so the test no longer includes an unused
symbol; if the import was intended for a test case, instead reference InputFile
where needed in the test functions (e.g., in any describe/it blocks) or add a
focused test that uses InputFile, otherwise simply remove the import to
eliminate the unused dependency.
test/services/users.test.js (1)

772-775: Use a valid password fixture in updatePassword() test.

Passing an empty string here diverges from the documented minimum length and weakens contract-level signal from this test.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/users.test.js` around lines 772 - 775, The test calls
users.updatePassword('<USER_ID>', '') with an empty string which violates the
documented minimum password length; replace the empty string with a valid
password fixture that meets the minimum requirements (e.g., use an existing test
fixture like VALID_PASSWORD, DEFAULT_PASSWORD, or create a strong fixture such
as 'Password123!'), ensuring users.updatePassword is exercised with a
realistic/valid password value so the test reflects the contract-level password
constraints.
test/services/sites.test.js (1)

22-23: Extract response normalization into a helper.

delete response.toString is repeated throughout the suite. Centralizing this in one helper will reduce noise and future maintenance churn.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/sites.test.js` around lines 22 - 23, Extract the repeated
"delete response.toString" into a reusable test helper (e.g., normalizeResponse
or stripToString) and replace all inline occurrences with calls to that helper;
implement the helper to accept the response object, check for and delete the
toString property (or return a shallow clone without it) and export/import or
require it into the test files (replace usages in tests like sites.test.js where
delete response.toString currently appears) so all tests call
normalizeResponse(response) instead of repeating the delete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 5-6: The 23.0.0 changelog is too terse and misses several
breaking/public API changes; expand the release notes to enumerate all breaking
changes and migration steps (include the `$sequence` type change from int→string
and any payload field migrations), list removed/renamed methods and their
replacements, document new services added, update examples showing how to
migrate clients that reference DatabasesIndexType, and ensure the API version
badge (1.9.0) is consistent with the documented changes; reference the symbols
`$sequence`, `DatabasesIndexType`, and any removed method names in the notes so
integrators can perform required code updates.

In `@src/client.ts`:
- Around line 278-309: When setting an impersonation selector in
setImpersonateUserId, setImpersonateUserEmail, or setImpersonateUserPhone, clear
the other impersonation headers and config fields so stale selectors aren't sent
on subsequent requests; e.g., in setImpersonateUserId(value) remove
'X-Appwrite-Impersonate-User-Email' and 'X-Appwrite-Impersonate-User-Phone' from
this.headers and clear config.impersonateuseremail and
config.impersonateuserphone (and similarly for the other two methods), then set
the intended header and config field and return this.
- Around line 456-458: The current code assigns data.toString directly which
creates an own enumerable property and alters response shapes; instead, when
data && typeof data === 'object' and NOT Array.isArray(data) and NOT (data
instanceof ArrayBuffer), add a non-enumerable toString using
Object.defineProperty on the data object (define value as a function that
returns JSONbig.stringify(data), set enumerable: false, configurable: true) so
consumers and tests aren't affected by an enumerable property; ensure you still
only run this for object-like responses.

In `@src/models.ts`:
- Around line 3869-3883: The JSDoc for the new webhook field ends mid-sentence:
update the comment for the property signatureKey (near security, httpUser,
httpPass) to a complete, user-facing description such as "Signature key used to
validate incoming webhook requests" or similar concise wording that clarifies
its purpose and usage so generated API docs are clear and grammatically correct.

In `@src/services/webhooks.ts`:
- Around line 31-44: The overload for list(...) mishandles calls like
list(undefined, false) because the current object-branch catches undefined
first-arg and discards the positional rest; update the branching in the list
function to detect the positional-overload case when paramsOrFirst === undefined
and rest.length > 0 (or when paramsOrFirst is an array), and in that case set
params = { queries: paramsOrFirst as string[] | undefined, total: rest[0] as
boolean }; otherwise keep the object-style cast for paramsOrFirst as { queries?:
string[], total?: boolean } so optional positional total is preserved. Ensure
you reference the list function and the paramsOrFirst/rest variables when making
the change.

In `@test/query.test.js`:
- Around line 9-13: Update the test case description string 'with a integer' to
correct grammar: change it to 'with an integer' in the test entry (the object
with properties description, value, expectedValues) so the test reads "with an
integer".

In `@test/services/functions.test.js`:
- Around line 28-71: The test currently only checks the mocked response and not
what the client sends; modify the create() (and similarly update()) test to
assert the outgoing request body by inspecting mockedFetch.mock.calls: parse the
request body JSON from the first call and assert it contains the new 23.0.0
fields (buildSpecification, runtimeSpecification, deploymentRetention) with the
non-default values you passed (e.g., use explicit non-default strings/numbers
rather than defaults) and that runtimeSpecification matches the provided runtime
(e.g., 'node-14.5'), so the test fails if the client still sends the old
specification or omits the new fields; apply the same assertion pattern to the
update() test block referenced at lines 148-190.

In `@test/services/sites.test.js`:
- Around line 64-69: The tests for sites.create and sites.update only assert
mocked responses and must also assert the actual outbound request body to catch
serialization regressions: update the tests around sites.create and the similar
sites.update case to capture/intercept the HTTP request (e.g., via the existing
mock HTTP client or nock) and assert that the sent JSON includes correctly
serialized buildSpecification, runtimeSpecification, deploymentRetention, and
startCommand fields (and their expected structure/keys/values). Reference the
sites.create and sites.update calls and assert the outbound payload contents
rather than only the response to ensure request-body serialization is validated.

In `@test/services/tables-d-b.test.js`:
- Around line 1263-1287: The test for TablesDB.createIndex is only asserting
response passthrough; add assertions to validate the actual network request made
by createIndex by inspecting mockedFetch.mock.calls[0] to ensure the request
method, URL/path and body match the expected payload (e.g., verify the JSON body
contains { type: 'key', columns: ['key'] } or the correct fields you expect from
the createIndex call) and that headers/content-type are correct; update the test
around the createIndex invocation to parse/mock the first mock call and assert
its request body and method before asserting the response equality.

In `@test/services/users.test.js`:
- Around line 414-417: Add a new unit test that exercises
users.updateImpersonator with the boolean false to ensure falsy values are
serialized; replicate the existing true-case test but call
users.updateImpersonator('<USER_ID>', false) and assert the outgoing
request/payload (or mocked API call) contains impersonator: false (not omitted
or undefined). Place this test alongside the existing true-branch test and use
the same mocking/expectation helpers to verify the request body and response
handling for the false case.
- Line 427: Replace the secret-like JWT literal used in the test fixture (the
'jwt' property shown in users.test.js) with a non-secret placeholder or a
clearly synthetic token (e.g. "TEST_JWT_TOKEN" or a token generated from a
test-only helper), and apply the same replacement to the other token-shaped
strings reported around lines 868-870; ensure tests still assert expected
behavior by using the placeholder or a test helper function (e.g.,
makeTestToken()) rather than hard-coded real-looking JWT strings.

---

Nitpick comments:
In `@docs/examples/databases/upsert-documents.md`:
- Line 7: The example uses an empty string in the setSession call which implies
an empty session is valid; replace .setSession('') with a named placeholder like
.setSession('<YOUR_SESSION_ID>') in the snippet (locate the setSession call) so
examples are consistent and clearly indicate the developer must supply a session
ID.

In `@package.json`:
- Line 51: The jest dependency in package.json is using a caret range ("jest":
"^29.7.0") while other devDependencies are pinned; update the "jest" entry to an
exact version (e.g., "jest": "29.7.0") to ensure consistent installs and
reproducible builds, keeping the rest of the devDependencies (like "tsup" and
"typescript") unchanged.

In `@src/services/project.ts`:
- Line 1: The import line in this module currently includes an unused symbol
UploadProgress; remove UploadProgress from the import list (leaving
AppwriteException, Client, and Payload) and verify there are no remaining
references to UploadProgress in this file so the import stays clean.

In `@test/role.test.js`:
- Around line 10-13: Add missing semicolons to the test statements using
Role.team, Role.member, and Role.label to match the existing style: terminate
the calls in test('team without role'), test('team with role'), test('member'),
and test('label') with semicolons so all test lines consistently end with
semicolons.

In `@test/services/account.test.js`:
- Line 2: The import of InputFile in test/services/account.test.js is unused;
remove the import statement "const { InputFile } =
require(\"../../dist/inputFile\");" (or the specific InputFile binding) so the
file no longer imports InputFile unnecessarily and the test file only contains
required modules.

In `@test/services/activities.test.js`:
- Line 2: The test file imports InputFile but never uses it; remove the unused
import statement referencing InputFile from the require call (or alternatively
use InputFile in the test if intended) to eliminate the dead import and satisfy
linting—look for the require(...) that assigns InputFile and delete that binding
or add meaningful usage of InputFile in the tests (e.g., constructing an
InputFile instance where appropriate).

In `@test/services/avatars.test.js`:
- Line 2: The import of InputFile in the test (the require line "const {
InputFile } = require(\"../../dist/inputFile\");") is unused; remove the
InputFile import (or the entire require statement if nothing else is imported
from that module) so the test file no longer contains the unused symbol
InputFile and lints cleanly.

In `@test/services/backups.test.js`:
- Line 2: Remove the unused import of InputFile: locate the top-level require
for InputFile (const { InputFile } = require("../../dist/inputFile");) in the
test file and delete it, or if the test should exercise InputFile instead,
replace the unused import by actually instantiating/using InputFile in the
relevant test cases (e.g., create a new InputFile(...) or call a method on
InputFile). Ensure no other references remain to InputFile after the change.

In `@test/services/databases.test.js`:
- Line 2: Remove the unused import InputFile from the test file: delete the
require line that imports InputFile (const { InputFile } = require(...)) so the
test no longer includes an unused symbol; if the import was intended for a test
case, instead reference InputFile where needed in the test functions (e.g., in
any describe/it blocks) or add a focused test that uses InputFile, otherwise
simply remove the import to eliminate the unused dependency.

In `@test/services/graphql.test.js`:
- Line 2: The import of InputFile is unused in test/services/graphql.test.js;
remove the line "const { InputFile } = require(\"../../dist/inputFile\");" (or
otherwise use InputFile in tests) so there are no stale/unused imports
referencing InputFile.

In `@test/services/health.test.js`:
- Line 2: The test imports InputFile but never uses it; remove the unused
require line for InputFile (const { InputFile } = require(...)) from the test
file or, if the test should exercise InputFile behavior, replace the unused
import by actually instantiating/using InputFile in the relevant test cases
(e.g., create an InputFile instance and assert expected behavior). Ensure no
other references to InputFile remain after removal to avoid lint/test failures.

In `@test/services/locale.test.js`:
- Line 2: The test file imports InputFile but never uses it; remove the unused
import statement (the require of InputFile) from the top of the test so there
are no unused symbols; search for the const { InputFile } declaration in the
test and delete that line, keeping other imports intact.

In `@test/services/messaging.test.js`:
- Line 2: The import of InputFile in the test file is unused; either remove the
line "const { InputFile } = require(\"../../dist/inputFile\");" or use InputFile
in the tests if it was intended, e.g., reference the InputFile symbol in the
test cases that need it; update test/services/messaging.test.js to eliminate the
unused import to satisfy linter and remove dead code.

In `@test/services/project.test.js`:
- Around line 1-6: Remove the unused InputFile import from the test file: delete
the `const { InputFile } = require("../../dist/inputFile");` line in
test/services/project.test.js (and similarly remove that import from the other
new test files in this PR) so only required symbols like `Client` and `Project`
remain; ensure no other references to InputFile exist before committing.

In `@test/services/sites.test.js`:
- Around line 22-23: Extract the repeated "delete response.toString" into a
reusable test helper (e.g., normalizeResponse or stripToString) and replace all
inline occurrences with calls to that helper; implement the helper to accept the
response object, check for and delete the toString property (or return a shallow
clone without it) and export/import or require it into the test files (replace
usages in tests like sites.test.js where delete response.toString currently
appears) so all tests call normalizeResponse(response) instead of repeating the
delete.

In `@test/services/teams.test.js`:
- Line 2: Remove the unused import of InputFile from the test file: delete or
comment out the require line that imports InputFile (const { InputFile } =
require("../../dist/inputFile");) in test/services/teams.test.js so only used
dependencies remain; if that import was intended for future tests, either use it
in an assertion or add a TODO comment linking the upcoming test instead of
leaving it unused.

In `@test/services/tokens.test.js`:
- Line 2: The test imports InputFile but never uses it; remove the unused import
statement "const { InputFile } = require(\"../../dist/inputFile\");" from the
test (or, if InputFile was intended to be used, replace the unused import by
actually instantiating or referencing InputFile in the test code) so there are
no unused symbols left.

In `@test/services/users.test.js`:
- Around line 772-775: The test calls users.updatePassword('<USER_ID>', '') with
an empty string which violates the documented minimum password length; replace
the empty string with a valid password fixture that meets the minimum
requirements (e.g., use an existing test fixture like VALID_PASSWORD,
DEFAULT_PASSWORD, or create a strong fixture such as 'Password123!'), ensuring
users.updatePassword is exercised with a realistic/valid password value so the
test reflects the contract-level password constraints.

In `@test/services/webhooks.test.js`:
- Around line 45-50: The test calls webhooks.create with an empty URL string;
replace the second argument in the webhooks.create call with a valid placeholder
URL (e.g., "https://example.com/webhook") so the test input aligns with other
descriptive placeholders and the mock response data; update the webhooks.create
invocation in test/services/webhooks.test.js accordingly.
- Around line 102-107: The update test calls webhooks.update('<WEBHOOK_ID>',
'<NAME>', '', []) with an empty string for the URL; change it to use a valid
placeholder URL (e.g. 'https://example.com/webhook' or the same placeholder used
in the create() test) so webhooks.update receives a proper URL parameter and the
test mirrors create()'s behavior.
- Line 2: Remove the unused require of InputFile in webhooks.test.js: the symbol
InputFile is imported via const { InputFile } = require("../../dist/inputFile");
but never used, so either remove that import line or replace it with the correct
dependency actually needed by the tests; ensure no other references to InputFile
remain in the file (search for InputFile) before committing the removal.
🪄 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: 58057121-15f5-4232-a032-c90ee5f8d118

📥 Commits

Reviewing files that changed from the base of the PR and between ae25f4a and 566f848.

📒 Files selected for processing (62)
  • CHANGELOG.md
  • README.md
  • docs/examples/databases/create-index.md
  • docs/examples/databases/upsert-documents.md
  • docs/examples/functions/create.md
  • docs/examples/functions/update.md
  • docs/examples/project/create-variable.md
  • docs/examples/project/delete-variable.md
  • docs/examples/project/get-variable.md
  • docs/examples/project/list-variables.md
  • docs/examples/project/update-variable.md
  • docs/examples/sites/create.md
  • docs/examples/sites/update.md
  • docs/examples/tablesdb/create-index.md
  • docs/examples/users/update-impersonator.md
  • docs/examples/webhooks/create.md
  • docs/examples/webhooks/delete.md
  • docs/examples/webhooks/get.md
  • docs/examples/webhooks/list.md
  • docs/examples/webhooks/update-signature.md
  • docs/examples/webhooks/update.md
  • package.json
  • src/client.ts
  • src/enums/backup-services.ts
  • src/enums/database-type.ts
  • src/enums/databases-index-type.ts
  • src/enums/scopes.ts
  • src/enums/tables-db-index-type.ts
  • src/enums/template-reference-type.ts
  • src/index.ts
  • src/models.ts
  • src/services/databases.ts
  • src/services/functions.ts
  • src/services/health.ts
  • src/services/project.ts
  • src/services/sites.ts
  • src/services/tables-db.ts
  • src/services/users.ts
  • src/services/webhooks.ts
  • test/id.test.js
  • test/operator.test.js
  • test/permission.test.js
  • test/query.test.js
  • test/role.test.js
  • test/services/account.test.js
  • test/services/activities.test.js
  • test/services/avatars.test.js
  • test/services/backups.test.js
  • test/services/databases.test.js
  • test/services/functions.test.js
  • test/services/graphql.test.js
  • test/services/health.test.js
  • test/services/locale.test.js
  • test/services/messaging.test.js
  • test/services/project.test.js
  • test/services/sites.test.js
  • test/services/storage.test.js
  • test/services/tables-d-b.test.js
  • test/services/teams.test.js
  • test/services/tokens.test.js
  • test/services/users.test.js
  • test/services/webhooks.test.js
💤 Files with no reviewable changes (1)
  • src/services/health.ts

Comment on lines +278 to +309
setImpersonateUserId(value: string): this {
this.headers['X-Appwrite-Impersonate-User-Id'] = value;
this.config.impersonateuserid = value;
return this;
}
/**
* Set ImpersonateUserEmail
*
* Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.
*
* @param value string
*
* @return {this}
*/
setImpersonateUserEmail(value: string): this {
this.headers['X-Appwrite-Impersonate-User-Email'] = value;
this.config.impersonateuseremail = value;
return this;
}
/**
* Set ImpersonateUserPhone
*
* Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.
*
* @param value string
*
* @return {this}
*/
setImpersonateUserPhone(value: string): this {
this.headers['X-Appwrite-Impersonate-User-Phone'] = value;
this.config.impersonateuserphone = value;
return this;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Impersonation setters leave stale selectors on the client.

If one request uses setImpersonateUserId() and the next uses setImpersonateUserEmail(), the same Client instance will send both headers. That makes the impersonated target ambiguous across successive calls.

Suggested fix
+    private clearImpersonation(): void {
+        delete this.headers['X-Appwrite-Impersonate-User-Id'];
+        delete this.headers['X-Appwrite-Impersonate-User-Email'];
+        delete this.headers['X-Appwrite-Impersonate-User-Phone'];
+        this.config.impersonateuserid = '';
+        this.config.impersonateuseremail = '';
+        this.config.impersonateuserphone = '';
+    }
+
     setImpersonateUserId(value: string): this {
+        this.clearImpersonation();
         this.headers['X-Appwrite-Impersonate-User-Id'] = value;
         this.config.impersonateuserid = value;
         return this;
     }
...
     setImpersonateUserEmail(value: string): this {
+        this.clearImpersonation();
         this.headers['X-Appwrite-Impersonate-User-Email'] = value;
         this.config.impersonateuseremail = value;
         return this;
     }
...
     setImpersonateUserPhone(value: string): this {
+        this.clearImpersonation();
         this.headers['X-Appwrite-Impersonate-User-Phone'] = value;
         this.config.impersonateuserphone = value;
         return this;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client.ts` around lines 278 - 309, When setting an impersonation selector
in setImpersonateUserId, setImpersonateUserEmail, or setImpersonateUserPhone,
clear the other impersonation headers and config fields so stale selectors
aren't sent on subsequent requests; e.g., in setImpersonateUserId(value) remove
'X-Appwrite-Impersonate-User-Email' and 'X-Appwrite-Impersonate-User-Phone' from
this.headers and clear config.impersonateuseremail and
config.impersonateuserphone (and similarly for the other two methods), then set
the intended header and config field and return this.

Comment on lines +456 to +458
if (data && typeof data === 'object') {
data.toString = () => JSONbig.stringify(data);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In JavaScript, when you assign obj.toString = fn, is toStringadded as an own enumerable property by default, and how canObject.defineProperty define it as non-enumerable?

💡 Result:

Yes, assigning obj.toString = fn adds toString as an own enumerable property by default. Properties created via simple assignment have the enumerable flag set to true by default. To define it as non-enumerable using Object.defineProperty, use: Object.defineProperty(obj, 'toString', { value: fn, enumerable: false, // explicitly set to false (defaults to false anyway) writable: true, // optional, depending on needs configurable: true // optional }); This creates or modifies the toString property as own but non-enumerable, so it won't appear in for...in loops, Object.keys, etc., but remains accessible via direct property access.

Citations:


🏁 Script executed:

# First, let's see the actual code context around lines 456-458 in src/client.ts
head -n 470 src/client.ts | tail -n 30

Repository: appwrite/sdk-for-node

Length of output: 1100


🏁 Script executed:

# Search for tests that delete response.toString to verify the claim
rg "delete.*toString" --type ts --type js -C 3

Repository: appwrite/sdk-for-node

Length of output: 50378


🏁 Script executed:

# Look for any other places where toString is being assigned to objects
rg "\.toString\s*=" --type ts --type js -C 2

Repository: appwrite/sdk-for-node

Length of output: 253


Remove the enumerable toString property from response objects.

Assigning data.toString = fn adds toString as an own enumerable property, which changes the observable shape of every object-like response. This breaks tests (which must delete response.toString before assertions) and affects consumers who spread or enumerate SDK responses.

Fix by using Object.defineProperty with enumerable: false and exclude arrays and ArrayBuffer:

Suggested fix
-        if (data && typeof data === 'object') {
-            data.toString = () => JSONbig.stringify(data);
-        }
+        if (data && typeof data === 'object' && !Array.isArray(data) && !(data instanceof ArrayBuffer)) {
+            Object.defineProperty(data, 'toString', {
+                value: () => JSONbig.stringify(data),
+                configurable: true,
+                enumerable: false,
+            });
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (data && typeof data === 'object') {
data.toString = () => JSONbig.stringify(data);
}
if (data && typeof data === 'object' && !Array.isArray(data) && !(data instanceof ArrayBuffer)) {
Object.defineProperty(data, 'toString', {
value: () => JSONbig.stringify(data),
configurable: true,
enumerable: false,
});
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client.ts` around lines 456 - 458, The current code assigns data.toString
directly which creates an own enumerable property and alters response shapes;
instead, when data && typeof data === 'object' and NOT Array.isArray(data) and
NOT (data instanceof ArrayBuffer), add a non-enumerable toString using
Object.defineProperty on the data object (define value as a function that
returns JSONbig.stringify(data), set enumerable: false, configurable: true) so
consumers and tests aren't affected by an enumerable property; ensure you still
only run this for object-like responses.

Comment on lines +3869 to +3883
* Indicated if SSL / TLS Certificate verification is enabled.
*/
security: boolean;
/**
* HTTP basic authentication username.
*/
httpUser: string;
/**
* HTTP basic authentication password.
*/
httpPass: string;
/**
* Signature key which can be used to validated incoming
*/
signatureKey: string;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tighten the new webhook field docs.

These comments are user-facing in generated API docs, and signatureKey currently ends mid-sentence.

✏️ Suggested fix
         /**
-         * Indicated if SSL / TLS Certificate verification is enabled.
+         * Indicates whether SSL/TLS certificate verification is enabled.
          */
         security: boolean;
@@
         /**
-         * Signature key which can be used to validated incoming
+         * Signature key used to validate incoming webhook deliveries.
          */
         signatureKey: string;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* Indicated if SSL / TLS Certificate verification is enabled.
*/
security: boolean;
/**
* HTTP basic authentication username.
*/
httpUser: string;
/**
* HTTP basic authentication password.
*/
httpPass: string;
/**
* Signature key which can be used to validated incoming
*/
signatureKey: string;
* Indicates whether SSL/TLS certificate verification is enabled.
*/
security: boolean;
/**
* HTTP basic authentication username.
*/
httpUser: string;
/**
* HTTP basic authentication password.
*/
httpPass: string;
/**
* Signature key used to validate incoming webhook deliveries.
*/
signatureKey: string;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/models.ts` around lines 3869 - 3883, The JSDoc for the new webhook field
ends mid-sentence: update the comment for the property signatureKey (near
security, httpUser, httpPass) to a complete, user-facing description such as
"Signature key used to validate incoming webhook requests" or similar concise
wording that clarifies its purpose and usage so generated API docs are clear and
grammatically correct.

Comment on lines +31 to +44
list(
paramsOrFirst?: { queries?: string[], total?: boolean } | string[],
...rest: [(boolean)?]
): Promise<Models.WebhookList> {
let params: { queries?: string[], total?: boolean };

if (!paramsOrFirst || (paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst))) {
params = (paramsOrFirst || {}) as { queries?: string[], total?: boolean };
} else {
params = {
queries: paramsOrFirst as string[],
total: rest[0] as boolean
};
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle skipped first args in the deprecated list overload.

webhooks.list(undefined, false) currently falls into the object-style branch and drops total, so the positional overload can't set later optional arguments unless queries is also provided.

💡 Suggested fix
-        if (!paramsOrFirst || (paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst))) {
+        if (rest.length === 0 && (!paramsOrFirst || (typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst)))) {
             params = (paramsOrFirst || {}) as { queries?: string[], total?: boolean };
         } else {
             params = {
-                queries: paramsOrFirst as string[],
+                queries: paramsOrFirst as string[] | undefined,
                 total: rest[0] as boolean            
             };
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/webhooks.ts` around lines 31 - 44, The overload for list(...)
mishandles calls like list(undefined, false) because the current object-branch
catches undefined first-arg and discards the positional rest; update the
branching in the list function to detect the positional-overload case when
paramsOrFirst === undefined and rest.length > 0 (or when paramsOrFirst is an
array), and in that case set params = { queries: paramsOrFirst as string[] |
undefined, total: rest[0] as boolean }; otherwise keep the object-style cast for
paramsOrFirst as { queries?: string[], total?: boolean } so optional positional
total is preserved. Ensure you reference the list function and the
paramsOrFirst/rest variables when making the change.

Comment on lines +28 to +71
test('test method create()', async () => {
const data = {
'\$id': '5e5ea5c16897e',
'\$createdAt': '2020-10-15T06:38:00.000+00:00',
'\$updatedAt': '2020-10-15T06:38:00.000+00:00',
'execute': [],
'name': 'My Function',
'enabled': true,
'live': true,
'logging': true,
'runtime': 'python-3.8',
'deploymentRetention': 7,
'deploymentId': '5e5ea5c16897e',
'deploymentCreatedAt': '2020-10-15T06:38:00.000+00:00',
'latestDeploymentId': '5e5ea5c16897e',
'latestDeploymentCreatedAt': '2020-10-15T06:38:00.000+00:00',
'latestDeploymentStatus': 'ready',
'scopes': [],
'vars': [],
'events': [],
'schedule': '5 4 * * *',
'timeout': 300,
'entrypoint': 'index.js',
'commands': 'npm install',
'version': 'v2',
'installationId': '6m40at4ejk5h2u9s1hboo',
'providerRepositoryId': 'appwrite',
'providerBranch': 'main',
'providerRootDirectory': 'functions/helloWorld',
'providerSilentMode': true,
'buildSpecification': 's-1vcpu-512mb',
'runtimeSpecification': 's-1vcpu-512mb',};
mockedFetch.mockImplementation(() => Response.json(data));

const response = await functions.create(
'<FUNCTION_ID>',
'<NAME>',
'node-14.5',
);

// Remove custom toString method on the objects to allow for clean data comparison.
delete response.toString;

expect(response).toEqual(data);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

create() / update() still don't exercise the 23.0.0 payload change.

create() already shows the problem: it sends 'node-14.5' but accepts a mocked 'python-3.8' response, so these tests only validate passthrough. They would still pass if the client kept sending the legacy specification field or omitted buildSpecification, runtimeSpecification, and deploymentRetention entirely. Please assert the outgoing mockedFetch body with non-default values for those new fields.

Also applies to: 148-190

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/functions.test.js` around lines 28 - 71, The test currently
only checks the mocked response and not what the client sends; modify the
create() (and similarly update()) test to assert the outgoing request body by
inspecting mockedFetch.mock.calls: parse the request body JSON from the first
call and assert it contains the new 23.0.0 fields (buildSpecification,
runtimeSpecification, deploymentRetention) with the non-default values you
passed (e.g., use explicit non-default strings/numbers rather than defaults) and
that runtimeSpecification matches the provided runtime (e.g., 'node-14.5'), so
the test fails if the client still sends the old specification or omits the new
fields; apply the same assertion pattern to the update() test block referenced
at lines 148-190.

Comment on lines +64 to +69
const response = await sites.create(
'<SITE_ID>',
'<NAME>',
'analog',
'node-14.5',
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Assert outbound payload for create() / update() to cover the actual breaking change.

These tests currently validate only mocked responses, so regressions in request body serialization for buildSpecification, runtimeSpecification, deploymentRetention, and startCommand would go undetected.

Suggested test hardening
 test('test method create()', async () => {
   const data = { ... };
   mockedFetch.mockImplementation(() => Response.json(data));

   const response = await sites.create(
     '<SITE_ID>',
     '<NAME>',
     'analog',
     'node-14.5',
   );

+  const [, init] = mockedFetch.mock.calls[0];
+  const body = JSON.parse(init.body);
+  expect(body).toMatchObject({
+    // include newly introduced fields when passed
+    // buildSpecification: 's-1vcpu-512mb',
+    // runtimeSpecification: 's-1vcpu-512mb',
+    // deploymentRetention: 7,
+    // startCommand: 'node custom-server.mjs',
+  });

   delete response.toString;
   expect(response).toEqual(data);
 });

Also applies to: 189-193

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/sites.test.js` around lines 64 - 69, The tests for sites.create
and sites.update only assert mocked responses and must also assert the actual
outbound request body to catch serialization regressions: update the tests
around sites.create and the similar sites.update case to capture/intercept the
HTTP request (e.g., via the existing mock HTTP client or nock) and assert that
the sent JSON includes correctly serialized buildSpecification,
runtimeSpecification, deploymentRetention, and startCommand fields (and their
expected structure/keys/values). Reference the sites.create and sites.update
calls and assert the outbound payload contents rather than only the response to
ensure request-body serialization is validated.

Comment on lines +1263 to +1287
test('test method createIndex()', async () => {
const data = {
'\$id': '5e5ea5c16897e',
'\$createdAt': '2020-10-15T06:38:00.000+00:00',
'\$updatedAt': '2020-10-15T06:38:00.000+00:00',
'key': 'index1',
'type': 'primary',
'status': 'available',
'error': 'string',
'columns': [],
'lengths': [],};
mockedFetch.mockImplementation(() => Response.json(data));

const response = await tablesDB.createIndex(
'<DATABASE_ID>',
'<TABLE_ID>',
'',
'key',
[],
);

// Remove custom toString method on the objects to allow for clean data comparison.
delete response.toString;

expect(response).toEqual(data);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

createIndex() doesn't validate the generated request.

This test passes even though the input type is 'key' and the mocked response says 'primary', which shows it's only asserting response passthrough. If TablesDB.createIndex() serialized the wrong method/body, this would still stay green. Please assert mockedFetch.mock.calls[0] as well for the request payload.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/tables-d-b.test.js` around lines 1263 - 1287, The test for
TablesDB.createIndex is only asserting response passthrough; add assertions to
validate the actual network request made by createIndex by inspecting
mockedFetch.mock.calls[0] to ensure the request method, URL/path and body match
the expected payload (e.g., verify the JSON body contains { type: 'key',
columns: ['key'] } or the correct fields you expect from the createIndex call)
and that headers/content-type are correct; update the test around the
createIndex invocation to parse/mock the first mock call and assert its request
body and method before asserting the response equality.

Comment on lines +414 to +417
const response = await users.updateImpersonator(
'<USER_ID>',
true,
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a false-value test for updateImpersonator().

Only the true branch is covered. Add a case with false to guard against regressions where falsy booleans are accidentally omitted from payload serialization.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/users.test.js` around lines 414 - 417, Add a new unit test that
exercises users.updateImpersonator with the boolean false to ensure falsy values
are serialized; replicate the existing true-case test but call
users.updateImpersonator('<USER_ID>', false) and assert the outgoing
request/payload (or mocked API call) contains impersonator: false (not omitted
or undefined). Place this test alongside the existing true-branch test and use
the same mocking/expectation helpers to verify the request body and response
handling for the false case.


test('test method createJWT()', async () => {
const data = {
'jwt': 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c',};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Replace secret-like fixture literals to avoid leak-scanner hits.

Line 427 and Lines 868-870 contain token-shaped strings that can trigger security tooling and CI false alarms.

Safe fixture replacement
- 'jwt': 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c',
+ 'jwt': '<JWT_TEST_TOKEN>',

- 'providerAccessToken': 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3',
- 'providerRefreshToken': 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3',
+ 'providerAccessToken': '<ACCESS_TOKEN_TEST_VALUE>',
+ 'providerRefreshToken': '<REFRESH_TOKEN_TEST_VALUE>',

Also applies to: 868-870

🧰 Tools
🪛 Betterleaks (1.1.1)

[high] 427-427: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/services/users.test.js` at line 427, Replace the secret-like JWT literal
used in the test fixture (the 'jwt' property shown in users.test.js) with a
non-secret placeholder or a clearly synthetic token (e.g. "TEST_JWT_TOKEN" or a
token generated from a test-only helper), and apply the same replacement to the
other token-shaped strings reported around lines 868-870; ensure tests still
assert expected behavior by using the placeholder or a test helper function
(e.g., makeTestToken()) rather than hard-coded real-looking JWT strings.

@abnegate abnegate merged commit 20b60d1 into main Mar 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants