fix(model-apps): never delete a generative page's files ourselves

Review caught a real data-loss path in the genpage teardown step: it
deleted the uxagentprojectfile rows first and the uxagentproject row
second. If the project delete was then dependency-blocked and skipped,
the files were already gone -- leaving the app that still referenced the
page pointing at an empty shell.

Measured against a live environment, which is what makes this concrete:

  - The project row IS dependency-tracked (component type 10372). On
    pages an app sitemap references it reports 1 dependent and DELETE
    returns 400 "cannot be deleted because it is referenced by 1 other
    components".
  - Its files are NOT. Across 20 files of 5 such referenced pages,
    RetrieveDependenciesForDelete returned 0 for every single one, so
    each would have deleted cleanly.
  - The uxagentproject -> uxagentprojectfile relationship is
    CascadeConfiguration Delete=Cascade, so the file loop was never
    needed in the first place.

The fix is therefore a deletion: delete only the project row and let the
platform cascade. A dependency block now happens before anything has
been written, so a skipped page is left completely intact. This also removes a
second defect the reviewer flagged -- a not-found thrown mid-loop aborted
before the project delete while deleteStep recorded the whole page as
successfully deleted, reporting a page gone that was still there.

Both regression tests were verified red against the old ordering before
being taken green; the skip test deliberately supplies files so it cannot
pass vacuously.

Also removes stale comments and test scaffolding describing the cross-app
sitemap scan that was replaced by platform arbitration, and corrects an
assertion message that still claimed deleteAppCascade removes genpages.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 731c1111-d9b6-4fd7-b9d3-347f762ca7f5

Author: akshay-viz

plugins/model-apps/scripts/lib/sdk-teardown.js

@@ -13,8 +13,8 @@
 //                      page is REFERENCED by an app, not owned by one — another app's sitemap, or a
 //                      form's UxAgentControl `RefId` in formxml, can point at the same row — so the
 //                      SDK reports them and the owner decides. Runs AFTER the app so the app's own
-//                      sitemap reference is gone and the cross-app scan only sees genuine other
-//                      consumers; a page any other app still references is SKIPPED, not deleted.
+//                      sitemap reference is already gone and the only dependency the platform can
+//                      still report is a GENUINE other consumer; such a page is SKIPPED, not deleted.
 //   1b. roles        — persona security roles. Deleted right after the app (BEFORE the data model): a
 //                      role holding a soon-to-be-deleted table's privileges could otherwise block that
 //                      table's delete. SEC-1: only roles the SDK itself authored (marked on the role
@@ -220,21 +220,18 @@ const KIND_HANDLERS = {
         return [];
       }
     },
-    // Files first, then the project — the parent cannot be removed while children reference it.
-    // A page still in use fails here with a dependency error, which `deleteStep` records as a SKIP
-    // (see isUndeletable) rather than a teardown failure: another app legitimately owning the page
-    // is an expected outcome, not a broken teardown.
+    // Delete ONLY the project row. Its `uxagentprojectfile` children go with it: the
+    // uxagentproject_uxagentprojectfile_uxagentprojectid relationship is CascadeConfiguration
+    // Delete=Cascade, so the platform removes them for us.
+    //
+    // Deleting the files ourselves first would be actively DESTRUCTIVE. Dataverse tracks a
+    // dependency on the PROJECT row (component type 10372) but NOT on its files (10373):
+    // measured on pages that an app sitemap references, the project reports 1 dependent and its
+    // DELETE is refused, while every one of its files reports ZERO dependents and would delete
+    // cleanly. So a files-first order would strip the content out of a page the platform is about
+    // to refuse to delete, leaving the app that still references it pointing at an empty shell —
+    // exactly the data loss this step exists to avoid. One delete, and the platform decides.
     async del(sdk, item) {
-      const files = await sdk.queryRecords('uxagentprojectfile', {
-        select: ['uxagentprojectfileid'],
-        filter: `_uxagentprojectid_value eq ${item.id}`,
-        paginate: true,
-      });
-      for (const f of files || []) {
-        if (f && f.uxagentprojectfileid) {
-          await sdk.deleteRecord('uxagentprojectfile', f.uxagentprojectfileid);
-        }
-      }
       await sdk.deleteRecord('uxagentproject', item.id);
     },
   },
