feat(model-apps): verify what persona security roles actually grant

[Akshay Maloo (akshaymaloo)]

Two verification gaps in /app-builder, both metadata-only — no new infrastructure, no live-browser dependency.

1. verify now proves what a persona security role GRANTS

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.

The new role-privileges check resolves each declared (entity, access) to its Dataverse PrivilegeId from the same metadata source the SDK writes against, and asserts the role holds it at at least the declared depth.

Subset, not equality — lib/role-privileges.js records why. Equality would false-fail on three legitimate causes:

• appAccess injects appmodule read
• unioned jobs escalate a shared entity+access to the max declared scope
• distinct entities can share one Dataverse privilege (a role holds one depth per privilege)

Fails closed on an unreadable role or table.

Implementation note for reviewers: the read deliberately 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 now 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 no extra reads — reporting a deployed failure as the job it broke ("persona P can no longer do job J") rather than only "view X is missing".

Compatibility

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.

Verification

• 1474 plugin tests pass (1446 → 1474; 28 new), 0 fail
• Both features red-green verified: disabling the lint wiring fails 2 tests; disabling the privilege check fails 3
• 159 eval tests, 6/6 repo validators
• Docs updated per the AGENTS.md map: schema, roadmap, CHANGELOG, file tree

[Copilot]

Pull request overview

This PR adds a proposal-only design note for “JTBD probes” to extend the /app-builder workflow beyond the current handoff point (“Then open the app in the browser”) by making personas[].jobs[].surfaces[] actionable. It also updates the app-builder roadmap to reference the proposal and position it in the “Quality & docs” phase.

Changes:

• Add jtbd-probe-design.md describing a four-rung approach: offline surface resolution → offline deep-link routing → Playwright drive → Dataverse-backed assertions.
• Update app-builder-roadmap.md to include the JTBD probes proposal and link to the new design note.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
plugins/model-apps/docs/jtbd-probe-design.md	New design note proposing an incremental, runged approach to executable JTBD probes, including constraints and anti-goals.
plugins/model-apps/docs/app-builder-roadmap.md	Adds a roadmap item referencing the JTBD probes proposal and links to the new design note.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (4)

plugins/model-apps/docs/jtbd-probe-design.md:133

• Same as above: lib/surface-route.js suggests a directory that doesn’t exist in this plugin. Consider aligning the proposed module location with the established scripts/lib/ convention.

`lib/surface-route.js`: a resolved ref → an MDA deep link.

plugins/model-apps/docs/jtbd-probe-design.md:111

• The proposed module path lib/surface-resolver.js doesn’t match the existing repo layout (there is no plugins/model-apps/lib/; shared code lives under scripts/lib/). Using the correct path in the design note will reduce implementation ambiguity.

This issue also appears on line 133 of the same file.

A new pure module `lib/surface-resolver.js`:

plugins/model-apps/docs/jtbd-probe-design.md:104

• Section numbering is inconsistent: this is labeled “## 3.” after “## 3a.” and “## 3b.” above, which makes references harder to follow in reviews and future discussions. Consider renaming this heading to keep the sequence monotonic.

## 3. Four rungs, each shippable alone

plugins/model-apps/docs/jtbd-probe-design.md:215

• This sentence says to prototype auth before writing any of Rungs 0–3, but earlier the doc explicitly stages Rungs 0–1 to be offline and valuable even if auth never works unattended. Tightening this to “Rungs 2–3” keeps the sequencing internally consistent.

for. **Prototype this before writing any of Rungs 0–3.** If a persisted profile cannot be made to

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

plugins/model-apps/scripts/verify-model-app.js:113

