fix(llmo): guard createOrFindSite from moving site with active enroll…

[Kanishkavijay39]

Summary:

• createOrFindSite was silently reassigning a site to a new org whenever the org IDs differed, regardless of whether the site had active products in its current org
• This matches the same guard already present in PLG onboarding (plg-onboarding.js:1050-1058)
• Now: if getSiteEnrollments() returns any enrollments, we throw an error instead of moving the site
• Sites with no active enrollments continue to be re-parented as before (LLMO-4176 behaviour preserved)

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description. Or if there's no issue created, make sure you
  describe here the problem you're solving.
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

If the PR is changing the API specification:

• make sure you add a "Not implemented yet" note the endpoint description, if the implementation is not ready
  yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
• make sure you add at least one example of the request and response.

If the PR is changing the API implementation or an entity exposed through the API:

• make sure you update the API specification and the examples to reflect the changes.

If the PR is introducing a new audit type:

• make sure you update the API specification with the type, schema of the audit result and an example

Related Issues

Thanks for contributing!

[github-actions]

This PR will trigger a patch release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @Kanishkavijay39,

Strengths

• Guard placement is correct (src/controllers/llmo/llmo-onboarding.js:999-1002): The enrollment check is inserted before setOrganizationId and save, so the guard prevents any partial mutation. A failed check leaves the site object completely unmodified.
• Well-shaped test coverage (test/controllers/llmo/llmo-onboarding.test.js:1260-1291): The new test asserts both the positive (exception thrown with meaningful message) and the negative (setOrganizationId and save were NOT called). The existing re-parent test is updated to stub getSiteEnrollments returning empty, confirming the happy path still works.
• Consistency with existing platform behavior: This mirrors the guard already present in PLG onboarding (plg-onboarding.js:1050-1058), aligning both onboarding paths to the same invariant.
• Well-scoped change: One behavioral fix, one new test, one existing-test fixup. No unrelated refactoring mixed in.

Issues

Important (Should Fix)

Optional chaining on enrollments treats unexpected undefined/null as "no enrollments" - src/controllers/llmo/llmo-onboarding.js:1000

enrollments?.length > 0 - if getSiteEnrollments() resolves to undefined or null (e.g., a bug in the data-access layer where a model method returns nothing instead of an empty array), the optional chaining silently evaluates to false and the re-parent proceeds. This is the exact scenario the guard is meant to prevent: a site with active products gets moved because the enrollment check was inconclusive.

For a guard whose explicit purpose is to prevent data corruption, the failure mode should be loud (fail closed), not permissive (fail open). Suggested fix:

const enrollments = await site.getSiteEnrollments();
if (!Array.isArray(enrollments)) {
  throw new Error(`Unable to verify enrollments for site ${baseURL}; aborting org move.`);
}
if (enrollments.length > 0) {
  throw new Error(`Site ${baseURL} is already associated with another org that has active products and cannot be moved.`);
}

If the team has established that getSiteEnrollments is contractually guaranteed to always return an array, this becomes less pressing - but given that this guard exists specifically to prevent data corruption, failing closed is the safer posture.

Recommendations

• Include org IDs in the error message for operational debuggability: The error currently includes baseURL but not the current or requested org IDs. When this surfaces in logs or Slack notifications, operators need to look up both orgs separately. Consider: Site ${baseURL} belongs to org ${site.getOrganizationId()} with active enrollments and cannot be moved to org ${organizationId}.
• Consider a typed/coded error: Callers further up the stack may want to distinguish "site has enrollments" from other errors programmatically. A custom error code (e.g., err.code = 'SITE_HAS_ACTIVE_ENROLLMENTS') allows upstream handlers to present appropriate user-facing feedback without string-matching.

Assessment

Ready to merge? With fixes
Reasoning: The core logic is correct, the change is minimal and focused, and the test coverage is solid. The optional-chaining concern is worth addressing because a safety guard that fails open under unexpected data-layer behavior contradicts the PR's stated intent. The fix is a one-line addition.

Next Steps

1. Address the optional chaining concern to ensure the guard fails closed on unexpected return values.
2. The recommendations (richer error message, error code) are optional improvements.

[MysticatBot]

Important: enrollments?.length > 0 - if getSiteEnrollments() resolves to undefined or null (data-access bug, model method returning nothing), the optional chaining silently evaluates to false and the re-parent proceeds. For a guard whose purpose is preventing data corruption, the failure mode should be loud:

const enrollments = await site.getSiteEnrollments();
if (!Array.isArray(enrollments)) {
  throw new Error(`Unable to verify enrollments for site ${baseURL}; aborting org move.`);
}
if (enrollments.length > 0) {
  throw new Error(`Site ${baseURL} is already associated with another org that has active products and cannot be moved.`);
}

If the contract guarantees an array, the optional chaining can be dropped in favor of plain .length > 0 so a contract violation surfaces loudly.