fix(preflight): address SITES-48037 review — split null-org from empty-imsOrgId, empty-imsOrgId → 400

@MysticatBot (must-fix): null org and empty imsOrgId are different data-quality
issues with different remediation paths — a null Organization means the site's
FK is dangling (needs data cleanup), while an empty imsOrgId means the org
exists but wasn't fully onboarded (needs config completion). Split into two
distinct branches with distinct log messages and error responses.

@bhellema: `!hasText(imsOrgId)` is now 400 (`PREFLIGHT_INVALID_REQUEST`), not
500. The empty imsOrgId is a config gap that the caller's admin has to
resolve; it isn't a retryable server error. Kept `Organization.findById`
throw (transient DB error) and `Organization not found` (broken FK) as 500
because those are genuinely server-side.

@MysticatBot (non-blocking): cached `site.getOrganizationId()` in `orgId` to
avoid re-calling the getter (and the theoretical re-throw risk in the catch
block if the getter itself fails). Test resilience: swapped
`expect(fetchStub.secondCall).to.be.null` for `expect(fetchStub).to.not.have.been.called`
— cleaner and independent of the HEAD probe's positioning.

Deferred (own follow-ups):
- @johobot: hasAccess(site) already loads the Organization; explore whether
  the ORM caches it on the site entity for reuse via site.getOrganization().
  Worth verifying caching semantics before threading it through.
- @MysticatBot: callMysticatAnalyze positional-param count. Refactor to
  options-object; own PR to keep this one focused.

Tests updated to match:
- "empty imsOrgId" → 400 + PREFLIGHT_INVALID_REQUEST
- "null org" → 500 + "Site organization not found" (distinct message)
- Assertion swap: not.have.been.called instead of secondCall.to.be.null

Author: Guy Pelletier

src/controllers/preflight.js

