feat: Console SDK update for version 5.0.0#69
Conversation
WalkthroughUpdates documentation and changelog; bumps package and client x-sdk-version and adds a JSONbig-backed toString on object responses. Adds Domain purchase/transfer confirmation methods to Domains. Replaces Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/models.ts (1)
8731-8771:⚠️ Potential issue | 🟠 MajorMake
clientSecretoptional inModels.Domain.The JSDoc correctly states this field is only present when
paymentStatusispending_confirmation, but the requiredstringtype is incorrect. SinceconfirmPurchaseandconfirmTransferInreturn the raw API payload without transformation, the type definition should match the actual response structure./** * Client secret for payment confirmation. Present only when paymentStatus is pending_confirmation. */ - clientSecret: string; + clientSecret?: string;Consider applying the same optional treatment to
attempts, which is marked "Development only" and likely has conditional presence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models.ts` around lines 8731 - 8771, Update the Models.Domain type so clientSecret is optional to match the API payload (change the required clientSecret: string to an optional field), and optionally make attempts optional as well; ensure this aligns with the conditional presence driven by paymentStatus (e.g., pending_confirmation) and with how confirmPurchase and confirmTransferIn return raw API responses so the type reflects possible absence of these fields.src/services/sites.ts (1)
144-175:⚠️ Potential issue | 🟠 MajorDon’t shift the deprecated positional overload.
Adding
startCommandbeforeoutputDirectorysilently reinterprets existing positional calls. An oldercreate(..., buildCommand, outputDirectory)orupdate(..., buildCommand, outputDirectory)call now posts that value asstartCommand, and every later argument slides one slot. Keep the old tail ordering for the deprecated overload, add a legacy-shape compatibility branch, or remove the overload entirely if this break is intentional.Also applies to: 621-652
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/sites.ts` around lines 144 - 175, The positional overload for create was changed and now places startCommand before outputDirectory which breaks older callers; restore the original deprecated positional tail ordering (keep outputDirectory where it used to be, with startCommand after it) and/or add a legacy-shape compatibility branch in create (and mirror the same fix in update) that detects the old positional call shape and remaps rest[] to the intended names (map buildCommand -> buildCommand, outputDirectory -> outputDirectory, startCommand -> startCommand) so existing positional callers continue to work; update the parameter unpacking logic around the create function (and the update function) that assigns rest[6]..rest[...] to ensure buildCommand, outputDirectory, startCommand, adapter, etc. remain in their original order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/examples/functions/update.md`:
- Around line 28-30: The example uses concrete destructive literals for update
fields (buildSpecification, runtimeSpecification, deploymentRetention) which can
overwrite existing values; change the example so these optional fields are
omitted when not updating (or set to undefined/null as appropriate for your API)
instead of using empty string '' or 0—remove or comment out the lines for
buildSpecification, runtimeSpecification, and deploymentRetention in the update
example so callers don't accidentally wipe existing specifications/retention.
In `@docs/examples/sites/create.md`:
- Around line 29-31: The example currently sets optional fields to concrete
values (buildSpecification: '', runtimeSpecification: '', deploymentRetention:
0) which will be interpreted as real settings; update the snippet to either
remove those keys entirely or replace them with clear placeholders (e.g.,
"<optional>" or omit) so copy-pasting does not submit actual empty-string or
zero values—look for the buildSpecification, runtimeSpecification, and
deploymentRetention entries in the create.md example and adjust them
accordingly.
In `@docs/examples/sites/update.md`:
- Around line 29-31: The example uses literal empty strings and 0 for optional
update fields (buildSpecification, runtimeSpecification, deploymentRetention)
which can accidentally overwrite existing values; change the example to either
omit these keys or set them to undefined (or clearly comment them as "omit to
keep existing") so copy-pasting the example won't reset specs or
retention—update the snippet around buildSpecification, runtimeSpecification,
and deploymentRetention to reflect that behavior.
In `@src/client.ts`:
- Around line 998-1000: The current assignment mutates all objects by directly
setting data.toString, which can overwrite legitimate properties and makes the
property enumerable; change the logic around the data variable so you only
modify plain JSON objects (e.g., check via Object.prototype.toString.call(data)
=== '[object Object]' or an isPlainObject helper) and set the toString as a
non-enumerable property using Object.defineProperty(data, 'toString', { value:
() => JSONbig.stringify(data), writable: true, configurable: true, enumerable:
false }); also ensure you skip this for binary/ArrayBuffer response paths
(responseType === 'arrayBuffer') and for cases where data already has an own
non-configurable toString to avoid throwing.
In `@src/models.ts`:
- Around line 6160-6175: The docs currently expose both
executions/executionsTotal and the new
functionsExecutions/functionsExecutionsTotal which look like duplicates; update
the TypeDoc comments to clarify intent by either marking the legacy
executions/executionsTotal pair as deprecated (add a deprecation note) or
explicitly documenting that executions/executionsTotal are legacy/aggregate
across both functions and sites while
functionsExecutions/functionsExecutionsTotal are function-only metrics; apply
the change in the comment blocks that document executions, executionsTotal,
functionsExecutions, and functionsExecutionsTotal so consumers know which pair
to read and prefer the new functions* pair for function-specific counts.
---
Outside diff comments:
In `@src/models.ts`:
- Around line 8731-8771: Update the Models.Domain type so clientSecret is
optional to match the API payload (change the required clientSecret: string to
an optional field), and optionally make attempts optional as well; ensure this
aligns with the conditional presence driven by paymentStatus (e.g.,
pending_confirmation) and with how confirmPurchase and confirmTransferIn return
raw API responses so the type reflects possible absence of these fields.
In `@src/services/sites.ts`:
- Around line 144-175: The positional overload for create was changed and now
places startCommand before outputDirectory which breaks older callers; restore
the original deprecated positional tail ordering (keep outputDirectory where it
used to be, with startCommand after it) and/or add a legacy-shape compatibility
branch in create (and mirror the same fix in update) that detects the old
positional call shape and remaps rest[] to the intended names (map buildCommand
-> buildCommand, outputDirectory -> outputDirectory, startCommand ->
startCommand) so existing positional callers continue to work; update the
parameter unpacking logic around the create function (and the update function)
that assigns rest[6]..rest[...] to ensure buildCommand, outputDirectory,
startCommand, adapter, etc. remain in their original order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f82c8c3b-33ad-4fb7-8038-807f5acef4ae
📒 Files selected for processing (22)
CHANGELOG.mdREADME.mddocs/examples/domains/confirm-purchase.mddocs/examples/domains/confirm-transfer-in.mddocs/examples/functions/create.mddocs/examples/functions/update.mddocs/examples/sites/create.mddocs/examples/sites/update.mdpackage.jsonsrc/client.tssrc/enums/appwrite-migration-resource.tssrc/enums/build-runtime.tssrc/enums/domain-purchase-payment-status.tssrc/enums/runtime.tssrc/enums/runtimes.tssrc/index.tssrc/models.tssrc/services/account.tssrc/services/domains.tssrc/services/functions.tssrc/services/organizations.tssrc/services/sites.ts
💤 Files with no reviewable changes (1)
- CHANGELOG.md
| buildSpecification: '', // optional | ||
| runtimeSpecification: '', // optional | ||
| deploymentRetention: 0 // optional |
There was a problem hiding this comment.
Avoid destructive literal values in the update example.
For update, '' and 0 are concrete mutations. A copied example can wipe the current specifications/retention instead of leaving them unchanged.
Suggested doc fix
- buildSpecification: '', // optional
- runtimeSpecification: '', // optional
- deploymentRetention: 0 // optional
+ // buildSpecification: '<BUILD_SPECIFICATION>', // optional
+ // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional
+ // deploymentRetention: <DEPLOYMENT_RETENTION> // optional📝 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.
| buildSpecification: '', // optional | |
| runtimeSpecification: '', // optional | |
| deploymentRetention: 0 // optional | |
| // buildSpecification: '<BUILD_SPECIFICATION>', // optional | |
| // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional | |
| // deploymentRetention: <DEPLOYMENT_RETENTION> // optional |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/examples/functions/update.md` around lines 28 - 30, The example uses
concrete destructive literals for update fields (buildSpecification,
runtimeSpecification, deploymentRetention) which can overwrite existing values;
change the example so these optional fields are omitted when not updating (or
set to undefined/null as appropriate for your API) instead of using empty string
'' or 0—remove or comment out the lines for buildSpecification,
runtimeSpecification, and deploymentRetention in the update example so callers
don't accidentally wipe existing specifications/retention.
| buildSpecification: '', // optional | ||
| runtimeSpecification: '', // optional | ||
| deploymentRetention: 0 // optional |
There was a problem hiding this comment.
Use placeholders or omit these optional fields in the example.
'' and 0 are real values, not placeholders. Copy-pasting this snippet will send concrete settings instead of leaving the optional fields unset.
Suggested doc fix
- buildSpecification: '', // optional
- runtimeSpecification: '', // optional
- deploymentRetention: 0 // optional
+ // buildSpecification: '<BUILD_SPECIFICATION>', // optional
+ // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional
+ // deploymentRetention: <DEPLOYMENT_RETENTION> // optional📝 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.
| buildSpecification: '', // optional | |
| runtimeSpecification: '', // optional | |
| deploymentRetention: 0 // optional | |
| // buildSpecification: '<BUILD_SPECIFICATION>', // optional | |
| // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional | |
| // deploymentRetention: <DEPLOYMENT_RETENTION> // optional |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/examples/sites/create.md` around lines 29 - 31, The example currently
sets optional fields to concrete values (buildSpecification: '',
runtimeSpecification: '', deploymentRetention: 0) which will be interpreted as
real settings; update the snippet to either remove those keys entirely or
replace them with clear placeholders (e.g., "<optional>" or omit) so
copy-pasting does not submit actual empty-string or zero values—look for the
buildSpecification, runtimeSpecification, and deploymentRetention entries in the
create.md example and adjust them accordingly.
| buildSpecification: '', // optional | ||
| runtimeSpecification: '', // optional | ||
| deploymentRetention: 0 // optional |
There was a problem hiding this comment.
Make the optional update fields copy-paste safe.
These literals send actual values. In an update example, that can unintentionally reset spec fields or retention instead of preserving the existing configuration.
Suggested doc fix
- buildSpecification: '', // optional
- runtimeSpecification: '', // optional
- deploymentRetention: 0 // optional
+ // buildSpecification: '<BUILD_SPECIFICATION>', // optional
+ // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional
+ // deploymentRetention: <DEPLOYMENT_RETENTION> // optional📝 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.
| buildSpecification: '', // optional | |
| runtimeSpecification: '', // optional | |
| deploymentRetention: 0 // optional | |
| // buildSpecification: '<BUILD_SPECIFICATION>', // optional | |
| // runtimeSpecification: '<RUNTIME_SPECIFICATION>', // optional | |
| // deploymentRetention: <DEPLOYMENT_RETENTION> // optional |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/examples/sites/update.md` around lines 29 - 31, The example uses literal
empty strings and 0 for optional update fields (buildSpecification,
runtimeSpecification, deploymentRetention) which can accidentally overwrite
existing values; change the example to either omit these keys or set them to
undefined (or clearly comment them as "omit to keep existing") so copy-pasting
the example won't reset specs or retention—update the snippet around
buildSpecification, runtimeSpecification, and deploymentRetention to reflect
that behavior.
| if (data && typeof data === 'object') { | ||
| data.toString = () => JSONbig.stringify(data); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 "responseType\s*=\s*'arrayBuffer'|arrayBuffer\(" src
rg -n -C3 "\[key:\s*string\]\s*:\s*any|DocumentList|Document<|type Payload" srcRepository: appwrite/sdk-for-console
Length of output: 33400
Limit toString assignment to plain JSON objects and define it non-enumerably.
The current code mutates all object responses—including ArrayBuffers from responseType === 'arrayBuffer' paths (line 974)—and overwrites any legitimate toString fields returned by the API or stored in user-controlled document data. Direct property assignment also creates an enumerable own property that leaks into Object.keys() and spread operations. Restrict this to plain objects and use Object.defineProperty with enumerable: false to prevent shape mutations.
Suggested fix
- if (data && typeof data === 'object') {
- data.toString = () => JSONbig.stringify(data);
+ if (
+ data &&
+ Object.prototype.toString.call(data) === '[object Object]' &&
+ !Object.prototype.hasOwnProperty.call(data, 'toString')
+ ) {
+ Object.defineProperty(data, 'toString', {
+ value() {
+ return JSONbig.stringify(this);
+ },
+ 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 998 - 1000, The current assignment mutates all
objects by directly setting data.toString, which can overwrite legitimate
properties and makes the property enumerable; change the logic around the data
variable so you only modify plain JSON objects (e.g., check via
Object.prototype.toString.call(data) === '[object Object]' or an isPlainObject
helper) and set the toString as a non-enumerable property using
Object.defineProperty(data, 'toString', { value: () => JSONbig.stringify(data),
writable: true, configurable: true, enumerable: false }); also ensure you skip
this for binary/ArrayBuffer response paths (responseType === 'arrayBuffer') and
for cases where data already has an own non-configurable toString to avoid
throwing.
| /** | ||
| * Aggregated number of function executions per period. | ||
| */ | ||
| functionsExecutions: Metric[]; | ||
| /** | ||
| * Total aggregated number of function executions. | ||
| */ | ||
| functionsExecutionsTotal: number; | ||
| /** | ||
| * Aggregated number of site executions per period. | ||
| */ | ||
| sitesExecutions: Metric[]; | ||
| /** | ||
| * Total aggregated number of site executions. | ||
| */ | ||
| sitesExecutionsTotal: number; |
There was a problem hiding this comment.
Clarify how functionsExecutions* differs from the existing executions* pair.
Line 6033 still documents executionsTotal as function executions, so after adding functionsExecutionsTotal this type now exposes two totals that appear to mean the same thing. Please either mark the legacy pair as aggregate/deprecated or update the docs so consumers know which pair to read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/models.ts` around lines 6160 - 6175, The docs currently expose both
executions/executionsTotal and the new
functionsExecutions/functionsExecutionsTotal which look like duplicates; update
the TypeDoc comments to clarify intent by either marking the legacy
executions/executionsTotal pair as deprecated (add a deprecation note) or
explicitly documenting that executions/executionsTotal are legacy/aggregate
across both functions and sites while
functionsExecutions/functionsExecutionsTotal are function-only metrics; apply
the change in the comment blocks that document executions, executionsTotal,
functionsExecutions, and functionsExecutionsTotal so consumers know which pair
to read and prefer the new functions* pair for function-specific counts.
premtsd-code
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/client.ts (1)
998-1000:⚠️ Potential issue | 🟠 MajorLimit
toStringassignment to plain JSON objects and define it non-enumerably.The issues flagged in the previous review remain unaddressed:
- ArrayBuffer responses (from line 974's
responseType === 'arrayBuffer'path) will have theirtoStringoverwritten sincetypeof ArrayBuffer === 'object'- Any legitimate
toStringproperty in API response data gets silently overwritten- Direct property assignment creates an enumerable property that leaks into
Object.keys()and spread operationsSuggested fix
- if (data && typeof data === 'object') { - data.toString = () => JSONbig.stringify(data); + if ( + data && + Object.prototype.toString.call(data) === '[object Object]' && + !Object.prototype.hasOwnProperty.call(data, 'toString') + ) { + Object.defineProperty(data, 'toString', { + value() { + return JSONbig.stringify(this); + }, + enumerable: false, + configurable: true, + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client.ts` around lines 998 - 1000, Limit the custom toString only to plain JSON objects and make it non-enumerable: inside the block handling `data` (where you currently check `if (data && typeof data === 'object')`) additionally verify the value is a plain object (e.g., via Object.prototype.toString.call(data) === '[object Object]' or similar) and that it does not already have its own `toString` (use Object.prototype.hasOwnProperty.call). Then create the `toString` using Object.defineProperty on `data` with a non-enumerable descriptor (enumerable: false, configurable: true, writable: true) and have it return JSONbig.stringify(data) so arrays, ArrayBuffers and existing toString implementations are not overwritten and the property does not leak into Object.keys()/spreads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/client.ts`:
- Around line 998-1000: Limit the custom toString only to plain JSON objects and
make it non-enumerable: inside the block handling `data` (where you currently
check `if (data && typeof data === 'object')`) additionally verify the value is
a plain object (e.g., via Object.prototype.toString.call(data) === '[object
Object]' or similar) and that it does not already have its own `toString` (use
Object.prototype.hasOwnProperty.call). Then create the `toString` using
Object.defineProperty on `data` with a non-enumerable descriptor (enumerable:
false, configurable: true, writable: true) and have it return
JSONbig.stringify(data) so arrays, ArrayBuffers and existing toString
implementations are not overwritten and the property does not leak into
Object.keys()/spreads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ff3f098-8427-47c8-822e-3f39e657510a
📒 Files selected for processing (3)
CHANGELOG.mdpackage.jsonsrc/client.ts
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
premtsd-code
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/client.ts (1)
998-1000:⚠️ Potential issue | 🟠 MajorLimit the
toStringpatch to plain JSON objects.This still mutates every object response, including
ArrayBufferpayloads from Line 973, and it can overwrite a realtoStringfield coming back from the API. Direct assignment also makes the helper enumerable, so it leaks intoObject.keys()and spread operations.Suggested fix
- if (data && typeof data === 'object') { - data.toString = () => JSONbig.stringify(data); - } + if (data && Object.prototype.toString.call(data) === '[object Object]') { + const ownToString = Object.getOwnPropertyDescriptor(data, 'toString'); + if (!ownToString || ownToString.configurable) { + Object.defineProperty(data, 'toString', { + value() { + return JSONbig.stringify(this); + }, + configurable: true, + writable: true, + enumerable: false, + }); + } + }#!/bin/bash # Verify affected binary-response paths and existing explicit toString() implementations. rg -n -C2 "responseType === 'arrayBuffer'|responseType\s*=\s*'arrayBuffer'|arrayBuffer\(" src rg -n -C2 "toString\(\): string" src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client.ts` around lines 998 - 1000, Limit the toString patch to plain POJOs and add it as a non-enumerable property: check that data is a plain object (e.g., Object.prototype.toString.call(data) === '[object Object]') and that it does not already own a toString property (use Object.prototype.hasOwnProperty.call(data, 'toString')), then use Object.defineProperty(data, 'toString', { value: () => JSONbig.stringify(data), enumerable: false, writable: false, configurable: true }) so you don't mutate ArrayBuffer/binary responses, overwrite an existing toString implementation, or leak the helper into Object.keys()/spreads.
🤖 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: Update the CHANGELOG entry to correct the breaking-change
note: replace the incorrect reference to `specification` with the new request
shape `buildSpecification` and `runtimeSpecification`, and clarify that
Functions and Sites now require those two parameters and support
`deploymentRetention`; update the first bullet so it reads that Functions and
Sites require `buildSpecification`/`runtimeSpecification` (not `specification`)
and mention `deploymentRetention` support.
---
Duplicate comments:
In `@src/client.ts`:
- Around line 998-1000: Limit the toString patch to plain POJOs and add it as a
non-enumerable property: check that data is a plain object (e.g.,
Object.prototype.toString.call(data) === '[object Object]') and that it does not
already own a toString property (use Object.prototype.hasOwnProperty.call(data,
'toString')), then use Object.defineProperty(data, 'toString', { value: () =>
JSONbig.stringify(data), enumerable: false, writable: false, configurable: true
}) so you don't mutate ArrayBuffer/binary responses, overwrite an existing
toString implementation, or leak the helper into Object.keys()/spreads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77da7c35-a9fe-4e4a-938e-5c633f1ecd6d
📒 Files selected for processing (3)
CHANGELOG.mdpackage.jsonsrc/client.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
| * Breaking: Functions and Sites now require `specification` parameter and support `deploymentRetention` | ||
| * Breaking: New `buildSpecification` and `runtimeSpecification` parameters for functions |
There was a problem hiding this comment.
Fix the breaking-change note for specification.
The first bullet says Functions and Sites now require specification, but this PR's API changes replace that surface with buildSpecification / runtimeSpecification. Leaving specification in the 5.0.0 notes will send upgraders to the wrong request shape.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 5 - 6, Update the CHANGELOG entry to correct the
breaking-change note: replace the incorrect reference to `specification` with
the new request shape `buildSpecification` and `runtimeSpecification`, and
clarify that Functions and Sites now require those two parameters and support
`deploymentRetention`; update the first bullet so it reads that Functions and
Sites require `buildSpecification`/`runtimeSpecification` (not `specification`)
and mention `deploymentRetention` support.
2 participants
This PR contains updates to the Console SDK for version 5.0.0.
Summary by CodeRabbit
New Features
Updates
Documentation