Skip to content

Commit 4eb1632

Browse files
akshay-vizCopilot
andcommitted
fix(model-apps): teardown owns generative-page deletion
The SDK's deleteAppCascade no longer deletes an app's generative pages. A uxagentproject is REFERENCED by an app, not owned by one -- another app's sitemap can carry the same GenPageId, and a form can embed the page through the MscrmControls.UxAgentControl PCF's RefId, a reference stored in formxml that appears in no sitemap at all. The SDK reports them instead and leaves the decision to the caller. We are that caller, and we are the one that CREATED the pages. Teardown now deletes them itself, in a new `genpage` step ordered immediately after the app. Ownership is established from the page manifest web resource -- the durable record of exactly which pages this build authored -- not from the app's sitemap, which would also list pages someone else added. The manifest is read before the web-resources phase deletes it. Safety is delegated to Dataverse rather than inferred from a scan. Saving an app that surfaces a page creates a real solution dependency, so deleting that page fails with "cannot be deleted because it is referenced by N other components", whether or not the app is published; the dependency clears only once the subarea is removed and published, or the app and its sitemap are deleted outright (which is what the step before this one just did). So the delete IS the check. Attempting it and reading the platform's answer is better than a pre-flight scan: it is authoritative rather than our model of the truth, it covers every surface the platform tracks instead of just sitemap XML, and it has no TOCTOU window. A dependency block is recorded as a SKIP for this kind (tolerateDependencyBlock) because another app legitimately owning the page is an expected outcome, not a broken teardown -- while staying a genuine failure for every other kind. Files are deleted before the project, since the parent cannot be removed while children reference it. Anything we cannot prove is ours -- an absent or unreadable manifest, a failed existence query -- deletes nothing. Tests: seven new cases covering manifest-scoped selection, delete ordering, the still-referenced skip, the dependency block remaining a failure for kinds that did not opt in, the already-gone page, and the missing manifest. Four existing plan-shape assertions updated for the new step; deleteRecord added to SKILL_SDK_SURFACE so the surface-contract test guards it against the real bundle. Full suite: 1443 pass, 0 fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 731c1111-d9b6-4fd7-b9d3-347f762ca7f5
1 parent 478e4f4 commit 4eb1632

3 files changed

Lines changed: 272 additions & 16 deletions

File tree

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

Lines changed: 127 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
//
99
// Order (each the mirror of the build's create order — dependents before their dependencies):
1010
// 1. app — the app module (references the sitemap + dashboard/form/view/chart components)
11+
// 1a. pages — generative pages (uxagentproject + files) this build AUTHORED, per the page
12+
// manifest. The SDK's deleteAppCascade no longer removes them: a
13+
// page is REFERENCED by an app, not owned by one — another app's sitemap, or a
14+
// form's UxAgentControl `RefId` in formxml, can point at the same row — so the
15+
// SDK reports them and the owner decides. Runs AFTER the app so the app's own
16+
// sitemap reference is gone and the cross-app scan only sees genuine other
17+
// consumers; a page any other app still references is SKIPPED, not deleted.
1118
// 1b. roles — persona security roles. Deleted right after the app (BEFORE the data model): a
1219
// role holding a soon-to-be-deleted table's privileges could otherwise block that
1320
// table's delete. SEC-1: only roles the SDK itself authored (marked on the role
@@ -39,10 +46,10 @@
3946

4047
const { topoOrderEntities } = require('./_graph.js');
4148
const { appUniqueName, commandsByEntity, defaultViewColumns, resolveExistingFormId, resolveRoleBusinessUnit, roleBuClause } = require('./sdk-build.js');
42-
const { manifestResourceName } = require('./page-manifest.js');
49+
const { manifestResourceName, parseManifestBase64 } = require('./page-manifest.js');
4350
const { relationshipSchemaName, manyToManySchemaName, lookupColumnsFor, SDK_ROLE_MARKER, canonicalPersonaName, FORM_GUID_RE } = require('./app-spec.js');
4451
const { selectSummaryTables } = require('./ai-candidates.js');
45-
const { isRestrictedSolution } = require('./system-solutions.js');
52+
const { isRestrictedSolution } = require('./system-solutions.js');
4653

