Open
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the global Parse.Cloud implementation with a per-ParseServer LegacyCloud, adds a centralized per-app in-memory TriggerStore, introduces Cloud Code SDK types and a Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Cloud code author
participant PS as ParseServer
participant LC as LegacyCloud
participant Parse as parse/node(Parse)
participant TS as TriggerStore
Dev->>PS: start ParseServer with cloud option
PS->>LC: new LegacyCloud() and initialize({appId,...})
LC->>Parse: require('parse/node') and Parse.initialize(...)
LC->>LC: set appId and Parse.serverURL
PS->>LC: during start use this.legacyCloud.Parse
PS->>LC: call bindToParseCloud()
LC->>Parse: attach Parse.Cloud methods (define, beforeSave, job, etc.)
Parse->>TS: Parse.Cloud methods proxy to TriggerStore (addFunction/addTrigger/addJob...)
Dev->>Parse: register cloud handlers via Parse.Cloud APIs
Runtime->>TS: TriggerStore invoked for handler lookup and execution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ 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 |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 6-9: The "./cloud" export in package.json is missing a types
mapping so TypeScript can't find declarations; update the package.json exports
entry for "./cloud" (the "./cloud": "./lib/cloud.js" mapping) to include a types
field pointing to the declaration file (for example "types":
"./types/cloud.d.ts"), or alternatively add a typesVersions mapping that maps
the "./cloud" subpath to the declaration file; ensure the declaration file path
matches your built .d.ts location and keep the existing "./cloud" value intact
while adding the types mapping.
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 120-159: The define and defineTrigger wrappers drop
validator.rateLimit because they forward the raw validator to triggers without
calling validateValidator() or applyRateLimit(); update define (around the
FunctionHandler wrapping and triggers.addFunction) and defineTrigger (around
wrappedHandler and triggers.addTrigger/addConnectTrigger) to first call
validateValidator(validator) then call applyRateLimit(validated,
nameOrClassIdentifier, this._appId) (or the equivalent helper used by
cloudDefine/cloudTrigger) and pass the validated/resulting validator (or ensure
applyRateLimit is applied) into
triggers.addFunction/addTrigger/addConnectTrigger so rate limits are enforced
for SDK-registered handlers.
- Around line 264-272: The setter for Parse.Server currently overwrites the
cached config with only newVal and appId, causing loss of required fields;
update the setter to first load the existing config via
Config.get(Parse.applicationId), merge that existing config with newVal
(ensuring appId is preserved/assigned), and then call Config.put(mergedConfig)
so partial assignments (e.g., Parse.Server = { enableSingleSchemaCache: true })
only modify specified keys and retain controllers and other required fields.
- Around line 319-339: The cloudAuthTrigger function parses an optional
Validator into the local validator variable but never registers it with
triggers.addTrigger, so validators for auth triggers (e.g., beforeLogin /
beforePasswordResetRequest) never run; update the call to triggers.addTrigger in
cloudAuthTrigger to pass the parsed validator as the validation handler (use the
validator variable) alongside type, className, and this._appId, and keep
applyRateLimit using that same validator—ensure you use the same
handler/validator ordering expected by triggers.addTrigger so existing overload
logic (string/object handler resolution) still works.
- Around line 22-33: getRoute currently maps system classes via ROUTE_MAP but
still appends the class name, causing e.g. `_User` -> `/users/_User...`; change
the logic in getRoute (and keep ROUTE_MAP) so that when parseClass is in
ROUTE_MAP you return the base segment `/${route}` (except the special '@File'
case which should remain `/${route}{/*id}` and keep '@Config' as `/${route}`),
and only for unmapped classes return `/${route}/${parseClass}{/*id}`; update
getRoute to check ROUTE_MAP[parseClass] first and return the appropriate base
route to ensure rate-limit paths match real REST routes.
In `@src/cloud-code/types.ts`:
- Around line 126-128: ObjectTriggerHandler currently allows Partial<T> | void
which incorrectly permits non-beforeSave handlers (afterSave, beforeDelete,
afterDelete) to return patches; create a distinct handler type for beforeSave
(e.g., BeforeSaveObjectTriggerHandler<T> = (request: ObjectTriggerRequest<T>) =>
BeforeSaveResult<T>) and change ObjectTriggerHandler<T> to be the void-only type
for afterSave/beforeDelete/afterDelete (i.e., (request: ObjectTriggerRequest<T>)
=> AfterTriggerResult). Update TriggerHandlerMap to use
BeforeSaveObjectTriggerHandler for the beforeSave key and ObjectTriggerHandler
for the other keys, referencing the existing types ObjectTriggerRequest,
BeforeSaveResult, AfterTriggerResult, ObjectTriggerHandler, and
TriggerHandlerMap in your changes.
In `@src/ParseServer.ts`:
- Around line 129-133: ParseServer currently constructs a concrete LegacyCloud
and uses its .Parse and .bindToParseCloud(), which bypasses the
CloudCodeRegistrar contract and prevents custom registrars from being used;
change ParseServer to retrieve the registrar instance via
CloudCodeRegistrar.getInstance() (or the registrar accessor used in this
codebase) instead of new LegacyCloud(), and call only the registrar's contract
methods (e.g., expose Parse via the registrar API and invoke bindToParseCloud()
through the registrar interface) rather than downcasting to LegacyCloud so
custom CloudCodeRegistrar implementations can plug in without being overwritten.
🪄 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: 15231df1-a0ac-4c07-88e1-05844979e470
📒 Files selected for processing (8)
package.jsonsrc/ParseServer.tssrc/cloud-code/CloudCodeRegistrar.tssrc/cloud-code/LegacyCloud.tssrc/cloud-code/Parse.Cloud.jssrc/cloud-code/README.mdsrc/cloud-code/types.tssrc/cloud.ts
💤 Files with no reviewable changes (1)
- src/cloud-code/Parse.Cloud.js
…letion The SDK adapter refactor replaced Parse.Cloud.js with new modular files (LegacyCloud.ts, TriggerStore.ts, etc.). Upstream modified the old file, so we keep our deletion.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/cloud-code/LegacyCloud.ts (3)
272-285:⚠️ Potential issue | 🟠 MajorAuth trigger validators are still dropped.
cloudAuthTrigger()parsesvalidatorand even uses it forrateLimit, but thetriggers.addTrigger()call omits the validation handler.beforeLogin,afterLogin,afterLogout, andbeforePasswordResetRequesttherefore never enforce their validator rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 272 - 285, The auth trigger registration currently drops the validator when calling triggers.addTrigger, so update the call to include the parsed validator (the same validator variable used for rateLimit) so beforeLogin/afterLogin/afterLogout/beforePasswordResetRequest enforce validation; ensure you pass the validator alongside type, className, handler and this._appId (and adjust the addTrigger argument order if necessary) and keep applyRateLimit using that same validator variable.
214-216:⚠️ Potential issue | 🟠 MajorPartial
Parse.Serverwrites still clobber the cached config.
Config.put(newVal)replaces the whole app config. A setter call likeParse.Server = { enableSingleSchemaCache: true }will drop controllers and the rest of the live configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 214 - 216, The setter in LegacyCloud.ts currently calls Config.put(newVal) which replaces the entire app config and clobbers live state; change it to load the existing config for Parse.applicationId (e.g., via Config.get or currentConfig), merge the incoming newVal into that existing config (shallow-merge only provided keys so controllers and other fields are preserved), ensure appId is set on the merged object, then call Config.put(mergedConfig) so only supplied properties (like enableSingleSchemaCache) are updated instead of overwriting the whole config.
27-31:⚠️ Potential issue | 🟠 MajorSystem-class rate-limit paths still don't match the real REST routes.
getRoute('_User')still resolves to/users/_User{/*id}and_Sessionto/sessions/_Session{/*id}here, sovalidator.rateLimiton those triggers never matches the actual/usersor/sessionsendpoints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 27 - 31, getRoute currently appends the parseClass name to the path even when ROUTE_MAP already maps that class to a top-level route (so getRoute('_User') -> `/users/_User{/*id}`), breaking rate-limit matching; update getRoute (function name getRoute, symbol ROUTE_MAP and parameter parseClass) so that if ROUTE_MAP[parseClass] exists you return `/${route}{/*id}` (same as the `@File/`@Config branches) instead of `/${route}/${parseClass}{/*id}`, otherwise fall back to the `classes` behavior; keep the existing `@File` and `@Config` special cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 98-107: LegacyCloud.initialize currently assigns
require('parse/node').Parse to global.Parse which causes cross-server
interference; remove writing to global.Parse and instantiate/retain a
per-instance Parse reference in LegacyCloud instead. Update call sites to pass
explicit appId: change maybeRunValidator to accept and use an explicit
applicationId (call getValidator(functionName, applicationId) instead of relying
on Parse.applicationId), modify runLiveQueryEventHandlers to require an appId
parameter and use it rather than defaulting to Parse.applicationId, and update
ParseLiveQueryServer to call runLiveQueryEventHandlers(this.config.appId, ...)
so live-query handlers run against the correct server; ensure any other places
that depended on global.Parse use the LegacyCloud instance or explicit appId
instead.
In `@src/cloud-code/TriggerStore.ts`:
- Around line 163-167: The validator map (s.validators) can retain stale entries
because addFunction/addTrigger/addConnectTrigger only add validators when
provided and removeFunction/removeTrigger never delete them; update these
methods (addFunction, addTrigger, addConnectTrigger) to explicitly remove any
existing validator entry for the given name when no validator is passed (i.e.,
delete s.validators[name] if validator is undefined) and update the removal
methods (removeFunction, removeTrigger, and the connect removal equivalent) to
also delete the corresponding s.validators[name] when removing the handler so
validators can't persist after a hook is replaced or removed.
- Around line 253-260: The clearAll() implementation currently only clears
TriggerStore._stores and therefore skips unregistering cloud rate-limiters;
update clearAll() to iterate over all stored appIds (keys of
TriggerStore._stores) and call the same cleanup logic as removeAllHooks(appId) —
i.e., for each appId call Config.get(appId)?.unregisterRateLimiters() before
clearing the store — or alternatively call removeAllHooks(appId) for each appId
to ensure unregisterRateLimiters runs for every app; ensure the final step still
clears TriggerStore._stores.
In `@src/cloud-code/validateTrigger.ts`:
- Around line 4-5: The current check in validateTrigger.ts only forbids
beforeSave for className === '_PushStatus' but the error says only afterSave is
allowed; change the predicate to reject any trigger on '_PushStatus' whose type
is not TriggerType.afterSave (i.e., if (className === '_PushStatus' && type !==
TriggerType.afterSave) throw 'Only afterSave is allowed on _PushStatus'),
updating the condition that references type, className, and TriggerType (inside
the validateTrigger function) so all unsupported hook types (afterDelete,
beforeFind, etc.) are consistently blocked.
---
Duplicate comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 272-285: The auth trigger registration currently drops the
validator when calling triggers.addTrigger, so update the call to include the
parsed validator (the same validator variable used for rateLimit) so
beforeLogin/afterLogin/afterLogout/beforePasswordResetRequest enforce
validation; ensure you pass the validator alongside type, className, handler and
this._appId (and adjust the addTrigger argument order if necessary) and keep
applyRateLimit using that same validator variable.
- Around line 214-216: The setter in LegacyCloud.ts currently calls
Config.put(newVal) which replaces the entire app config and clobbers live state;
change it to load the existing config for Parse.applicationId (e.g., via
Config.get or currentConfig), merge the incoming newVal into that existing
config (shallow-merge only provided keys so controllers and other fields are
preserved), ensure appId is set on the merged object, then call
Config.put(mergedConfig) so only supplied properties (like
enableSingleSchemaCache) are updated instead of overwriting the whole config.
- Around line 27-31: getRoute currently appends the parseClass name to the path
even when ROUTE_MAP already maps that class to a top-level route (so
getRoute('_User') -> `/users/_User{/*id}`), breaking rate-limit matching; update
getRoute (function name getRoute, symbol ROUTE_MAP and parameter parseClass) so
that if ROUTE_MAP[parseClass] exists you return `/${route}{/*id}` (same as the
`@File/`@Config branches) instead of `/${route}/${parseClass}{/*id}`, otherwise
fall back to the `classes` behavior; keep the existing `@File` and `@Config` special
cases.
🪄 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: 3821d467-0a53-45a0-8fa0-6c339967b7f1
📒 Files selected for processing (8)
src/ParseServer.tssrc/cloud-code/LegacyCloud.tssrc/cloud-code/README.mdsrc/cloud-code/TriggerStore.tssrc/cloud-code/types.tssrc/cloud-code/validateTrigger.tssrc/cloud.tssrc/triggers.js
✅ Files skipped from review due to trivial changes (3)
- src/cloud-code/README.md
- src/cloud.ts
- src/cloud-code/types.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/cloud-code/LegacyCloud.ts (2)
100-109:⚠️ Potential issue | 🟠 Major
LegacyCloudis still backed by the shared Parse SDK singleton.
require('parse/node').Parsegives you the process-wide SDK object. Re-initializing it here and assigning it toglobal.Parsemeans the lastParseServerstarted in-process wins; laterParse.Cloud/Parse.Serverbindings then operate against that shared mutable state instead of a single app.Run this read-only check to confirm the remaining global Parse /
Parse.applicationIddependencies:#!/bin/bash rg -n -C2 "require\\('parse/node'\\)\\.Parse|global\\.Parse|bindToParseCloud|Parse\\.Cloud\\[" \ src/cloud-code/LegacyCloud.ts src/ParseServer.ts rg -n -C2 "Parse\\.applicationId|runLiveQueryEventHandlers\\(|maybeRunValidator\\(" \ src/triggers.js src/LiveQuery/ParseLiveQueryServer.tsExpected result: any
global.Parsewrite orParse.applicationIdfallback means the adapter is still process-global instead of app-scoped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 100 - 109, The initialize method currently reinitializes and writes the process-wide Parse singleton (require('parse/node').Parse) and assigns it to global.Parse; stop mutating the global SDK: keep the Parse SDK instance scoped to the LegacyCloud instance (use this._parse only), remove the global assignment (delete the line that sets (global as any).Parse), and ensure all other code paths use this._parse instead of relying on global.Parse or Parse.applicationId; run the suggested ripgrep checks from the review to find remaining globals like global.Parse, Parse.applicationId, Parse.Cloud, bindToParseCloud, runLiveQueryEventHandlers, and update those call sites to accept/forward the instance from LegacyCloud (or change them to instance methods) so the adapter is app-scoped rather than process-global.
27-33:⚠️ Potential issue | 🟠 MajorThe system-class route fix is still incomplete.
getRoute('_User')now returns/users, butcloudBeforeSave()registers bothPOSTandPUT. That covers signup, but it still misses object-level routes likePUT /users/:objectId, sovalidator.rateLimitwill not fire for user updates.@Configis the only mapped route that should stay exact here.🔧 Suggested fix
function getRoute(parseClass: string): string { const route = ROUTE_MAP[parseClass]; if (route) { - if (parseClass === '@File') return `/${route}{/*id}`; - return `/${route}`; + if (parseClass === '@Config') return `/${route}`; + return `/${route}{/*id}`; } return `/classes/${parseClass}{/*id}`; }Also applies to: 127-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 27 - 33, getRoute currently returns collection-only routes for mapped system classes (e.g., returning "/users" for "_User") which prevents registering object-level handlers; update getRoute so that when ROUTE_MAP[parseClass] exists you return a collection+object route ("/{route}{/*id}") for all mapped classes except the special exact-only "@Config" (keep the existing special-case for "@File" too); adjust the logic in getRoute to check for parseClass === '@File' and parseClass === '@Config' and return the appropriate string so object-level routes (e.g., PUT /users/:objectId) are generated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 187-195: cloudSendEmail currently logs and returns undefined when
no mail adapter exists which causes callers to treat the call as successful;
update cloudSendEmail (in LegacyCloud) to instead reject when emailAdapter is
missing: keep the config.loggerController.error call for logging, then throw or
return Promise.reject(new Error('No mail adapter configured for Parse Server'))
so callers (e.g., await Parse.Cloud.sendEmail(...)) receive a rejected promise
and can handle the failure.
---
Duplicate comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 100-109: The initialize method currently reinitializes and writes
the process-wide Parse singleton (require('parse/node').Parse) and assigns it to
global.Parse; stop mutating the global SDK: keep the Parse SDK instance scoped
to the LegacyCloud instance (use this._parse only), remove the global assignment
(delete the line that sets (global as any).Parse), and ensure all other code
paths use this._parse instead of relying on global.Parse or Parse.applicationId;
run the suggested ripgrep checks from the review to find remaining globals like
global.Parse, Parse.applicationId, Parse.Cloud, bindToParseCloud,
runLiveQueryEventHandlers, and update those call sites to accept/forward the
instance from LegacyCloud (or change them to instance methods) so the adapter is
app-scoped rather than process-global.
- Around line 27-33: getRoute currently returns collection-only routes for
mapped system classes (e.g., returning "/users" for "_User") which prevents
registering object-level handlers; update getRoute so that when
ROUTE_MAP[parseClass] exists you return a collection+object route
("/{route}{/*id}") for all mapped classes except the special exact-only
"@Config" (keep the existing special-case for "@File" too); adjust the logic in
getRoute to check for parseClass === '@File' and parseClass === '@Config' and
return the appropriate string so object-level routes (e.g., PUT
/users/:objectId) are generated.
🪄 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: 69ba9810-341e-47d2-ba56-106a3c13355d
📒 Files selected for processing (4)
package.jsonsrc/cloud-code/LegacyCloud.tssrc/cloud-code/types.tssrc/cloud.ts
✅ Files skipped from review due to trivial changes (2)
- src/cloud.ts
- src/cloud-code/types.ts
Comment on lines
+187
to
+195
| cloudSendEmail(data: Record<string, unknown>): unknown { | ||
| const config = Config.get(this._appId); | ||
| const emailAdapter = config.userController.adapter; | ||
| if (!emailAdapter) { | ||
| config.loggerController.error('Failed to send email because no mail adapter is configured for Parse Server.'); | ||
| return; | ||
| } | ||
| return emailAdapter.sendMail(data); | ||
| } |
There was a problem hiding this comment.
cloudSendEmail() should reject when mail is unavailable.
When no mail adapter is configured, this logs and returns undefined. await Parse.Cloud.sendEmail(...) will therefore resolve as if the operation succeeded, which makes missing email configuration very hard to detect from Cloud Code.
💡 Suggested fix
if (!emailAdapter) {
- config.loggerController.error('Failed to send email because no mail adapter is configured for Parse Server.');
- return;
+ const message = 'Failed to send email because no mail adapter is configured for Parse Server.';
+ config.loggerController.error(message);
+ return Promise.reject(new Error(message));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/LegacyCloud.ts` around lines 187 - 195, cloudSendEmail
currently logs and returns undefined when no mail adapter exists which causes
callers to treat the call as successful; update cloudSendEmail (in LegacyCloud)
to instead reject when emailAdapter is missing: keep the
config.loggerController.error call for logging, then throw or return
Promise.reject(new Error('No mail adapter configured for Parse Server')) so
callers (e.g., await Parse.Cloud.sendEmail(...)) receive a rejected promise and
can handle the failure.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git log --oneline --all -- src/cloud-code/Parse.Cloud.js | head -5
git show HEAD~1:src/cloud-code/Parse.Cloud.js 2>/dev/null | grep -A 15 'sendEmail'
# Also try checking the alpha branch or the base branch
git log --oneline origin/alpha -- src/cloud-code/Parse.Cloud.js 2>/dev/null | head -5
git show origin/alpha:src/cloud-code/Parse.Cloud.js 2>/dev/null | grep -A 15 'sendEmail'
Length of output: 1824
@dblythy, you're correct — the old Parse.Cloud.js had identical behavior: log an error and return undefined without rejecting. LegacyCloud.ts faithfully preserves this. I apologize for the noise.
(´• ω •`) ʕ•ᴥ•ʔ
✏️ Learnings added
Learnt from: dblythy
Repo: parse-community/parse-server PR: 10256
File: src/cloud-code/LegacyCloud.ts:187-195
Timestamp: 2026-03-22T03:50:31.365Z
Learning: In parse-community/parse-server (`src/cloud-code/LegacyCloud.ts`, previously `src/cloud-code/Parse.Cloud.js`), `Parse.Cloud.sendEmail()` intentionally logs an error and returns `undefined` (does not reject/throw) when no mail adapter is configured. This is existing, preserved behavior — do not flag it as a bug.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10076
File: src/Routers/FilesRouter.js:401-404
Timestamp: 2026-02-25T21:35:52.338Z
Learning: In Parse Server, values modified by triggers (like beforeSave) are not re-validated by the framework. It's the developer's responsibility to ensure their trigger code handles data correctly, especially when using master key access. This design philosophy applies consistently across all triggers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10238
File: spec/PagesRouter.spec.js:843-847
Timestamp: 2026-03-18T21:07:30.884Z
Learning: In parse-community/parse-server (src/Routers/PagesRouter.js), when `emailVerifySuccessOnInvalidEmail: true` (the default), the `resendVerificationEmail` handler intentionally suppresses ALL errors — including both nonexistent-user rejections and generic backend failures — and redirects to the success page. This mirrors the behavior in UsersRouter for the equivalent API route. Tests that spy on `UserController.prototype.resendVerificationEmail` to return a generic rejection and assert a success redirect are therefore correct and intentional, not a test design flaw. A separate set of end-to-end tests in the `exploits` describe block covers the actual nonexistent-user scenario without mocking.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR `#9938` and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 10204
File: src/Security/CheckGroups/CheckGroupServerConfig.js:145-147
Timestamp: 2026-03-14T21:39:34.558Z
Learning: In Parse Server's `src/Security/CheckGroups/CheckGroupServerConfig.js`, the "Request complexity limits enabled" security check intentionally uses `return` (not `throw 1`) when `requestComplexity` is `undefined` in PS9. This is by design: `requestComplexity` is opt-in in the current major version and will only become a hard-failing security check (throw 1 when undefined) in PS10. The deprecation DEPPS18 in `src/Deprecator/Deprecations.js` communicates this upcoming default change to operators.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10149
File: src/Controllers/DatabaseController.js:1865-1873
Timestamp: 2026-03-08T21:51:35.204Z
Learning: In `src/Controllers/DatabaseController.js` (`performInitialization`), the `ensureAuthDataUniqueness` index creation is intentionally non-fatal (warn-only, no rethrow). Unlike username/email uniqueness which has been enforced since Parse Server's inception, authData uniqueness is new. Existing production databases may have duplicate authData entries caused by the race condition this PR fixes, so crashing on startup would be a breaking change. The application-level `ensureUniqueAuthDataId` check in `RestWrite.js` still runs as a fallback, and the warning log gives operators visibility to resolve duplicates manually before the index can be created successfully.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:08.440Z
Learning: For Parse Server 9 release preparation, the parse/push-adapter dependency should be upgraded to version >= 8.0.0, not 7.x, as version 8.x is required despite dropping Node 18 support (which aligns with Parse Server 9's removal of EOL Node versions).
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10170
File: src/Adapters/Auth/mfa.js:162-168
Timestamp: 2026-03-10T04:21:42.141Z
Learning: In parse-community/parse-server, all auth adapters that return `{ save: ... }` from `validateLogin` (e.g., `src/Adapters/Auth/mfa.js`) go through a non-atomic read-modify-write path: `src/Auth.js` forwards the `save` payload and `src/RestWrite.js` persists it filtered only by `objectId`. This is a pre-existing architectural pattern of the adapter interface, not specific to any single adapter. Making individual adapter state changes (e.g., consuming a recovery code) atomic would require an architectural change to the adapter interface itself, and is considered out of scope for individual adapter fixes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/triggers.js`:
- Around line 40-41: The validator lookup is still using the global
Parse.applicationId while validators are stored per-app via
TriggerStore.addFunction in addFunction; update all local maybeRunValidator(...)
call sites in this file (including the ones around lines noted: the call sites
near addFunction and the other areas referenced) to pass the correct app id from
request.config (i.e., thread config.applicationId into maybeRunValidator), and
where runTrigger or any path that calls maybeRunValidator does not have
request.config, change the caller to explicitly accept and forward the
applicationId (or request.config) so that maybeRunValidator uses the per-app id
rather than Parse.applicationId; ensure runTrigger and related call paths are
updated to propagate request.config.applicationId into the validator lookup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cloud-code/TriggerStore.ts (1)
287-290: Consider error isolation for live query handlers.If any handler throws an exception, subsequent handlers in the array won't execute. This could cause data loss for other listeners in error scenarios.
♻️ Optional: Wrap each handler invocation in try/catch
static runLiveQueryEventHandlers(appId: string, data: unknown): void { const s = TriggerStore._stores.get(appId); - if (s) s.liveQuery.forEach(handler => handler(data)); + if (s) { + s.liveQuery.forEach(handler => { + try { + handler(data); + } catch (err) { + logging.logger.error('Live query event handler error:', err); + } + }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/TriggerStore.ts` around lines 287 - 290, TriggerStore.runLiveQueryEventHandlers currently invokes all handlers from TriggerStore._stores.get(appId)?.liveQuery directly so a thrown exception in one handler aborts the rest; change the loop to call each handler inside its own try/catch so one handler's error does not prevent others from running, and in the catch log the error with context (include appId and a handler identifier like index or handler.name) to aid debugging while continuing to the next handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cloud-code/TriggerStore.ts`:
- Around line 287-290: TriggerStore.runLiveQueryEventHandlers currently invokes
all handlers from TriggerStore._stores.get(appId)?.liveQuery directly so a
thrown exception in one handler aborts the rest; change the loop to call each
handler inside its own try/catch so one handler's error does not prevent others
from running, and in the catch log the error with context (include appId and a
handler identifier like index or handler.name) to aid debugging while continuing
to the next handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e64e2c42-7641-4bcc-b262-48c36d082d5e
📒 Files selected for processing (4)
src/ParseServer.tssrc/cloud-code/TriggerStore.tssrc/cloud-code/validateTrigger.tssrc/triggers.js
✅ Files skipped from review due to trivial changes (1)
- src/cloud-code/validateTrigger.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ParseServer.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## alpha #10256 +/- ##
==========================================
- Coverage 92.53% 92.05% -0.48%
==========================================
Files 192 194 +2
Lines 16481 16497 +16
Branches 226 275 +49
==========================================
- Hits 15251 15187 -64
- Misses 1210 1280 +70
- Partials 20 30 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
spec/TriggerStore.spec.js (1)
3-4: Remove unusedrequestimport.The
requestmodule is imported but never used in this test file.🧹 Remove unused import
const Parse = require('parse/node'); -const request = require('../lib/request');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/TriggerStore.spec.js` around lines 3 - 4, The test file imports an unused module via the line that assigns request (const request = require('../lib/request')); remove that unused import to clean up the test; locate the require for '../lib/request' in TriggerStore.spec.js and delete the const request = ... line (or let the linter auto-fix unused imports) so only used imports like Parse remain.src/cloud-code/TriggerStore.ts (1)
287-290: Consider error isolation for live query event handlers.If one handler throws, subsequent handlers won't execute. This could cause silent failures in production when multiple handlers are registered.
🛡️ Proposed defensive iteration
static runLiveQueryEventHandlers(appId: string, data: unknown): void { const s = TriggerStore._stores.get(appId); - if (s) s.liveQuery.forEach(handler => handler(data)); + if (s) { + for (const handler of s.liveQuery) { + try { + handler(data); + } catch (e) { + logging.logger.error('Live query event handler error:', e); + } + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/TriggerStore.ts` around lines 287 - 290, runLiveQueryEventHandlers currently invokes each handler directly so a thrown exception stops subsequent handlers; modify TriggerStore.runLiveQueryEventHandlers to iterate s.liveQuery and call each handler inside a try/catch block to isolate failures, and in the catch log the error (include appId and any minimal identifying info about the handler) so one bad handler won’t prevent others from running.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/TriggerStore.spec.js`:
- Around line 3-4: The test file imports an unused module via the line that
assigns request (const request = require('../lib/request')); remove that unused
import to clean up the test; locate the require for '../lib/request' in
TriggerStore.spec.js and delete the const request = ... line (or let the linter
auto-fix unused imports) so only used imports like Parse remain.
In `@src/cloud-code/TriggerStore.ts`:
- Around line 287-290: runLiveQueryEventHandlers currently invokes each handler
directly so a thrown exception stops subsequent handlers; modify
TriggerStore.runLiveQueryEventHandlers to iterate s.liveQuery and call each
handler inside a try/catch block to isolate failures, and in the catch log the
error (include appId and any minimal identifying info about the handler) so one
bad handler won’t prevent others from running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14c4db96-2059-49d1-b423-cf4dd97d8be4
📒 Files selected for processing (6)
package.jsonspec/TriggerStore.spec.jssrc/ParseServer.tssrc/cloud-code/TriggerStore.tssrc/cloud-code/validateTrigger.tssrc/triggers.js
✅ Files skipped from review due to trivial changes (1)
- src/cloud-code/validateTrigger.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/triggers.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Summary by CodeRabbit
New Features
Documentation
Tests