Skip to content

Commit 25610c1

Browse files
akshay-vizCopilot
andcommitted
feat(model-apps): verify what persona roles GRANT, and make JTBD surfaces checkable
Implements the two cheapest items from the JTBD-probe design in this same PR. Both are METADATA-ONLY - no browser, no live-org dependency, no new architecture. 1. verify now proves what a persona role GRANTS, not just that it exists. The `role` check asserted only that a role ROW exists carrying the SDK ownership marker. It never looked at privileges - so a role created with the wrong access, or one whose privilege write failed after the row landed, verified clean. This was the gap I missed in the design note and only found when challenged on what a probe adds over verify. The new `role-privileges` check resolves each declared (entity, access) to its Dataverse PrivilegeId from the SAME metadata source the SDK writes against - EntityDefinitions(...)?$select=Privileges - and asserts the role holds it at AT LEAST the declared depth. SUBSET, not equality, and lib/role-privileges.js records why: appAccess injects appmodule read, unioned jobs escalate a shared entity+access to the max declared scope, and distinct entities can share ONE Dataverse privilege (a role holds one depth per privilege). Equality would fail on all three while telling us nothing true. Fails CLOSED on an unreadable role or table. Note the read does NOT go through sdk.fetchEntityMetadata: that returns a projected shape which drops Privileges entirely, so routing through it would have silently reported every privilege as unreadable. 2. personas[].jobs[].surfaces[] is checked instead of documentary. app-spec.js validated each entry as a non-empty string and stopped; spec-lint warned only when the array was EMPTY. So a job could name "My Open Work Orders" when no such view existed anywhere in the spec and every gate passed. lib/surface-resolver.js resolves each entry against the spec's own views, forms, pages (key OR name), dashboards, tables and sitemap titles. spec-lint WARNS on no match - a warning, never an error, because app-spec.js is loose on purpose: a surface may legitimately name an out-of-the-box artifact this spec does not author. verify adds a `job-surface` rollup - a PURE rollup over checks already computed, so it costs no extra reads - translating a technical failure into the business impact it caused: not "view X is missing" but "persona P can no longer do job J". That closes the static half of the MDA spec's own §7 Category-2 "JTBD reachability" eval, which nothing produced before. Both wire into verifySpec's existing READER-GATED seam (the pattern entityRelationships / commandBar already use), so an existence-only reader behaves exactly as it did before. Tests: 28 new (1446 -> 1474 pass, 0 fail). Both features red-green verified: disabling the lint wiring fails 2, disabling the privilege check fails 3. Evals 159 pass, 6/6 validators. Docs updated per the AGENTS.md map (schema, roadmap, CHANGELOG, file tree). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42626da2-b66f-4162-acaa-b1127ef23d89
1 parent deded14 commit 25610c1

11 files changed

Lines changed: 673 additions & 17 deletions

File tree

plugins/model-apps/AGENTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,9 @@ scripts/
423423
sdk-teardown.js ← app-builder teardown engine (planTeardown is pure)
424424
sdk-http-client.js ← az-token HttpClient for the vendored SDK
425425
spec-lint.js / app-spec.js ← App Spec guardrail lint + validation
426+
spec-shape.js ← shared structural normalization for both authoring gates
427+
surface-resolver.js ← pure: resolve personas[].jobs[].surfaces[] to the spec artifacts that satisfy them
428+
role-privileges.js ← pure: declared persona privileges + subset comparison against a deployed role
426429
odata.js ← OData literal escaping helpers
427430
genpage-cli.js ← pac model genpage upload/list/download wrapper
428431
hydrate-spec.js ← reconstruct an App Spec from a deployed app (edit flow)

plugins/model-apps/CHANGELOG.md

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,29 @@ All notable changes to the **model-apps** plugin.
44

55
## [Unreleased] — 2.4.4
66

7-
Adds plugin update notices, fixes four crash paths, and corrects a smoke-eval
8-
assertion that could never pass live.
7+
Adds plugin update notices, proves what persona roles actually grant, makes
8+
jobs-to-be-done surfaces checkable, fixes four crash paths, and corrects a
9+
smoke-eval assertion that could never pass live.
910