4754
// OData v4 string-literal escaping lives in ./odata.js. `odataStr` is kept as a backward-compatible
4855
// alias because it is part of this module's exported (and unit-tested) surface.
@@ -74,6 +81,17 @@ function isNotFound(err) {
7481
// teardown skips it instead of failing — the same best-effort spirit as isNotFound. Deliberately
7582
// NARROW: it must NOT match a dependency block ("...cannot be deleted because it is referenced by
7683
// N other components"), which is a genuine leftover the teardown must surface, not swallow.
84+
// Dataverse refused a delete because another component still references the record:
85+
// "The <entity>(<id>) component cannot be deleted because it is referenced by N other components."
86+
// For MOST kinds that is a genuine leftover the teardown must surface. For a generative page it is
87+
// the correct, expected answer — the page belongs to whoever still points at it — so only handlers
88+
// that opt in via `tolerateDependencyBlock` treat it as a skip.
89+
function isDependencyBlocked(err) {
90+
if (!err) return false;
91+
const msg = String((err && err.message) || '').toLowerCase();
92+
return /cannot be deleted because it is referenced by/.test(msg) || /referenced by \d+ other component/.test(msg);
93+
}
94+
7795
function isUndeletable(err) {
7896
if (!err) return false;
7997
const msg = String((err && err.message) || '').toLowerCase();
@@ -120,15 +138,18 @@ const KIND_HANDLERS = {
120138
const items = await sdk.resolveArtifact('app', { uniqueName: target.uniqueName });
121139
return (items || []).map((x) => ({ id: x.id, name: x.name, appModuleIdUnique: x.appModuleIdUnique }));
122140
},
123-
// deleteAppCascade fail-fast-deletes the app module, then best-effort cleans up the
124-
// orphaned sitemap + generative-page rows (uxagentproject[file]). It returns a structured
125-
// { success, deleted, failures } result (older vendored bundles returned void). The app
126-
// record itself is gone once this resolves, but an individual child-cleanup step can still
127-
// fail — which the old void contract swallowed, silently leaving orphaned rows while the
128-
// teardown reported a clean delete. Surface any GENUINE child failure so the run reports
129-
// ok=false with the exact leftovers. A not-found child failure means the row already
130-
// cascaded away (not a leftover), so it is tolerated — the same best-effort spirit as the
131-
// step-level isNotFound handling in deleteStep.
141+
// deleteAppCascade fail-fast-deletes the app module together with its sitemap (atomically), and
142+
// returns a structured { success, deleted, failures, retained } result (older vendored bundles
143+
// returned void). It deliberately does NOT delete the app's generative pages — a `uxagentproject`
144+
// is referenced by an app, not owned by one, so it reports them in `retained` and the owner
145+
// decides. The `genpage` step that follows is that decision: it deletes the pages
146+
// THIS build authored, per the page manifest, skipping any another app still references.
147+
//
148+
// The app record itself is gone once this resolves, but a cleanup step can still fail — which the
149+
// old void contract swallowed, silently leaving orphaned rows while teardown reported a clean
150+
// delete. Surface any GENUINE failure so the run reports ok=false with the exact leftovers. A
151+
// not-found failure means the row already cascaded away (not a leftover), so it is tolerated —
152+
// the same best-effort spirit as the step-level isNotFound handling in deleteStep.
132153
async del(sdk, item) {
133154
const result = await sdk.deleteAppCascade(item.id, item.appModuleIdUnique);
134155
const failures = (result && Array.isArray(result.failures) ? result.failures : []).filter(
@@ -144,6 +165,79 @@ const KIND_HANDLERS = {
144165
}
145166
},
146167
},
168+
// Generative pages the build authored. The SDK's `deleteAppCascade` deliberately does NOT delete
169+
// these: a `uxagentproject` is REFERENCED by an app, not owned by one, so the SDK
170+
// reports them in `retained` and leaves the decision to the caller. WE are the caller that CREATED
171+
// them, and the page manifest is the durable record of exactly which pages this build authored —
172+
// so teardown deletes those, and only those.
173+
//
174+
// Safety is delegated to DATAVERSE, not inferred from a scan. Verified against a live environment:
175+
// saving an app that surfaces a page creates a real solution dependency, and DELETE on that page
176+
// returns 400 "component cannot be deleted because it is referenced by N other components" —
177+
// whether or not the app is published. The dependency clears only when the referencing sitemap is
178+
// removed AND published, or when the app+sitemap are deleted outright (which is what the step
179+
// before this one just did).
180+
//
181+
// So the delete IS the check. Attempting it and reading the platform's answer is strictly better
182+
// than a pre-flight scan: it is authoritative (the platform's own dependency graph, not our model
183+
// of it), it covers every surface the platform tracks rather than just sitemap XML, and it has no
184+
// TOCTOU window — a pre-check can go stale between the check and the delete, this cannot.
185+
genpage: {
186+
// A page another app still references is a SKIP, not a failure — see isDependencyBlocked.
187+
tolerateDependencyBlock: true,
188+
async resolve(sdk, target) {
189+
if (typeof sdk.queryRecords !== 'function') return [];
190+
// The manifest lives in a web resource this same teardown deletes later (web-resources phase),
191+
// so it is still readable here.
192+
let manifest = null;
193+
try {
194+
const rows = await sdk.queryRecords('webresource', {
195+
select: ['content'],
196+
filter: `name eq '${odataStr(target.manifestName)}'`,
197+
top: 1,
198+
});
199+
if (rows && rows[0] && rows[0].content) manifest = parseManifestBase64(rows[0].content);
200+
} catch {
201+
// No manifest readable → nothing provably ours → delete nothing. Leaving a row behind is
202+
// recoverable; deleting a page we cannot prove we authored is not.
203+
return [];
204+
}
205+
const authored = [];
206+
for (const p of (manifest && manifest.pages) || []) {
207+
if (p && typeof p.pageId === 'string' && FORM_GUID_RE.test(p.pageId)) {
208+
authored.push({ id: p.pageId, name: p.name || p.key || p.pageId });
209+
}
210+
}
211+
if (!authored.length) return [];
212+
213+
// Only pages that still exist (a re-run, or a maker deleting one by hand, is not a failure).
214+
try {
215+
const filter = authored.map((a) => `uxagentprojectid eq ${String(a.id).toLowerCase()}`).join(' or ');
216+
const rows = await sdk.queryRecords('uxagentproject', { select: ['uxagentprojectid'], filter });
217+
const live = new Set((rows || []).map((r) => String(r.uxagentprojectid).toLowerCase()));
218+
return authored.filter((a) => live.has(String(a.id).toLowerCase()));
219+
} catch {
220+
return [];
221+
}
222+
},
223+
// Files first, then the project — the parent cannot be removed while children reference it.
224+
// A page still in use fails here with a dependency error, which `deleteStep` records as a SKIP
225+
// (see isUndeletable) rather than a teardown failure: another app legitimately owning the page
226+
// is an expected outcome, not a broken teardown.
227+
async del(sdk, item) {
228+
const files = await sdk.queryRecords('uxagentprojectfile', {
229+
select: ['uxagentprojectfileid'],
230+
filter: `_uxagentprojectid_value eq ${item.id}`,
231+
paginate: true,
232+
});
233+
for (const f of files || []) {
234+
if (f && f.uxagentprojectfileid) {
235+
await sdk.deleteRecord('uxagentprojectfile', f.uxagentprojectfileid);
236+
}
237+
}
238+
await sdk.deleteRecord('uxagentproject', item.id);
239+
},
240+
},
147241
dashboard: {
148242
async resolve(sdk, target) {
149243
const items = await sdk.resolveArtifact('dashboard', { name: target.name });
@@ -358,6 +452,19 @@ function planTeardown(spec) {
358452
const steps = [];
359453
if (spec.app && spec.solution) {
360454
steps.push({ kind: 'app', phase: 'app', label: `app module "${spec.app.name}"`, target: { uniqueName: appUniqueName(spec) } });
455+
// Generative pages, AFTER the app. The SDK no longer deletes them — a page is
456+
// referenced by an app, not owned by one, so the SDK reports them and the owner decides. We are
457+
// the owner: the page manifest records exactly which pages this build authored. Ordered after the
458+
// app so the app's own sitemap reference is already gone and the cross-app scan only ever sees a
459+
// GENUINE other consumer. Emitted for every app-bearing spec (not gated on spec.pages) so a spec
460+
// that dropped its pages still cleans up what it previously created; resolve is a no-op when the
461+
// manifest is absent or lists nothing.
462+
steps.push({
463+
kind: 'genpage',
464+
phase: 'pages',
465+
label: 'generative pages authored by this app',
466+
target: { manifestName: manifestResourceName(appUniqueName(spec)) },
467+
});
361468
}
362469
// Persona security roles — deleted right after the app, before the data model (a role holding a
363470
// table's privileges could block that table's delete). The role handler is SEC-1 safe (marker-gated)
@@ -511,6 +618,15 @@ async function deleteStep(sdk, handler, items) {
511618
deletedIds.push(item.id);
512619
continue;
513620
}
621+
if (handler.tolerateDependencyBlock && isDependencyBlocked(err)) {
622+
// The platform refused because something else still references this record. For a
623+
// generative page that is the CORRECT outcome, not a leftover: the page belongs to whoever
624+
// still points at it, and Dataverse is the authority on that (live-measured — saving an app
625+
// that surfaces a page creates the dependency, published or not). Record it as skipped so
626+
// the run stays auditable without reporting a false failure.
627+
skippedIds.push(item.id);
628+
continue;
629+
}
514630
if (isUndeletable(err)) {
515631
// A system/managed artifact (e.g. an auto-generated "Active <Entity>" view that shares
516632
// the spec view's name) — not ours to remove. Record it as skipped without failing.

plugins/model-apps/scripts/tests/sdk-surface-contract.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const SKILL_SDK_SURFACE = [
4949
'createWebResource',
5050
'deleteAppCascade',
5151
'deleteGlobalOptionSet',
52+
'deleteRecord',
5253
'deleteRelationship',
5354
'deleteRemoteArtifact',
5455
'deleteSecurityRole',

0 commit comments

Comments
 (0)