@@ -528,22 +528,34 @@ function PreflightController(ctx, log, env) {
     // source of truth for the site → organization mapping; mysticat requires
     // us to supply it and does not fall back to any local projection.
     //
-    // Fail-closed on Organization load failure or missing/empty imsOrgId —
-    // mysticat's analyze route rejects the request with 422 if the field is
-    // absent, so let-it-through-and-fail-late would just defer the same 500
-    // by one hop. Better to surface the data issue here where the error
-    // message can name the site and organization directly.
-    let imsOrgId;
+    // Three distinct failure modes, each with its own remediation path:
+    //   1. Organization.findById throws (DB unreachable / transient) → 500;
+    //      retryable, alerting-worthy.
+    //   2. Organization row not found (FK integrity issue — dangling
+    //      site.organization_id) → 500; not retryable, needs data cleanup.
+    //   3. Organization exists but imsOrgId is empty/missing (config gap —
+    //      site is registered but its parent org wasn't fully onboarded) →
+    //      400; not retryable by us, the caller's admin needs to populate the
+    //      org's imsOrgId before this site can preflight.
+    //
+    // Distinguishing (2) and (3) in the log message matters for triage —
+    // they surface different data-quality issues with different owners.
+    const orgId = site.getOrganizationId();
+    let organization;
     try {
-      const organization = await dataAccess.Organization.findById(site.getOrganizationId());
-      imsOrgId = organization?.getImsOrgId();
+      organization = await dataAccess.Organization.findById(orgId);
     } catch (e) {
-      log.error(`[Preflight] Failed to load Organization ${site.getOrganizationId()} for site ${siteId}: ${e.message}`);
+      log.error(`[Preflight] Failed to load Organization ${orgId} for site ${siteId}: ${e.message}`);
       return preflightError('PREFLIGHT_INTERNAL_ERROR', 'Failed to resolve site organization', 500);
     }
+    if (!organization) {
+      log.error(`[Preflight] Organization ${orgId} not found for site ${siteId} — dangling FK, needs data cleanup.`);
+      return preflightError('PREFLIGHT_INTERNAL_ERROR', 'Site organization not found', 500);
+    }
+    const imsOrgId = organization.getImsOrgId();
     if (!hasText(imsOrgId)) {
-      log.error(`[Preflight] Organization ${site.getOrganizationId()} for site ${siteId} has no imsOrgId — cannot proceed with preflight (mysticat requires it).`);
-      return preflightError('PREFLIGHT_INTERNAL_ERROR', 'Site organization is missing imsOrgId', 500);
+      log.error(`[Preflight] Organization ${orgId} for site ${siteId} has no imsOrgId — org not fully onboarded; cannot proceed with preflight (mysticat requires it).`);
+      return preflightError('PREFLIGHT_INVALID_REQUEST', 'Site organization is missing imsOrgId', 400);
     }
 
     // Validate the URL belongs to this site

test/controllers/preflight.test.js

@@ -1649,11 +1649,11 @@ describe('Preflight Controller', () => {
       expect(mockDataAccess.Organization.findById).to.have.been.calledWith('org-123');
     });
 
-    it('returns 500 when the Organization has no imsOrgId (SITES-48037 fail-closed)', async () => {
-      // Empty imsOrgId cannot be forwarded to mysticat (which now rejects the
-      // request without it). Fail here where the log line can name the site
-      // and organization; deferring the same failure by one hop to mysticat's
-      // 422 wouldn't add diagnostic value.
+    it('returns 400 when the Organization has no imsOrgId (SITES-48037 fail-closed)', async () => {
+      // Empty imsOrgId is a config gap — the site is registered but its parent
+      // org wasn't fully onboarded. 400 (not 500) because this isn't a
+      // retryable server error; the caller's admin needs to populate the org's
+      // imsOrgId before this site can preflight.
       mockDataAccess.Organization.findById.resolves({
         getId: () => 'org-123',
         getImsOrgId: () => '',
@@ -1663,12 +1663,15 @@ describe('Preflight Controller', () => {
         data: { url: 'https://main--example-site.aem.page/test.html' },
         attributes: { authInfo: mockAuthInfo },
       });
-      expect(response.status).to.equal(500);
+      expect(response.status).to.equal(400);
       const result = await response.json();
-      expect(result.errorCode).to.equal('PREFLIGHT_INTERNAL_ERROR');
+      expect(result.errorCode).to.equal('PREFLIGHT_INVALID_REQUEST');
       expect(result.message).to.equal('Site organization is missing imsOrgId');
-      // Never reached the mysticat call.
-      expect(fetchStub.secondCall).to.be.null;
+      // Fail-closed happens after hasAccess but BEFORE the HEAD probe (which
+      // is fetch call 1) and the mysticat call (fetch call 2). Assert fetch
+      // was never invoked — cleaner than `secondCall.to.be.null` and order-
+      // independent of any future HEAD-probe placement changes.
+      expect(fetchStub).to.not.have.been.called;
     });
 
     it('returns 500 when Organization.findById throws (SITES-48037)', async () => {
@@ -1684,10 +1687,13 @@ describe('Preflight Controller', () => {
       expect(result.message).to.equal('Failed to resolve site organization');
     });
 
-    it('returns 500 when Organization.findById returns null (SITES-48037)', async () => {
-      // Site row exists but its Organization row was deleted / never existed.
-      // The optional-chain in the controller (`organization?.getImsOrgId()`)
-      // handles this; the missing-imsOrgId branch takes over.
+    it('returns 500 when Organization.findById returns null — dangling FK (SITES-48037)', async () => {
+      // Distinct from the empty-imsOrgId case: null org = data-integrity issue
+      // at the FK level (site.organization_id points to a row that doesn't
+      // exist — needs cleanup). Empty imsOrgId = config gap at the field
+      // level (needs org onboarding). Different remediation paths → different
+      // error messages, and 500 (not 400) because it's genuinely broken data
+      // in our tables that no client action can fix.
       mockDataAccess.Organization.findById.resolves(null);
       const response = await preflightController.createPreflight({
         params: { siteId: 'test-site-123' },
@@ -1697,7 +1703,7 @@ describe('Preflight Controller', () => {
       expect(response.status).to.equal(500);
       const result = await response.json();
       expect(result.errorCode).to.equal('PREFLIGHT_INTERNAL_ERROR');
-      expect(result.message).to.equal('Site organization is missing imsOrgId');
+      expect(result.message).to.equal('Site organization not found');
     });
 
     it('does not include x-page-auth header when HEAD returns 200 (no page-auth needed)', async () => {