1011
### Added
12+
- **`verify` now proves what a persona security role GRANTS, not just that it
13+
exists.** The `role` check only asserted a role row carrying the SDK ownership
14+
marker, so a role built with the wrong access — or one whose privilege write
15+
failed after the row landed — verified clean. The new `role-privileges` check
16+
resolves every declared `(entity, access)` to its Dataverse `PrivilegeId` from
17+
the same metadata source the SDK writes against, and asserts the role holds it
18+
at **at least** the declared depth. A **subset** check by design: extra
19+
privileges are never a finding, because `appAccess` injects `appmodule` read,
20+
unioned jobs escalate a shared entity+access to the max declared scope, and
21+
distinct entities can share one Dataverse privilege. Fails **closed** on an
22+
unreadable role or table. Reader-gated, so existence-only callers are unchanged.
23+
- **`personas[].jobs[].surfaces[]` is checked instead of documentary.** Each entry
24+
is now resolved against the spec's own views, forms, pages, dashboards, tables
25+
and sitemap titles. `spec-lint` **warns** when a surface matches nothing — a
26+
warning, not an error, because a surface may legitimately name an out-of-the-box
27+
artifact this spec never authors. `verify` adds a `job-surface` rollup that
28+
reports a deployed failure as the job it broke ("persona P can no longer do job
29+
J"), rather than only "view X is missing".
1130
- **Automatic plugin update notice.** Every user-invocable skill now runs the
1231
non-blocking `scripts/check-version.js` preflight, which compares the installed
1332
Model Apps version with `origin/main` and shows update commands for the active

plugins/model-apps/docs/app-builder-roadmap.md

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -231,17 +231,27 @@ Source: `IMPROVEMENTS-07-15-app-builder.md` (Project Management V1/V2 diff + a s
231231
- 🔲 **Spec templates** — domain starters (support desk, CRM, asset tracking) as one-shot scaffolds.
232232

233233
### Phase: Quality & docs
234-
- 🔲 **JTBD probes — close the headless loop (design proposed, not built).** The build is
235-
machine-checked end to end right up to `SKILL.md`'s *"Then open the app in the browser"*, where a
236-
human takes over — which caps how far the skill iterates on its own, and is the same gap the MDA
237-
spec's §5 caveat names from the other side (a ✅ primitive ≠ a coherent app). Proposal:
238-
make `personas[].jobs[].surfaces[]` **executable** instead of documentary, in four independently
239-
shippable rungs — resolve surfaces (pure/offline) → route to MDA deep links (pure/offline) → drive
240-
via the already-configured Playwright MCP → assert the row landed in Dataverse. Rungs 0–1 need no
241-
live org and close the static half of the spec's §7 Category 2 "JTBD reachability" eval on their
242-
own. **Auth is the gating unknown** (`az`/`pac` issue Web API tokens; UCI needs a browser session,
243-
and no persisted profile is configured today) — prototype it before building anything downstream.
244-
See [`docs/jtbd-probe-design.md`](./jtbd-probe-design.md).
234+
-**JTBD probes — step 1 + rung 0 shipped (2026-08-14).** Two of the cheapest items from
235+
[`docs/jtbd-probe-design.md`](./jtbd-probe-design.md), both **metadata-only, no browser**:
236+
- **`verify` now proves what a persona role GRANTS, not just that it exists.** The `role` check
237+
only asserted a row carrying the SDK marker, so a role built with the wrong access — or one whose
238+
privilege write failed after the row landed — verified clean. The new `role-privileges` check
239+
resolves each declared `(entity, access)` to its Dataverse `PrivilegeId` from the **same
240+
metadata source the SDK writes against** and asserts the role holds it at **at least** the
241+
declared depth. Deliberately a **subset** check (`lib/role-privileges.js` explains why equality
242+
would false-fail on `appAccess` injection, max-scope union, and shared privileges), and
243+
**fail-closed** on an unreadable role or table.
244+
- **`surfaces[]` is no longer documentary.** `lib/surface-resolver.js` resolves every
245+
`personas[].jobs[].surfaces[]` entry against the spec's own views/forms/pages/dashboards/tables
246+
/sitemap titles; `spec-lint` **warns** when one matches nothing (a warning, not an error — a
247+
surface may legitimately name an OOB artifact), and `verify` adds a `job-surface` rollup that
248+
reports a *deployed* failure as the job it broke. That closes the static half of the MDA spec's
249+
§7 Category-2 "JTBD reachability" eval, which nothing produced before.
250+
- **Still open from that design:** the deep-link routing (rung 1) and the browser-driven rungs
251+
2–3, both gated on the unattended-auth prototype. Note a v1 probe driving as the *author* (an
252+
admin) proves reachability, **not** authorization — a persona test user is the core of that work.
253+
- 🔲 **JTBD probes — rungs 1–3 (design proposed, not built).** See
254+
[`docs/jtbd-probe-design.md`](./jtbd-probe-design.md) §6 for the gating unknowns (auth first).
245255
-**Sample-run UX fixes (2026-07-27, from a live Property-Listings build).**
246256
-**#1 live build status.** A long build now writes `<workspace>/.maker-workspace/build-status.json` (a single-object snapshot — `state`/`steps`/`lastPhase`/`lastLabel` — overwritten every step) alongside the `build-log.jsonl` trace, and prints a `▸ live progress:` path at start. So a multi-minute build is observable even when the launching shell buffers stdout. SKILL.md now tells the agent to stream (Tee, not Select-Object) and read the status file.
247257
-**#2 wireframes shown.** SKILL.md Phase-1 preview step now REQUIRES pasting the `preview-app.js`/`preview-form.js` wireframe output to the user (not summarizing "looks right") before the approval gate — the user must see the forms/sitemap/pages they approve.

plugins/model-apps/references/app-spec-schema.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ privilege removes it — the role converges to the spec).
605605
**Field reference**
606606
- `persona` (**required**) — the security role's display name; also its idempotency key. Must be unique across `personas[]`.
607607
- `jobs[]` (**required**, ≥1) — `{ name, description?, surfaces?, privileges[] }`. `privileges[]` is required and non-empty per job.
608-
- `jobs[].surfaces[]` (optional) — the view/form/page names (or page `key`s) that let this persona **do** the job. Documentary only: it is never applied to Dataverse. It renders the jobs→surfaces traceability table in `model-app-plan.md`, and a job with no `surfaces[]` is flagged by `spec-lint.js` as a design gapnothing in the app demonstrably lets that persona do that job.
608+
- `jobs[].surfaces[]` (optional) — the view/form/page names (or page `key`s) that let this persona **do** the job. Never applied to Dataverse. It renders the jobs→surfaces traceability table in `model-app-plan.md`; a job with no `surfaces[]` is flagged by `spec-lint.js` as a design gap, and a surface that **matches nothing this spec builds** is flagged too (`lib/surface-resolver.js` resolves each entry against `views[]` / `forms[]` / `pages[]` (key **or** name) / `dashboards[]` / `entities[]` / sitemap subarea titles, case-insensitively). Both are **warnings**, never errors — a surface may legitimately name an out-of-the-box artifact this spec does not author. `verify-model-app` additionally rolls a *deployed* failure up to the job that depended on it (`job-surface`), so "view X is missing" also reads as "persona P can no longer do job J".
609609
- `privileges[].entity` (**required**) — a table **logical name** (e.g. `account`, `msdyn_workorder`). May be a table this spec doesn't author (standard/system tables are common); existence is resolved against live metadata by the build, not at lint time.
610610
- `privileges[].access` (**required**) — one or more of `read · create · write · delete · append · appendTo · assign · share`.
611611
- `privileges[].scope` (optional, default `user`) — `user` (Basic) · `businessUnit` (Local) · `parentChild` (Deep) · `organization` (Global), least→most permissive.
@@ -627,6 +627,15 @@ of the **same name in the same business unit** share one role by design (the sec
627627
first's) — give personas distinct names, or a distinct `businessUnitId`, if you need separate roles. In
628628
`--changed-only` mode a persona change forces a **full build** (there is no partial security apply yet).
629629
630+
**Verification.** `verify-model-app` proves the role **row** exists carrying the SDK ownership marker
631+
(`role`) *and* — when the reader supplies role/entity privilege access — that the role actually
632+
**grants** every declared privilege at **at least** the declared depth (`role-privileges`). The depth
633+
comparison is a **subset** check by design: extra privileges are never a finding, because `appAccess`
634+
injects `appmodule` read, unioned jobs escalate a shared entity+access to the max declared scope, and
635+
distinct entities can share one Dataverse privilege (a role holds one depth per privilege). It fails
636+
**closed** — an unreadable role, or a table whose privilege metadata cannot be read, is reported
637+
rather than skipped.
638+
630639
**Validation rules** (`validateAppSpec`): `persona` required + unique; each job needs a `name` and a
631640
non-empty `privileges[]`; `access` values and `scope` must be valid tokens; `appAccess` must be a
632641
boolean; `businessUnitId` and `assignTo` ids must be GUIDs. Two apply-time checks need live metadata and
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// plugins/model-apps/scripts/lib/role-privileges.js
2+
// PURE: what privileges a persona's security role MUST hold, and whether a deployed role holds them.
3+
//
4+
// WHY this exists. `verifySpec`'s role check proved only that a role ROW exists carrying the SDK
5+
// ownership marker. It never looked at the role's privileges — so a role created with the wrong
6+
// access, or one whose privilege write silently failed after the row was created, verified clean.
7+
// That is the cheapest gap in the JTBD-probe design (docs/jtbd-probe-design.md §3b): it needs a
8+
// metadata read, not a browser.
9+
//
10+
// SUBSET, not equality. We assert the role holds AT LEAST every declared privilege at AT LEAST the
11+
// declared depth. Extra privileges are never a finding, and that is deliberate — three legitimate
12+
// sources add privileges the spec does not literally list:
13+
// 1. `appAccess` injects `appmodule` read (see personaRoleSpecFor).
14+
// 2. Several jobs unioned together escalate a shared entity+access to the MAX declared scope.
15+
// 3. Distinct entities can share ONE Dataverse privilege, and a role holds one depth per
16+
// privilege — so the SDK raises that privilege to the highest scope any of them asked for.
17+
// An equality check would fail on all three while telling us nothing true.
18+
//
19+
// Dataverse reference — privilege depth (`Depth` on ReplacePrivilegesRole, `RolePrivilegeDepth`):
20+
// Basic (user) < Local (business unit) < Deep (parent/child) < Global (organization).
21+
// https://learn.microsoft.com/en-us/power-apps/developer/data-platform/security-model
22+
'use strict';
23+
24+
// App Spec scope -> Dataverse depth name, and its ORDER. The order is what makes this a subset
25+
// check: a role holding Global satisfies a declared Basic. Mirrors the vendored SDK's own mapping
26+
// (verified against scripts/vendor/cds-maker-sdk.cjs) so a comparison cannot disagree with the write.
27+
const SCOPE_DEPTH = { user: 'Basic', businessUnit: 'Local', parentChild: 'Deep', organization: 'Global' };
28+
const DEPTH_RANK = { basic: 1, local: 2, deep: 3, global: 4 };
29+
// App Spec access token -> Dataverse PrivilegeType, again mirroring the SDK.
30+
const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' };
31+
32+
const rankOf = (depth) => DEPTH_RANK[String(depth == null ? '' : depth).trim().toLowerCase()] || 0;
33+
34+
// Flatten a persona to the (entity, access, scope) triples its role must satisfy, taking the MAX
35+
// scope per (entity, access) exactly as the builder's union does. `appAccess` is folded in here so
36+
// the expectation matches what the build actually writes rather than what the author typed.
37+
function declaredPrivileges(persona) {
38+
const byKey = new Map(); // "<entity>|<access>" -> { entity, access, scope }
39+
const addAll = (list) => {
40+
for (const pr of list || []) {
41+
if (!pr || !pr.entity) continue;
42+
const entity = String(pr.entity).trim().toLowerCase();
43+
const scope = pr.scope || 'user';
44+
for (const a of pr.access || []) {
45+
const access = String(a).trim();
46+
if (!access) continue;
47+
const key = `${entity}|${access.toLowerCase()}`;
48+
const prev = byKey.get(key);
49+
// Max scope wins — the same rule the builder applies when unioning jobs into one role.
50+
if (!prev || rankOf(SCOPE_DEPTH[scope]) > rankOf(SCOPE_DEPTH[prev.scope])) byKey.set(key, { entity, access, scope });
51+
}
52+
}
53+
};
54+
for (const j of (persona && persona.jobs) || []) addAll(j && j.privileges);
55+
addAll(persona && persona.additionalPrivileges);
56+
// Mirrors personaRoleSpecFor: unless the persona opts out, the build grants appmodule read so the
57+
// app actually opens for them. Verifying it matters — without it the role exists but the app does not.
58+
if (!persona || persona.appAccess !== false) addAll([{ entity: 'appmodule', access: ['read'], scope: 'organization' }]);
59+
return [...byKey.values()];
60+
}
61+
62+
// Compare declared privileges against what the role actually holds.
63+
// `entityPrivileges`: Map<entityLogical, [{ Name, PrivilegeId, PrivilegeType }]> — from
64+
// EntityDefinitions(LogicalName='x')?$select=Privileges, the SAME source the
65+
// SDK resolves against when it writes.
66+
// `actualByPrivilegeId`: Map<privilegeId(lowercased), depthName>
67+
// Returns { ok, missing:[{ entity, access, scope, reason, privilegeName? }] }.
68+
// An entity whose metadata could not be read is reported as a finding, never skipped — a read
69+
// failure must not read as "nothing missing" (fail closed).
70+
function compareRolePrivileges(declared, entityPrivileges, actualByPrivilegeId) {
71+
const missing = [];
72+
for (const d of declared) {
73+
const privs = entityPrivileges.get(d.entity);
74+
if (!privs) {
75+
missing.push({ ...d, reason: `could not read privilege metadata for '${d.entity}'` });
76+
continue;
77+
}
78+
const type = ACCESS_TYPE[d.access.toLowerCase()];
79+
const p = type && privs.find((x) => x && x.PrivilegeType === type);
80+
if (!p) {
81+
missing.push({ ...d, reason: `'${d.entity}' exposes no '${d.access}' privilege` });
82+
continue;
83+
}
84+
const held = actualByPrivilegeId.get(String(p.PrivilegeId || '').trim().toLowerCase());
85+
if (!held) {
86+
missing.push({ ...d, privilegeName: p.Name, reason: `role does not hold ${p.Name}` });
87+
continue;
88+
}
89+
const want = SCOPE_DEPTH[d.scope] || 'Basic';
90+
if (rankOf(held) < rankOf(want)) {
91+
missing.push({ ...d, privilegeName: p.Name, reason: `role holds ${p.Name} at ${held}, below the declared ${want}` });
92+
}
93+
}
94+
return { ok: missing.length === 0, missing };
95+
}
96+
97+
module.exports = { declaredPrivileges, compareRolePrivileges, SCOPE_DEPTH, ACCESS_TYPE, DEPTH_RANK };

plugins/model-apps/scripts/lib/spec-lint.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// relationship schema-name vs lookup-name collision Dataverse rejects.
55
const { relationshipSchemaName, relationshipFor, invalidChoiceSampleTokens, isPlatformIconRef } = require('./app-spec.js');
66
const { normalizeSpecShape } = require('./spec-shape.js');
7+
const { resolveSurfaces, unresolvedSurfaceMessage } = require('./surface-resolver.js');
78

89
const CHOICE_OPTION_WARN = 12;
910
const SEQNUM_RE = /\{SEQNUM(:\d+)?\}/i;
@@ -390,6 +391,11 @@ function lintAppSpec(spec) {
390391
W(`persona "${persona}" job "${job.name}" is not mapped to a surface (jobs[].surfaces[]) — nothing in this app demonstrably lets that persona do the job.`);
391392
}
392393
}
394+
// A surface that names nothing this spec builds is the NEXT failure after "no surfaces at all":
395+
// the job claims coverage that does not exist. Only a warning, because a surface may legitimately
396+
// name an out-of-the-box artifact this spec never authors (the same reason app-spec.js validates
397+
// surfaces as shape-only) — see lib/surface-resolver.js.
398+
for (const u of resolveSurfaces(spec).unresolved) W(unresolvedSurfaceMessage(u));
393399
if (!(Array.isArray(spec.pages) && spec.pages.length)) {
394400
W('no pages[] — per the genpage-first policy, non-record surfaces (overview/landing, dashboard, analytics, guided or wizard flows) should be generative pages. If this app is genuinely record-CRUD only, ignore this.');
395401
}

0 commit comments

Comments
 (0)