• privilegedepthmask is documented as a bitmask; mapping only the exact values 1/2/4/8 will return an empty depth string if the service ever returns a combined mask (e.g. 3/5/6/7), causing false failures in the role-privileges subset comparison. Translate masks by taking the most-permissive bit set (Global > Deep > Local > Basic).

      const DEPTH_BY_MASK = { 1: 'Basic', 2: 'Local', 4: 'Deep', 8: 'Global' };
      const rows = await sdk.queryRecords('roleprivileges', {
        select: ['privilegeid', 'privilegedepthmask'],
        filter: `roleid eq ${roleId}`,
        top: 5000,

plugins/model-apps/docs/jtbd-probe-design.md:3

• The doc still states "proposal, not yet built" / "Nothing here is implemented", but this PR actually implements rung 0 (surface resolution + lint/verify wiring) and the role privilege-depth verification. Update the status/title to reflect that parts have shipped and clarify which rungs remain proposed, otherwise readers (and the PR description) will be inaccurate.

# JTBD probes — design note (proposal, not yet built)

**Status:** proposal for review. Nothing here is implemented.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (4)

plugins/model-apps/scripts/verify-model-app.js:127

• readerFor() now always exposes entityPrivileges()/rolePrivileges(), but entityPrivileges depends on opts.httpClient. build-model-app.js calls readerFor(provisionSdk, …) without passing httpClient, so role-privilege verification will fail closed for every persona (entityPrivileges throws, entityPrivileges map stays empty, compareRolePrivileges reports every declared privilege as unreadable). Either thread httpClient through build-model-app’s verify reader, or gate these readers so they are only present when an httpClient is supplied.

    entityPrivileges: async (logical) => {
      const name = String(logical).toLowerCase();
      const url = `/EntityDefinitions(LogicalName='${odataLit(name)}')?$select=LogicalName,Privileges`;
      const res = await opts.httpClient.get(url);
      if (!res || res.status < 200 || res.status >= 300) return null;

plugins/model-apps/scripts/lib/surface-resolver.js:67

• declaredSurfaces() has the same malformed-shape hazard: for (const p of spec.personas || []) will throw if personas is a truthy non-array (object/string), and (p && p.jobs) || [] will throw if jobs is truthy non-array. If this helper is meant to be safe on WIP specs, use Array.isArray guards for personas/jobs/surfaces.

function declaredSurfaces(spec) {
  const out = [];
  for (const p of spec.personas || []) {
    for (const j of (p && p.jobs) || []) {
      for (const s of (j && j.surfaces) || []) {
        if (typeof s === 'string' && s.trim()) out.push({ persona: (p.persona || '').trim(), job: (j.name || '').trim(), surface: s.trim() });

plugins/model-apps/scripts/verify-model-app.js:113

• privilegedepthmask is a bitmask; Dataverse can return combined values (e.g., 7 or 15) representing the max depth. Mapping only {1,2,4,8} turns combined masks into an empty depth string, which then reads as “held at , below declared …” and can false-fail privilege checks. Decode the bitmask by taking the highest set bit (Global > Deep > Local > Basic).

      const DEPTH_BY_MASK = { 1: 'Basic', 2: 'Local', 4: 'Deep', 8: 'Global' };
      const rows = await sdk.queryRecords('roleprivileges', {
        select: ['privilegeid', 'privilegedepthmask'],
        filter: `roleid eq ${roleId}`,
        top: 5000,

plugins/model-apps/scripts/lib/surface-resolver.js:55

• buildIndex() iterates with for (const x of spec.<field> || []). If a malformed/in-progress spec has a truthy non-array (e.g., { views: {} }), spec.views || [] yields an object and for..of throws, contradicting the module’s “tolerates a malformed spec” intent. Guard each collection with Array.isArray (similar to spec-lint’s arrOf()) before iterating.

This issue also appears on line 62 of the same file.

  for (const v of spec.views || []) add(v && v.name, { kind: 'view', name: v.name, entity: v.entity });
  for (const f of spec.forms || []) add(f && f.name, { kind: 'form', name: f.name, entity: f.entity });
  for (const d of spec.dashboards || []) add(d && d.name, { kind: 'dashboard', name: d.name });
  for (const p of spec.pages || []) {
    // A page is referenced by stable KEY in schemaVersion 2 and by display name in prose, so accept

[Akshay Maloo (akshaymaloo)]

Superseded by #425 — same commit, on a branch renamed to match the shipped scope. The exploratory design note that was originally part of this PR has been dropped; only the two verification improvements ship.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

plugins/model-apps/scripts/verify-model-app.js:222

• entityPrivileges now relies on opts.envUrl to build an absolute Dataverse Web API URL. The current readerFor(...) call does not pass the env URL, so the privilege metadata read will return null and the new role-privileges check will fail-closed. Pass the env URL through the reader options (and ensure other callers of the exported readerFor do the same).

  const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient }));

plugins/model-apps/scripts/verify-model-app.js:128

• entityPrivileges calls opts.httpClient.get() with a relative URL (/EntityDefinitions...). The injected createAzHttpClient explicitly refuses non-absolute URLs (it expects the SDK-style full URL https://<org>/api/data/v9.x/...), so this will throw and make the role-privileges check fail-closed even in healthy environments.

      const name = String(logical).toLowerCase();
      const url = `/EntityDefinitions(LogicalName='${odataLit(name)}')?$select=LogicalName,Privileges`;
      const res = await opts.httpClient.get(url);
      if (!res || res.status < 200 || res.status >= 300) return null;
      return (res.body && res.body.Privileges) || null;

plugins/model-apps/scripts/lib/role-privileges.js:8

• Comment typo: this line starts with ////, which looks accidental and makes the header harder to read.

//// SUBSET, not equality. We assert the role holds AT LEAST every declared privilege at AT LEAST the