@@ -455,10 +452,10 @@ function planTeardown(spec) {
     // Generative pages, AFTER the app. The SDK no longer deletes them — a page is
     // referenced by an app, not owned by one, so the SDK reports them and the owner decides. We are
     // the owner: the page manifest records exactly which pages this build authored. Ordered after the
-    // app so the app's own sitemap reference is already gone and the cross-app scan only ever sees a
-    // GENUINE other consumer. Emitted for every app-bearing spec (not gated on spec.pages) so a spec
-    // that dropped its pages still cleans up what it previously created; resolve is a no-op when the
-    // manifest is absent or lists nothing.
+    // app so the app's own sitemap reference is already gone and any dependency the platform still
+    // reports belongs to a GENUINE other consumer. Emitted for every app-bearing spec (not gated on
+    // spec.pages) so a spec that dropped its pages still cleans up what it previously created;
+    // resolve is a no-op when the manifest is absent or lists nothing.
     steps.push({
       kind: 'genpage',
       phase: 'pages',

plugins/model-apps/scripts/tests/sdk-teardown.test.js

@@ -447,7 +447,7 @@ test('app teardown resolves via resolveArtifact and delegates the full cascade t
   await h.del(sdk, items[0]);
   assert.strictEqual(cascadeCalls.length, 1, 'deleteAppCascade called once');
   assert.strictEqual(cascadeCalls[0].appModuleId, 'app-1', 'app module id passed');
-  assert.strictEqual(cascadeCalls[0].appModuleIdUnique, 'u-1', 'unique id passed (deleteAppCascade handles sitemap + genpage internally)');
+  assert.strictEqual(cascadeCalls[0].appModuleIdUnique, 'u-1', 'unique id passed (deleteAppCascade removes the sitemap; generative pages are a separate step)');
 });
 
 test('app teardown skips when app not found (resolve returns [])', async () => {
@@ -992,19 +992,18 @@ test('planTeardown trims the persona name in the role step (matches the SDK-crea
 // owns that decision for the pages IT authored. These pin who deletes what, and when it backs off.
 // ---------------------------------------------------------------------------------------------
 
-// A minimal SDK stand-in for the genpage handler: a page manifest in a web resource, the live
-// uxagentproject/file rows, and the appmodule sitemaps the cross-app scan reads.
-// A minimal SDK stand-in for the genpage handler: a page manifest in a web resource, the live
-// uxagentproject/file rows, and the OTHER apps + sitemaps the cross-app scan walks.
-// `otherApps` is [{ unique, xml }] — the scan enumerates appmodules, then reads each one's sitemap
-// via appmodulecomponent (componenttype 62) -> sitemap row. A real env ALWAYS has system apps, and
-// fetchAppsForPages fail-closes on an empty enumeration, so every case supplies at least one.
-function genpageSdk({ manifestPages = [], livePages = [], files = {}, otherApps = [{ unique: 'system_app', xml: '<SiteMap />' }], sitemapThrows = false } = {}) {
+// A minimal SDK stand-in for the genpage handler: the page manifest (in a web resource) that says
+// which pages this build authored, and the live `uxagentproject` rows used to skip ones already
+// gone. The `uxagentprojectfile` branch exists ONLY so tests can prove the handler never touches
+// it — deleting the project cascades to its files, and deleting them ourselves would gut a page
+// the platform is about to refuse to delete.
+function genpageSdk({ manifestPages = [], livePages = [], files = {} } = {}) {
   const deleted = [];
+  const queried = [];
   const manifestJson = JSON.stringify({ schemaVersion: 1, pages: manifestPages });
-  const byUnique = new Map(otherApps.map((a, i) => [a.unique, { i, xml: a.xml }]));
   const sdk = {
     async queryRecords(entity, opts = {}) {
+      queried.push(entity);
       const filter = String(opts.filter || '');
       if (entity === 'webresource') {
         return [{ content: Buffer.from(manifestJson, 'utf8').toString('base64') }];
@@ -1016,33 +1015,13 @@ function genpageSdk({ manifestPages = [], livePages = [], files = {}, otherApps
         const owner = (/_uxagentprojectid_value eq ([0-9a-f-]+)/i.exec(filter) || [])[1] || '';
         return (files[owner.toLowerCase()] || []).map((id) => ({ uxagentprojectfileid: id }));
       }
-      if (entity === 'appmodule') {
-        const m = /uniquename eq '([^']+)'/.exec(filter);
-        if (m) {
-          const hit = byUnique.get(m[1]);
-          return hit ? [{ appmoduleid: `app-${hit.i}`, appmoduleidunique: `uniq-${hit.i}` }] : [];
-        }
-        // The env-wide enumeration.
-        return otherApps.map((a, i) => ({ appmoduleid: `app-${i}`, appmoduleidunique: `uniq-${i}`, uniquename: a.unique }));
-      }
-      if (entity === 'appmodulecomponent') {
-        const m = /_appmoduleidunique_value eq (\S+)/.exec(filter);
-        return m ? [{ objectid: `sm-${m[1]}`, componenttype: 62 }] : [];
-      }
-      if (entity === 'sitemap') {
-        if (sitemapThrows) throw new Error('sitemap unreadable');
-        const m = /sitemapid eq ([\w-]+)/.exec(filter) || /sm-uniq-(\d+)/.exec(filter);
-        const idx = m ? Number(String(m[1]).replace(/\D/g, '')) : 0;
-        const app = otherApps[idx];
-        return app ? [{ sitemapxml: app.xml }] : [];
-      }
       return [];
     },
     async deleteRecord(entity, id) {
       deleted.push(`${entity}:${id}`);
     },
   };
-  return { sdk, deleted };
+  return { sdk, deleted, queried };
 }
 const PAGE_1 = '11111111-1111-4111-8111-111111111111';
 const PAGE_2 = '22222222-2222-4222-8222-222222222222';
@@ -1056,14 +1035,14 @@ test('genpage resolve returns only pages the manifest says WE authored', async (
   assert.deepStrictEqual(items.map((i) => i.id), [PAGE_1]);
 });
 
-test('genpage del removes the file rows before the project', async () => {
-  const { sdk, deleted } = genpageSdk({ files: { [PAGE_1.toLowerCase()]: ['f1', 'f2'] } });
+// Deleting the project cascades to its files (the uxagentproject -> uxagentprojectfile
+// relationship is CascadeConfiguration Delete=Cascade), so ONE delete is both necessary and
+// sufficient. Deleting files ourselves first would be destructive — see the skip test below.
+test('genpage del deletes ONLY the project row and never touches its files', async () => {
+  const { sdk, deleted, queried } = genpageSdk({ files: { [PAGE_1.toLowerCase()]: ['f1', 'f2'] } });
   await KIND_HANDLERS.genpage.del(sdk, { id: PAGE_1, name: 'Overview' });
-  assert.deepStrictEqual(deleted, [
-    'uxagentprojectfile:f1',
-    'uxagentprojectfile:f2',
-    `uxagentproject:${PAGE_1}`,
-  ]);
+  assert.deepStrictEqual(deleted, [`uxagentproject:${PAGE_1}`]);
+  assert.ok(!queried.includes('uxagentprojectfile'), 'must not enumerate the page files');
 });
 
 // Dataverse is the authority on whether a page is still in use: saving an app that surfaces a page
@@ -1087,11 +1066,41 @@ test('a dependency block is still a FAILURE for kinds that did not opt in', asyn
   await assert.rejects(() => deleteStep({}, handler, [{ id: 'v1' }]), /referenced by/);
 });
 
-test('genpage deletes a page no other app references', async () => {
+// THE regression this ordering exists for. Dataverse tracks a dependency on the page ROW
+// (component type 10372) but NOT on its files (10373): measured on pages an app sitemap
+// references, the project reports 1 dependent and its DELETE is refused, while every one of its
+// files reports ZERO dependents and would delete cleanly. So if the handler deleted files first,
+// a skipped page would be left as an empty shell for the app that still references it.
+test('a still-referenced page is SKIPPED with its files left completely intact', async () => {
+  const err = new Error('The uxagentproject(11111111) component cannot be deleted because it is referenced by 1 other components.');
+  const deleted = [];
+  const sdk = {
+    // The page HAS files — a handler that enumerated and deleted them first would destroy the
+    // content of a page the platform then refuses to delete.
+    async queryRecords(entity) {
+      return entity === 'uxagentprojectfile'
+        ? [{ uxagentprojectfileid: 'f1' }, { uxagentprojectfileid: 'f2' }]
+        : [];
+    },
+    async deleteRecord(entity, id) {
+      deleted.push(`${entity}:${id}`);
+      if (entity === 'uxagentproject') throw err;
+    },
+  };
+  const r = await deleteStep(sdk, KIND_HANDLERS.genpage, [{ id: PAGE_1, name: 'Overview' }]);
+  assert.deepStrictEqual(r.skippedIds, [PAGE_1]);
+  assert.deepStrictEqual(r.deletedIds, []);
+  assert.deepStrictEqual(
+    deleted,
+    [`uxagentproject:${PAGE_1}`],
+    'the page delete must be the ONLY write attempted — its files must survive the skip'
+  );
+});
+
+test('genpage resolve ignores live pages the manifest does not claim', async () => {
   const { sdk } = genpageSdk({
     manifestPages: [{ key: 'overview', name: 'Overview', pageId: PAGE_1 }],
-    livePages: [PAGE_1],
-    otherApps: [{ unique: 'other_app', xml: `<SiteMap><Area><Group><SubArea GenPageId="${PAGE_2}" /></Group></Area></SiteMap>` }],
+    livePages: [PAGE_1, PAGE_2],
   });
   const items = await KIND_HANDLERS.genpage.resolve(sdk, { manifestName: 'new_app_pagemanifest' });
   assert.deepStrictEqual(items.map((i) => i.id), [PAGE_1]);