diff --git a/.github/workflows/validate-repository-metadata.yml b/.github/workflows/validate-repository-metadata.yml index aed6507fa..e092575e4 100644 --- a/.github/workflows/validate-repository-metadata.yml +++ b/.github/workflows/validate-repository-metadata.yml @@ -10,6 +10,7 @@ jobs: runs-on: ubuntu-latest env: POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT: "1" + POWER_PLATFORM_SKILLS_TELEMETRY_MODEL_APPS_OPTOUT: "1" steps: - name: checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 @@ -36,3 +37,13 @@ jobs: - name: validate-secure-process-execution run: node scripts/validate-secure-process-execution.js + + # This repository is public, and the genpage eval fixtures are captured + # agent transcripts that will faithfully record whatever live environment + # an eval was run against. Guarding this in CI is the only reliable + # control: a pasted transcript looks correct to a reviewer. + - name: test-no-real-environments-validator + run: node --test scripts/tests/validate-no-real-environments.test.js + + - name: validate-no-real-environments + run: node scripts/validate-no-real-environments.js diff --git a/AGENTS.md b/AGENTS.md index 7b95a02fb..c2a4ce52f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,6 +6,52 @@ This file provides guidance to AI Agents when working with code in this reposito A **plugin marketplace** for Power Platform development by Microsoft. The Open Plugins marketplace manifest (`marketplace.json`) references individual plugins in `plugins/`. Each plugin has its own `AGENTS.md` with plugin-specific guidance. +## This Repo Is PUBLIC — keep internal detail out of it + +Everything here — code, docs, commit messages, PR descriptions, and branch names — is world-readable. +Write for a reader outside Microsoft who cannot see any internal system. + +**Do not commit:** + +- **Internal hosts, repos or paths** — `*.ghe.com` links, internal repo names, or a path into an + internal repo's doc tree. Naming one discloses its existence and its structure. +- **References to internal documents**, including indexes into them — "spec rank 14", + "Group N P1", "(resolves C2, I1)", "see the R1 review". A reader who cannot open the document + gets nothing from the pointer, and it advertises the document. +- **Real environment, tenant or org identifiers** — Dataverse environment names/URLs, tenant GUIDs, + subscription ids. These name real infrastructure. In examples use an obvious placeholder + (`https://contoso.crm.dynamics.com`, ``); to record that something was live-verified, keep + the **claim** and drop the environment ("live-verified", not "live-verified on "). +- **Internal review process** — who or which model reviewed something, how many rounds, internal + finding ids. "Adversarially reviewed" is a useful signal; the rest is internal. +- **Exploratory design docs for UNBUILT work** — roadmaps, prioritisation, and candid notes on + limitations. A design doc for something that **shipped** is fine and often valuable (see + `plugins/model-apps/docs/`); a proposal for something that has not is internal. +- **Pointers to files that are not in the repo** — a citation nobody can resolve is noise at best. + +**Fine to commit:** ADO / `AB#` work-item ids (opaque, and `AB#` is the standard Azure Boards ↔ +GitHub link syntax), and the location of a first-party source repo a maintainer needs in order to +rebuild a vendored bundle. Both carry real "why" context and disclose no content. + +**When you must record internal context**, put it in the PR conversation or an internal doc — not in +a committed file. When editing an existing doc, keep this rule in mind for the lines around your +change, not just the ones you add. + +**CI enforcement (partial).** `node scripts/validate-no-real-environments.js` (wired into the +`validate-repository-metadata` workflow) fails the build when a real Dataverse host, tenant, or +previously-removed identifier appears under `plugins/model-apps/**` or `evals/model-apps/**`. It +matches on *shape* — `org<8 hex>` is what Dataverse auto-generates, so it is rejected even though it +starts with the otherwise-allowed word `org` — rather than only re-catching known strings. Run it +locally after touching eval fixtures or any file that quotes an environment URL. + +The scan is **scoped to model-apps only**, and this is a real gap rather than an oversight: other +plugins still carry pre-existing references of this class (for example real `org<8 hex>` orgs cited +in power-pages provenance comments), so widening the scan today would fail unrelated PRs. Scrub a +plugin first, then add it to `SCAN_PATHS`. The guard also cannot see the *local part* of a UPN, so +`firstname.lastname@contoso.onmicrosoft.com` passes — use a role word (`maker@`, `tester@`). +Captured `pac auth list` transcripts are the most common source of all three; when scrubbing one, +prefer an **equal-length** placeholder so the fixed-width table stays aligned. + ## Repository Structure ``` diff --git a/evals/model-apps/app-builder/EVAL_GUIDE.md b/evals/model-apps/app-builder/EVAL_GUIDE.md index 83a4c3a08..32da95ac8 100644 --- a/evals/model-apps/app-builder/EVAL_GUIDE.md +++ b/evals/model-apps/app-builder/EVAL_GUIDE.md @@ -1,7 +1,7 @@ # App-Builder Offline Structural Eval Harness — Guide > Companion to `evals/model-apps/genpage/EVAL_GUIDE.md`. -> Source of truth: `plugins/model-apps/docs/app-builder-staged-flow-design.md` §13. +> Source of truth: `plugins/model-apps/docs/app-builder-design.md` §13. ## What we evaluate @@ -117,5 +117,5 @@ The live tier (`plugins/model-apps/scripts/smoke-eval.js`) is the thin live smok ## Cross-links - Plugin `AGENTS.md` → *Eval Suite* -- `plugins/model-apps/docs/app-builder-staged-flow-design.md` §13 — structural eval oracles +- `plugins/model-apps/docs/app-builder-design.md` §13 — structural eval oracles - `evals/model-apps/genpage/EVAL_GUIDE.md` — the parallel eval suite for `/genpage` diff --git a/evals/model-apps/genpage/fixtures/1-account-card-gallery/genpage-plan.md b/evals/model-apps/genpage/fixtures/1-account-card-gallery/genpage-plan.md index 87085f2d7..c155d99a9 100644 --- a/evals/model-apps/genpage/fixtures/1-account-card-gallery/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/1-account-card-gallery/genpage-plan.md @@ -16,8 +16,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Sales Hub (12345678-1234-1234-1234-123456789abc) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/1-account-card-gallery/workflow-log.md b/evals/model-apps/genpage/fixtures/1-account-card-gallery/workflow-log.md index 936054fe5..281b6ce61 100644 --- a/evals/model-apps/genpage/fixtures/1-account-card-gallery/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/1-account-card-gallery/workflow-log.md @@ -12,8 +12,8 @@ - (Run separately, not chained with &&) ### Auth check -- `pac auth list` → active profile aurora365-user1@auroratstgeo.onmicrosoft.com -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile contoso-user001@contosotest1.onmicrosoft.com +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Entity discovery - `pac model list-tables --search 'account'` — Account entity detected as existing (exact logical-name match: `account`) diff --git a/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/genpage-plan.md b/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/genpage-plan.md index 68112dd53..734011137 100644 --- a/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/genpage-plan.md @@ -16,8 +16,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Recruitment Hub (44444444-3333-4444-5555-666666666666) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/workflow-log.md b/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/workflow-log.md index ca38f8bf1..ab372cf79 100644 --- a/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/11-recruitment-multi-page/workflow-log.md @@ -11,8 +11,8 @@ - `pac help` → PAC CLI Version 2.11.0 (> 2.10.0 verified) ### Auth check -- `pac auth list` → active profile aurora365-user1@auroratstgeo.onmicrosoft.com -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile contoso-user001@contosotest1.onmicrosoft.com +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Entity discovery - `pac model list-tables --search 'contact,appointment'` — both entities detected as existing diff --git a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/README.md b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/README.md index e0e850530..8c23bfb05 100644 --- a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/README.md +++ b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/README.md @@ -6,7 +6,7 @@ First real capture taken after the v2.2 planner-spec tightening landed. ## Source Captured from a real `/genpage` session against -`https://aurorabapenv610b3.crmtest.dynamics.com`. Driven by +`https://contosobapenv0001.crmtest.dynamics.com`. Driven by `claude-sonnet-4-6` under the v2.1 plugin + v2.2 Phase 0.5 manifest generator + v2.2 spec-tightening. Working dir was `D:/temp/recruitment-app/`. diff --git a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/genpage-plan.md b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/genpage-plan.md index bf7ab8b9d..c02a41df6 100644 --- a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/genpage-plan.md @@ -10,7 +10,7 @@ D:/temp/recruitment-app D:/Projects/power-platform-skills/plugins/model-apps ## Environment -- URL: https://aurorabapenv610b3.crmtest.dynamics.com +- URL: https://contosobapenv0001.crmtest.dynamics.com - App: create new: Recruitment App - Languages: English (1033) only - Solution: Crdec34 diff --git a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/workflow-log.md b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/workflow-log.md index 689d761b7..9317713fb 100644 --- a/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/11-recruitment-pages-real/workflow-log.md @@ -12,14 +12,14 @@ PAC CLI Version 2.11.0+g06bb2eb (.NET 10.0.8) `pac auth list` Index Active Kind Name User Cloud Type Environment Environment Url -[1] UNIVERSAL akmaloo@microsoft.com Public OperatingSystem -[2] UNIVERSAL aurorauser14@capintegration01.onmicrosoft.com Test User TMSBAPEnv5ee52 https://tmsbapenv5ee52.crmtest.dynamics.com/ -[3] * UNIVERSAL Aurora365-User1@auroratstgeo.onmicrosoft.com Test User AuroraBAPEnv610b3 https://aurorabapenv610b3.crmtest.dynamics.com/ +[1] UNIVERSAL developer@contoso.com Public OperatingSystem +[2] UNIVERSAL fabrikamusr1@fabrikamtenant01.onmicrosoft.com Test User FabrikamEnv001 https://fabrikamenv001.crmtest.dynamics.com/ +[3] * UNIVERSAL Contoso-User001@contosotest1.onmicrosoft.com Test User ContosoBAPEnv0001 https://contosobapenv0001.crmtest.dynamics.com/ -AskUserQuestion: Which environment should be used for the recruitment app? → [3] Aurora365-User1@auroratstgeo.onmicrosoft.com — AuroraBAPEnv610b3 (currently active) +AskUserQuestion: Which environment should be used for the recruitment app? → [3] Contoso-User001@contosotest1.onmicrosoft.com — ContosoBAPEnv0001 (currently active) `pac auth select --index 3` -New default profile: * UNIVERSAL : Aurora365-User1@auroratstgeo.onmicrosoft.com Test https://aurorabapenv610b3.crmtest.dynamics.com/ +New default profile: * UNIVERSAL : Contoso-User001@contosotest1.onmicrosoft.com Test https://contosobapenv0001.crmtest.dynamics.com/ `pac model list-languages` Found 1 enabled language(s): @@ -75,7 +75,7 @@ User response: approved - Files written: package.json, genpage.d.ts (features: charts) ### Decisions -- Environment: AuroraBAPEnv610b3 (https://aurorabapenv610b3.crmtest.dynamics.com/) +- Environment: ContosoBAPEnv0001 (https://contosobapenv0001.crmtest.dynamics.com/) - PAC CLI: 2.11.0 (> 2.10.0 requirement met) - Node: v20.18.2 - Languages: English only — no localization code needed diff --git a/evals/model-apps/genpage/fixtures/13-contact-localization/genpage-plan.md b/evals/model-apps/genpage/fixtures/13-contact-localization/genpage-plan.md index 4345dba40..cfbf47a1d 100644 --- a/evals/model-apps/genpage/fixtures/13-contact-localization/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/13-contact-localization/genpage-plan.md @@ -14,8 +14,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Sales Hub (55555555-4444-5555-6666-777777777777) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/13-contact-localization/workflow-log.md b/evals/model-apps/genpage/fixtures/13-contact-localization/workflow-log.md index 4b9ff4a56..b800dbe5c 100644 --- a/evals/model-apps/genpage/fixtures/13-contact-localization/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/13-contact-localization/workflow-log.md @@ -11,8 +11,8 @@ - `pac help` → PAC CLI Version 2.11.0 (> 2.10.0 verified) ### Auth check -- `pac auth list` → active profile aurora365-user1@auroratstgeo.onmicrosoft.com -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile contoso-user001@contosotest1.onmicrosoft.com +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Entity discovery - `pac model list-tables --search 'contact'` — contact entity exists diff --git a/evals/model-apps/genpage/fixtures/15-support-tickets-real/README.md b/evals/model-apps/genpage/fixtures/15-support-tickets-real/README.md index 7e0150ce2..3e8a218ca 100644 --- a/evals/model-apps/genpage/fixtures/15-support-tickets-real/README.md +++ b/evals/model-apps/genpage/fixtures/15-support-tickets-real/README.md @@ -6,7 +6,7 @@ fully green.** Replaces the original memory-game capture under eval id 15. ## Source Captured from a real `/genpage` session against -`https://aurorabapenv610b3.crmtest.dynamics.com`. Driven by +`https://contosobapenv0001.crmtest.dynamics.com`. Driven by `claude-sonnet-4-6` under the v2.1 plugin + v2.2 manifest generator + v2.2 planner-spec tightening. Working dir was `D:/temp/support-tickets/`. diff --git a/evals/model-apps/genpage/fixtures/15-support-tickets-real/entity-creation-log.md b/evals/model-apps/genpage/fixtures/15-support-tickets-real/entity-creation-log.md index 7b09ad66b..3b1cba511 100644 --- a/evals/model-apps/genpage/fixtures/15-support-tickets-real/entity-creation-log.md +++ b/evals/model-apps/genpage/fixtures/15-support-tickets-real/entity-creation-log.md @@ -1,7 +1,7 @@ # Entity Creation Log ## Environment -- URL: https://aurorabapenv610b3.crmtest.dynamics.com/ +- URL: https://contosobapenv0001.crmtest.dynamics.com/ - Solution: Crdec34 - Publisher Prefix: cr diff --git a/evals/model-apps/genpage/fixtures/15-support-tickets-real/genpage-plan.md b/evals/model-apps/genpage/fixtures/15-support-tickets-real/genpage-plan.md index 62ae97805..b3134e5f3 100644 --- a/evals/model-apps/genpage/fixtures/15-support-tickets-real/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/15-support-tickets-real/genpage-plan.md @@ -10,7 +10,7 @@ D:/temp/support-tickets D:/Projects/power-platform-skills/plugins/model-apps ## Environment -- URL: https://aurorabapenv610b3.crmtest.dynamics.com +- URL: https://contosobapenv0001.crmtest.dynamics.com - App: Genpage Publish Test (3fc905b9-7854-f111-a821-70a8a59ce7bc) - Languages: English (1033) only - Solution: Crdec34 diff --git a/evals/model-apps/genpage/fixtures/15-support-tickets-real/workflow-log.md b/evals/model-apps/genpage/fixtures/15-support-tickets-real/workflow-log.md index ded4dc013..bd27311e8 100644 --- a/evals/model-apps/genpage/fixtures/15-support-tickets-real/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/15-support-tickets-real/workflow-log.md @@ -13,12 +13,12 @@ PAC CLI Version 2.11.0+g06bb2eb (.NET 10.0.8) — > 2.10.0 confirmed. ### Step 2: Authenticate and Select Environment `pac auth list` -Three profiles found. Profile [3] (Aurora365-User1@auroratstgeo.onmicrosoft.com, AuroraBAPEnv610b3) is active (*). +Three profiles found. Profile [3] (Contoso-User001@contosotest1.onmicrosoft.com, ContosoBAPEnv0001) is active (*). `pac auth select --index 3` -Confirmed profile 3 active: Aurora365-User1@auroratstgeo.onmicrosoft.com — https://aurorabapenv610b3.crmtest.dynamics.com/ +Confirmed profile 3 active: Contoso-User001@contosotest1.onmicrosoft.com — https://contosobapenv0001.crmtest.dynamics.com/ -Working with environment: AuroraBAPEnv610b3 (https://aurorabapenv610b3.crmtest.dynamics.com/) +Working with environment: ContosoBAPEnv0001 (https://contosobapenv0001.crmtest.dynamics.com/) ### Step 3: Gather Requirements @@ -66,7 +66,7 @@ App selected: Genpage Publish Test (3fc905b9-7854-f111-a821-70a8a59ce7bc) `pac solution list` Found 2 non-managed solutions: - Crdec34 (Common Data Services Default Solution) — publisher: Cr54cc7 -- Default (Default Solution) — publisher: DefaultPublisheraurorabapenv610b3 +- Default (Default Solution) — publisher: DefaultPublishercontosobapenv0001 Entity creation required (cr_ticket) → solution question presented. @@ -103,7 +103,7 @@ genpage-plan.md written to D:/temp/support-tickets/genpage-plan.md | Decision | Value | |----------|-------| -| Environment | AuroraBAPEnv610b3 — https://aurorabapenv610b3.crmtest.dynamics.com/ | +| Environment | ContosoBAPEnv0001 — https://contosobapenv0001.crmtest.dynamics.com/ | | Languages | English (1033) only | | cr_ticket exists | No — needs creation | | App | Genpage Publish Test (3fc905b9-7854-f111-a821-70a8a59ce7bc) | @@ -122,17 +122,17 @@ genpage-plan.md written to D:/temp/support-tickets/genpage-plan.md ## Phase 2 — Entity creation pre-flight + builder - `node D:/Projects/power-platform-skills/plugins/model-apps/scripts/check-auth.js` (first run) - - Result: ok=false, blocker=whoami_403, az=akmaloo@microsoft.com, pac=Aurora365-User1 -- User instructed to run `az login --username Aurora365-User1@auroratstgeo.onmicrosoft.com` + - Result: ok=false, blocker=whoami_403, az=developer@contoso.com, pac=Contoso-User001 +- User instructed to run `az login --username Contoso-User001@contosotest1.onmicrosoft.com` - `node D:/Projects/power-platform-skills/plugins/model-apps/scripts/check-auth.js` (second run) - - Result: ok=true, identitiesMatch=true, envUrl=https://aurorabapenv610b3.crmtest.dynamics.com + - Result: ok=true, identitiesMatch=true, envUrl=https://contosobapenv0001.crmtest.dynamics.com - Dispatched genpage-entity-builder agent - Reads Solution=Crdec34, Publisher Prefix=cr from plan ## Environment - Wrote D:/temp/support-tickets/provision-input.json with: - Table: cr_ticket (columns: cr_name/primary, cr_priority/picklist, cr_status/picklist, cr_duedate/datetime) - Sample data: 10 cr_ticket records - Solution: Crdec34 (embedded in JSON) - - `node D:/Projects/power-platform-skills/plugins/model-apps/scripts/provision-entities.js --env https://aurorabapenv610b3.crmtest.dynamics.com --input @D:/temp/support-tickets/provision-input.json --apply --sample-data` + - `node D:/Projects/power-platform-skills/plugins/model-apps/scripts/provision-entities.js --env https://contosobapenv0001.crmtest.dynamics.com --input @D:/temp/support-tickets/provision-input.json --apply --sample-data` - SDK handles metadata propagation - Transactional log: entity-creation-log.md diff --git a/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/genpage-plan.md b/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/genpage-plan.md index 033372259..f08a589f8 100644 --- a/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/genpage-plan.md @@ -10,7 +10,7 @@ D:/temp/sales-dashboard D:/Projects/power-platform-skills/plugins/model-apps ## Environment -- URL: https://aurorabapenv610b3.crmtest.dynamics.com/ +- URL: https://contosobapenv0001.crmtest.dynamics.com/ - App: DSTest-A-WithDataSources (35913103-4e59-f111-a821-000d3a37616d) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/workflow-log.md b/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/workflow-log.md index 27a5b9ec2..28581198f 100644 --- a/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/2-mock-dashboard-real/workflow-log.md @@ -13,9 +13,9 @@ Microsoft PowerPlatform CLI Version: 2.11.0+g06bb2eb (.NET 10.0.8) — PAC CLI V ### Authentication `pac auth list` -Three profiles found. Profile [3] (`Aurora365-User1@auroratstgeo.onmicrosoft.com`, env `AuroraBAPEnv610b3`, URL `https://aurorabapenv610b3.crmtest.dynamics.com/`) is active (marked `*`). No re-selection needed. +Three profiles found. Profile [3] (`Contoso-User001@contosotest1.onmicrosoft.com`, env `ContosoBAPEnv0001`, URL `https://contosobapenv0001.crmtest.dynamics.com/`) is active (marked `*`). No re-selection needed. -Working with environment: AuroraBAPEnv610b3 (https://aurorabapenv610b3.crmtest.dynamics.com/). +Working with environment: ContosoBAPEnv0001 (https://contosobapenv0001.crmtest.dynamics.com/). ### Requirements (intent pre-resolved by orchestrator) diff --git a/evals/model-apps/genpage/fixtures/2-mock-dashboard/genpage-plan.md b/evals/model-apps/genpage/fixtures/2-mock-dashboard/genpage-plan.md index 6f9e6b9c0..dfd2ceab7 100644 --- a/evals/model-apps/genpage/fixtures/2-mock-dashboard/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/2-mock-dashboard/genpage-plan.md @@ -16,8 +16,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Sales Hub (12345678-1234-1234-1234-123456789abc) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/2-mock-dashboard/workflow-log.md b/evals/model-apps/genpage/fixtures/2-mock-dashboard/workflow-log.md index 94000c9e3..b6b0aae04 100644 --- a/evals/model-apps/genpage/fixtures/2-mock-dashboard/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/2-mock-dashboard/workflow-log.md @@ -12,8 +12,8 @@ - (Commands run separately, not chained with &&) ### Auth check -- `pac auth list` → active profile `aurora365-user1@auroratstgeo.onmicrosoft.com` -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile `contoso-user001@contosotest1.onmicrosoft.com` +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Discovery questions (AskUserQuestion) - Question 1 (new or edit): user answered "Create new page(s)" diff --git a/evals/model-apps/genpage/fixtures/4-case-wizard/genpage-plan.md b/evals/model-apps/genpage/fixtures/4-case-wizard/genpage-plan.md index 32aabb219..69c8365d8 100644 --- a/evals/model-apps/genpage/fixtures/4-case-wizard/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/4-case-wizard/genpage-plan.md @@ -16,8 +16,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Service Hub (22222222-1111-2222-3333-444444444444) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/4-case-wizard/workflow-log.md b/evals/model-apps/genpage/fixtures/4-case-wizard/workflow-log.md index b34c0ec1e..31a73855a 100644 --- a/evals/model-apps/genpage/fixtures/4-case-wizard/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/4-case-wizard/workflow-log.md @@ -11,8 +11,8 @@ - `pac help` → PAC CLI Version 2.11.0 (> 2.10.0 verified) ### Auth check -- `pac auth list` → active profile aurora365-user1@auroratstgeo.onmicrosoft.com -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile contoso-user001@contosotest1.onmicrosoft.com +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Entity discovery - `pac model list-tables --search 'incident,contact'` — both entities detected as existing (exact logical-name matches: `incident`, `contact`) diff --git a/evals/model-apps/genpage/fixtures/5-kanban-task-board/genpage-plan.md b/evals/model-apps/genpage/fixtures/5-kanban-task-board/genpage-plan.md index c6586b0db..31451bdcc 100644 --- a/evals/model-apps/genpage/fixtures/5-kanban-task-board/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/5-kanban-task-board/genpage-plan.md @@ -10,7 +10,7 @@ D:/temp/task-board D:/Projects/power-platform-skills/plugins/model-apps ## Environment -- URL: https://aurorabapenv610b3.crmtest.dynamics.com +- URL: https://contosobapenv0001.crmtest.dynamics.com - App: Genpage Publish Test (3fc905b9-7854-f111-a821-70a8a59ce7bc) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/5-kanban-task-board/workflow-log.md b/evals/model-apps/genpage/fixtures/5-kanban-task-board/workflow-log.md index 8d8abc56d..a5aab55de 100644 --- a/evals/model-apps/genpage/fixtures/5-kanban-task-board/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/5-kanban-task-board/workflow-log.md @@ -12,11 +12,11 @@ PAC CLI Version 2.11.0+g06bb2eb (.NET 10.0.8) — version > 2.10.0 confirmed. `pac auth list` Index Active Kind Name User Cloud Type Environment Environment Url -[1] UNIVERSAL akmaloo@microsoft.com Public OperatingSystem -[2] UNIVERSAL aurorauser14@capintegration01.onmicrosoft.com Test User TMSBAPEnv5ee52 https://tmsbapenv5ee52.crmtest.dynamics.com/ -[3] * UNIVERSAL Aurora365-User1@auroratstgeo.onmicrosoft.com Test User AuroraBAPEnv610b3 https://aurorabapenv610b3.crmtest.dynamics.com/ +[1] UNIVERSAL developer@contoso.com Public OperatingSystem +[2] UNIVERSAL fabrikamusr1@fabrikamtenant01.onmicrosoft.com Test User FabrikamEnv001 https://fabrikamenv001.crmtest.dynamics.com/ +[3] * UNIVERSAL Contoso-User001@contosotest1.onmicrosoft.com Test User ContosoBAPEnv0001 https://contosobapenv0001.crmtest.dynamics.com/ -AskUserQuestion: "Multiple auth profiles found. Which environment would you like to use?" → [3] Aurora365-User1@auroratstgeo.onmicrosoft.com — AuroraBAPEnv610b3 (currently active) +AskUserQuestion: "Multiple auth profiles found. Which environment would you like to use?" → [3] Contoso-User001@contosotest1.onmicrosoft.com — ContosoBAPEnv0001 (currently active) `pac model list-languages` Found 1 enabled language(s): English (United States) LCID 1033 — no localization needed. @@ -40,7 +40,7 @@ EnterPlanMode called User response: approved ### Decisions -- Working environment: AuroraBAPEnv610b3 (https://aurorabapenv610b3.crmtest.dynamics.com/) +- Working environment: ContosoBAPEnv0001 (https://contosobapenv0001.crmtest.dynamics.com/) - Languages: English (1033) only - Entity status: task — exists (Standard table), no creation required - App: Genpage Publish Test (3fc905b9-7854-f111-a821-70a8a59ce7bc) @@ -58,7 +58,7 @@ User response: approved - Result: wrote `package.json` and `genpage.d.ts` (no extra features — no charts/datepicker/timepicker needed) ## Phase 2 — Entities -Skipped — `task` Standard entity confirmed to exist in AuroraBAPEnv610b3. +Skipped — `task` Standard entity confirmed to exist in ContosoBAPEnv0001. ## Phase 3 — App selection Existing app: Genpage Publish Test (`3fc905b9-7854-f111-a821-70a8a59ce7bc`). `pac model create` not invoked. diff --git a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/entity-creation-log.md b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/entity-creation-log.md index 408ac6ac4..985220e54 100644 --- a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/entity-creation-log.md +++ b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/entity-creation-log.md @@ -1,7 +1,7 @@ # Entity Creation Log ## Environment -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- URL: https://contosobapenv0002.crm10.dynamics.com/ - Solution: Default - Publisher Prefix: cr diff --git a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/genpage-plan.md b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/genpage-plan.md index 850821e88..1cda500ff 100644 --- a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/genpage-plan.md +++ b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/genpage-plan.md @@ -16,8 +16,8 @@ D:\Projects\power-platform-skills\plugins\model-apps ## Environment -- Active Profile: aurora365-user1@auroratstgeo.onmicrosoft.com -- URL: https://aurorabapenv4ab3f.crm10.dynamics.com/ +- Active Profile: contoso-user001@contosotest1.onmicrosoft.com +- URL: https://contosobapenv0002.crm10.dynamics.com/ - App: Recruitment Hub (33333333-2222-3333-4444-555555555555) - Languages: English (1033) only - Solution: Default diff --git a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/workflow-log.md b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/workflow-log.md index 2ed67e2a0..2d682b7b1 100644 --- a/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/workflow-log.md +++ b/evals/model-apps/genpage/fixtures/7-job-candidates-new-entities/workflow-log.md @@ -11,8 +11,8 @@ - `pac help` → PAC CLI Version 2.11.0 (> 2.10.0 verified) ### Auth check -- `pac auth list` → active profile aurora365-user1@auroratstgeo.onmicrosoft.com -- Active environment: https://aurorabapenv4ab3f.crm10.dynamics.com/ (reported to user) +- `pac auth list` → active profile contoso-user001@contosotest1.onmicrosoft.com +- Active environment: https://contosobapenv0002.crm10.dynamics.com/ (reported to user) ### Entity discovery - `pac model list-tables --search 'cr_candidate'` — exact logical-name match: NOT FOUND @@ -28,7 +28,7 @@ ### Solution selection - Build needs metadata work (new entities) → solution selection question asked - AskUserQuestion solution selection question presented -- `node dataverse-request.js https://aurorabapenv4ab3f.crm10.dynamics.com /solutions` to enumerate existing solutions +- `node dataverse-request.js https://contosobapenv0002.crm10.dynamics.com /solutions` to enumerate existing solutions - dominant prefix detection: env shows mixed prefixes — no dominant prefix flagged - User selected: "Use Default Solution" - Recorded in plan: `Solution: Default`, `Publisher Prefix: cr` @@ -51,7 +51,7 @@ - Relationship: 1:N from cr_jobrequisition to cr_candidate (lookup: cr_jobrequisition) - Sample data: 2 cr_jobrequisition records, 8 cr_candidate records (parent binding via $parent/match) - Solution: Default (embedded in JSON) -- node ${CLAUDE_PLUGIN_ROOT}/scripts/provision-entities.js --env https://aurorabapenv4ab3f.crm10.dynamics.com --input @job-candidates/provision-input.json --apply --sample-data +- node ${CLAUDE_PLUGIN_ROOT}/scripts/provision-entities.js --env https://contosobapenv0002.crm10.dynamics.com --input @job-candidates/provision-input.json --apply --sample-data - SDK handles table ordering and metadata propagation - Transaction log written to job-candidates/entity-creation-log.md diff --git a/plugins/model-apps/AGENTS.md b/plugins/model-apps/AGENTS.md index e178d5c46..d52c43aee 100644 --- a/plugins/model-apps/AGENTS.md +++ b/plugins/model-apps/AGENTS.md @@ -45,12 +45,18 @@ able to tell what moved from the docs alone): | `AGENTS.md` (this file — `CLAUDE.md` symlinks to it) | Per-component behavioral specs, the canonical file tree, conventions, build/test | You add/rename a script, change a component's behavior, or change how to build/test | | [`docs/architecture.md`](docs/architecture.md) | Wiring / flow **diagrams** for both skills (`/genpage` + `/app-builder`) | You change the orchestration, phase pipeline, or how the pieces connect | | [`docs/app-builder-roadmap.md`](docs/app-builder-roadmap.md) | `/app-builder` **roadmap / TODO** (Complete + Pending by phase) | You ship or reprioritize an app-builder capability | +| [`docs/app-builder-design.md`](docs/app-builder-design.md) | `/app-builder` **design record** — Part I staged-flow architecture (**cited from code by section number — never renumber**), Part II the `--changed-only` contract | You change the staged flow or the partial-apply contract | | [`CHANGELOG.md`](CHANGELOG.md) | Keep-a-Changelog — concise bullets (detail lives in PRs/docs) | Any user-visible change | | [`references/app-spec-schema.md`](references/app-spec-schema.md) | The App Spec contract | You change the App Spec shape or validation | Don't duplicate content across these — **cross-link instead** (a second copy only drifts, as the file tree and teardown order both did before). +**This repo is public.** Before adding to any of these docs, re-read the repo-root `AGENTS.md` → +*"This Repo Is PUBLIC"*. The docs here have already had to be scrubbed once for internal repo paths, +a real Dataverse environment name, review provenance, and indexes into documents an outside reader +cannot open. Record that kind of context in the PR conversation instead. + ## app-builder — intent → model-driven app A second skill (`/app-builder`) builds a whole **model-driven app** (tables, columns, @@ -117,7 +123,8 @@ the pipeline and delegates each script's **behavioral spec** to the entries belo `@odata.nextLink` pagination (`queryRecords({ paginate:true })`), so it verifies EVERY app in the environment rather than one 5000-row page; a pagination fault (the SDK's repeated-nextLink guard) still fails **closed** rather than scanning a partial list. Classic `dashboards[]` are opt-in. - **All Dataverse access is via the SDK**, so metadata is persisted under + **All of the build's Dataverse access is via the SDK** (see "Dataverse Access From Scripts" for the + sanctioned exceptions elsewhere), so metadata is persisted under `/.maker-workspace/` for reuse/edits. The 14 phases (`solution·data-model·sample-data·web-resources·views·charts·forms·commands·dashboards·app-shell·pages·ai-features·security·publish`) are unchanged; independent ops run with bounded parallelism. @@ -132,7 +139,7 @@ the pipeline and delegates each script's **behavioral spec** to the entries belo the pages phase (uploads just the changed page, skips the full build) — gated on an identity-bound snapshot (`.maker-workspace/apply-snapshot.json`); any non-page edit (or an edit to a pre-existing app) falls back to a full build. Teardown tombstones+deletes the snapshot. See - [`docs/changed-only-design.md`](docs/changed-only-design.md) for the v1 scope + contract. + [`docs/app-builder-design.md`](docs/app-builder-design.md) for the v1 scope + contract. **App TABLE components are pinned by OData REFERENCE** (ADO 6612527). The SDK sends `{ '@odata.id': '()' }` per sitemap table, NOT an `@odata.type` instance: `Microsoft.Dynamics.CRM.entity` names a real Dataverse table (metadata-as-data), so the old instance @@ -362,8 +369,7 @@ feature-flags.json ← Default-OFF feature flags (currently connecto docs/ architecture.md ← Wiring/flow diagrams for BOTH skills (/genpage + /app-builder) app-builder-roadmap.md ← /app-builder roadmap / TODO (Complete + Pending by phase) - app-builder-staged-flow-design.md ← Historical staged-flow design notes - changed-only-design.md ← /app-builder --changed-only design contract + app-builder-design.md ← /app-builder design record (Part I staged flow · Part II --changed-only) agents/ ← Agent definitions (invoked by skills via Task tool) genpage-planner.md ← Requirements, discovery, plan doc, user approval (create flow) genpage-connector-builder.md ← Orchestrator-invoked connector gate/discovery; writes connector bindings @@ -423,6 +429,9 @@ scripts/ sdk-teardown.js ← app-builder teardown engine (planTeardown is pure) sdk-http-client.js ← az-token HttpClient for the vendored SDK spec-lint.js / app-spec.js ← App Spec guardrail lint + validation + spec-shape.js ← shared structural normalization for both authoring gates + surface-resolver.js ← pure: resolve personas[].jobs[].surfaces[] to the spec artifacts that satisfy them + role-privileges.js ← pure: declared persona privileges + subset comparison against a deployed role odata.js ← OData literal escaping helpers genpage-cli.js ← pac model genpage upload/list/download wrapper hydrate-spec.js ← reconstruct an App Spec from a deployed app (edit flow) @@ -537,6 +546,45 @@ values in `feature-flags.json` at the plugin root. values in the committed file (so a typo can't silently do nothing, or — after a flip to `true` — accidentally enable the wrong thing). +**Each gated feature has a SINGLE OWNER agent, and every entry point must go through it or the +shared helper.** Both currently-gated features gate at the same five places, so the rule is stated +once here and only the per-feature specifics are tabled below: + +1. **Discovery** — the owner agent runs the probe first; planners/edit-planners delegate to it and + never gate inline. +2. **Scripts** — each entry-point script calls the shared `exitIfDisabled()` helper (DRY — + never an inlined gate) and fails closed with exit 3 when OFF. +3. **Deploy** — the SKILL phase **re-probes** the flag and treats an absent/malformed bindings + section as *no bindings*, so a plan authored while the flag was ON cannot deploy after it goes OFF. +4. **ALM** — solution packaging honours the flag (or documents why it needs no gate). +5. **Codegen** — `genpage-page-builder` emits feature code **only** when the plan carries an actual + binding table, never on an absent/sentinel section. + +| | `connectors` | `custom-api` | +|---|---|---| +| **Owner agent** | `genpage-connector-builder` | `genpage-customapi-builder` | +| **Plan section** | `## Connector Bindings` | `## Custom API Bindings` | +| **Gated scripts** | `list-connections.js`, `create-connection-reference.js` | `list-custom-apis.js` | +| **Deploy phase** | SKILL Phase 4.5 | SKILL Phase 4.6 | +| **ALM** | the `--connection-refs` branch of `add-page-to-solution.js` | none needed — `config.json`'s `actionBindings` travels inside the page's `uxagentprojectfile` rows automatically (the Custom APIs themselves are a separate deployment prerequisite, bound by name) | +| **Emits** | connector code | `executeAction` / `executeFunction` / `listBoundActions` | + +One connectors-only nuance: at Phase 4.5 the `/genpage` orchestrator re-probes and passes the +verbatim result as `Connectors: enabled|disabled` in every page-builder dispatch — **that dispatch +value wins over the plan's `## Connector Bindings` section.** + +Both flags currently ship **OFF**, each waiting on cross-repo dependencies: + +- **`connectors`** — the pac CLI connector verbs (PowerPlatform-Scale-AdminTools), the GenUX + authoring control (power-platform-ux), and the maker/admin ECS setting must all release first. +- **`custom-api`** — the AIBuilder CoderAgent action prompt, the shared `pai-gen-ux-action-runtime` + plus the UCI and Controls host runtimes, a pac CLI `model genpage upload --actions` verb to + persist `actionBindings` into `config.json`, and the `GenUxPluginActionAllowList` ECS setting. + Note the maker-facing name is "Custom API" while the shipped wire contract stays + `actionBindings` / `executeAction` (see `references/custom-api.md`). + +## TSX source lexer — known limits + **`scripts/lib/source-literals.js` — known limits.** It is a hand-rolled TSX lexer, not a parser: the plugin ships dependency-free, so there is no TypeScript to call. It tracks code / line comment / block comment / string / template / regex / JSX tag / JSX text, and @@ -565,51 +613,6 @@ Residual limits, accepted deliberately: Neither shape occurs in the corpus, and both fail *loudly* (exit 3, retryable) rather than silently. If you hit one, widen the tests first. -**Connectors gate — the single owner is `genpage-connector-builder`.** Every connector -entry point must go through it or the helper; the checklist of places that gate: - -1. Discovery + codegen — the top-level `/genpage` orchestrator dispatches - `genpage-connector-builder`, which runs the initial probe. At Phase 4.5 the - orchestrator re-probes with - `node "${PLUGIN_ROOT}/scripts/lib/feature-flags.js" connectors` and passes the - verbatim result as `Connectors: enabled|disabled` in every page-builder dispatch; - this dispatch value wins over the plan's `## Connector Bindings` section. -2. Scripts — `list-connections.js` / `create-connection-reference.js` (`exitIfConnectorsDisabled`). -3. Deploy — SKILL Phase 4.5 **re-probes** the flag and treats absent/malformed - `## Connector Bindings` as no bindings (a plan authored while ON must not deploy - connectors after OFF). -4. ALM — the `--connection-refs` branch of `add-page-to-solution.js`. -5. Codegen — `genpage-page-builder` emits connector code only when the plan has an - actual binding table (never on an absent/sentinel section). - -The **`connectors`** flag currently ships OFF: GenPage connector support needs the -pac CLI connector verbs (PowerPlatform-Scale-AdminTools), the GenUX authoring control -(power-platform-ux), and the maker/admin ECS setting to all be released first. - -**Custom API gate — the single owner is `genpage-customapi-builder`.** Every Custom API -entry point must go through it or the helper; the checklist of places that gate: - -1. Discovery — `genpage-customapi-builder` runs the probe first (planner + edit-planner - delegate to it; they do not gate inline). -2. Scripts — `list-custom-apis.js` (`exitIfCustomApiDisabled`). -3. Deploy — SKILL Phase 4.6 **re-probes** the flag and treats absent/malformed - `## Custom API Bindings` as no bindings (a plan authored while ON must not deploy - Custom API bindings after OFF). -4. ALM — none needed: `config.json`'s `actionBindings` travels inside the page's - `uxagentprojectfile` rows automatically (the Custom APIs themselves are a separate - deployment prerequisite, bound by name). -5. Codegen — `genpage-page-builder` emits `executeAction` / `executeFunction` / - `listBoundActions` code only when the plan has an actual binding table (never on an - absent/sentinel section). - -The **`custom-api`** flag currently ships OFF: GenPage Custom API invocation needs the -AIBuilder CoderAgent action prompt, the shared `pai-gen-ux-action-runtime` plus the UCI and -Controls host runtimes, a pac CLI `model genpage upload --actions` verb -(PowerPlatform-Scale-AdminTools) to persist `actionBindings` into `config.json`, and the -`GenUxPluginActionAllowList` ECS setting to all be released first. Note the maker-facing name -is "Custom API" while the shipped wire contract stays `actionBindings` / `executeAction` -(see `references/custom-api.md`). - ## Hooks & Validators Hooks are registered centrally in `hooks/hooks.json` (auto-loaded by the plugin @@ -680,7 +683,42 @@ repo-root `shared/telemetry/`; `scripts/lib/telemetry/lib` is a **physical copy* - **Accessibility** — WCAG AA, ARIA labels, keyboard navigation, semantic HTML - **Complete code** — no placeholders, TODOs, or ellipses in final output -## Skill Authoring Guidelines +## Dataverse Access From Scripts + +**Default: go through the vendored SDK.** Anything the SDK models — tables, columns, relationships, +views, charts, forms, commands, dashboards, app modules, sitemaps, solutions, roles, settings — is +read and written through `createMakerSdk`. That is not style: the SDK persists metadata under +`/.maker-workspace/` for reuse and edits, resolves artifact identity the same way the +build does, and owns retry/pagination behaviour. A read that bypasses it can disagree with the write +about which artifact it is talking about. + +Two escape hatches exist, and both are deliberate. The maker SDK models the *maker* surface; parts of +Dataverse simply are not in it. + +| Hatch | Use for | Examples in tree | +|---|---|---| +| `dataverseRequest()` in `lib/dataverse-auth.js` (and the `dataverse-request.js` CLI) | Dataverse surfaces the SDK does not model at all | `WhoAmI` (`check-auth.js`), `customapis` (`list-custom-apis.js`), `connectionreferences` (`create-connection-reference.js`), solution-component adds (`add-page-to-solution.js`) | +| The raw `httpClient` from `createAzHttpClient` | A surface the SDK *does* touch but whose response it **projects away** | `entityPrivileges` in `verify-model-app.js` — `fetchEntityMetadata` returns `{logicalName, displayName, entitySetName, attributes, relationships}` and drops `Privileges` entirely. The projection's omission is permanent (it is disk-cached and best-effort, the wrong contract for a security read) and pinned by an SDK guardrail test. **Transitional:** the SDK is gaining a dedicated `getEntityPrivileges()`; switch to it and drop this raw read once the vendored bundle carries it | + +**Prefer `dataverseRequest()` over the raw client.** It already handles the API path, auth, headers +and timeouts. Reach for `httpClient` only when you must share the exact client instance the SDK is +using, as the verify reader does. + +When you do go direct, all four of these apply: + +1. **Comment WHY the SDK cannot serve it** — name the SDK method you would otherwise call and what it + drops or lacks. "Deliberately not `sdk.fetchEntityMetadata`" is the difference between a + documented exception and something a later reader "simplifies" back into a silent bug. +2. **Absolute URL including `/api/data/v9.2`** when using the raw `httpClient`. It is the transport + the SDK drives, so it takes full request URLs and validates them with `new URL(url)` for its + same-origin guard — a relative path throws there rather than resolving against the org. +3. **GUIDs unquoted.** Record ids and `_x_value` lookups are `Edm.Guid`; `id eq ''` fails with + *"A binary operator with incompatible types was detected"*. See `references/troubleshooting.md`. +4. **Test the reader itself, not only an injected stub.** The `entityPrivileges` URL bug shipped + because every test injected a fake reader into `verifySpec`, so the real one was never executed — + and `verify-spec` catches per-entity read failures, so it would have failed silently on every live + run rather than crashing. Drive at least one test through the real client's request seam. + - Keep SKILL.md under 500 lines - Use short, descriptive `name` field (e.g., `genpage`) diff --git a/plugins/model-apps/CHANGELOG.md b/plugins/model-apps/CHANGELOG.md index 8d6bc2e75..4092f6cb2 100644 --- a/plugins/model-apps/CHANGELOG.md +++ b/plugins/model-apps/CHANGELOG.md @@ -4,10 +4,29 @@ All notable changes to the **model-apps** plugin. ## [Unreleased] — 2.4.4 -Adds plugin update notices, fixes four crash paths, and corrects a smoke-eval -assertion that could never pass live. +Adds plugin update notices, proves what persona roles actually grant, makes +jobs-to-be-done surfaces checkable, fixes four crash paths, and corrects a +smoke-eval assertion that could never pass live. ### Added +- **`verify` now proves what a persona security role GRANTS, not just that it + exists.** The `role` check only asserted a role row carrying the SDK ownership + marker, so a role built with the wrong access — or one whose privilege write + failed after the row landed — verified clean. The new `role-privileges` check + resolves every 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. A **subset** check by design: extra + privileges are never a finding, because `appAccess` injects `appmodule` read, + unioned jobs escalate a shared entity+access to the max declared scope, and + distinct entities can share one Dataverse privilege. Fails **closed** on an + unreadable role or table. Reader-gated, so existence-only callers are unchanged. +- **`personas[].jobs[].surfaces[]` is checked instead of documentary.** Each entry + is now resolved against the spec's own views, forms, pages, dashboards, tables + and sitemap titles. `spec-lint` **warns** when a surface matches nothing — a + warning, not an error, because a surface may legitimately name an out-of-the-box + artifact this spec never authors. `verify` adds a `job-surface` rollup that + reports a deployed failure as the job it broke ("persona P can no longer do job + J"), rather than only "view X is missing". - **Automatic plugin update notice.** Every user-invocable skill now runs the non-blocking `scripts/check-version.js` preflight, which compares the installed Model Apps version with `origin/main` and shows update commands for the active diff --git a/plugins/model-apps/agents/genpage-connector-builder.md b/plugins/model-apps/agents/genpage-connector-builder.md index 75892e0f5..1215bb059 100644 --- a/plugins/model-apps/agents/genpage-connector-builder.md +++ b/plugins/model-apps/agents/genpage-connector-builder.md @@ -47,7 +47,7 @@ You will be invoked via `Task` with a prompt that includes: - **Mode:** `create` or `edit`. - **Working directory** — where to write outputs and read/write logs. - **Plugin root** (`${PLUGIN_ROOT}`) — where the JS scripts live. -- **Environment URL** — e.g. `https://aurorabapenv4ab3f.crmtest.dynamics.com`. +- **Environment URL** — e.g. `https://contoso.crm.dynamics.com`. - **Intent** — the source(s) the request implies (e.g. "SharePoint documents", "current weather", "Office 365 users") and, for `edit`, whether the maker wants to **add**, **replace**, or **remove** connector data. diff --git a/plugins/model-apps/agents/genpage-customapi-builder.md b/plugins/model-apps/agents/genpage-customapi-builder.md index b3b76c59d..a3fb219ed 100644 --- a/plugins/model-apps/agents/genpage-customapi-builder.md +++ b/plugins/model-apps/agents/genpage-customapi-builder.md @@ -35,7 +35,7 @@ You will be invoked via `Task` with a prompt that includes: - **Mode:** `create` or `edit`. - **Working directory** — where to write outputs and read/write logs. - **Plugin root** (`${PLUGIN_ROOT}`) — where the JS scripts live. -- **Environment URL** — e.g. `https://aurorabapenv4ab3f.crmtest.dynamics.com`. +- **Environment URL** — e.g. `https://contoso.crm.dynamics.com`. - **Page tables** — the Dataverse table logical name(s) the page is bound to (from the plan's `## Existing Entities` / `pageInput`), or "none" for a mock/global-only page. - **Intent** — the server-side operation(s) the request implies (e.g. "approve the order", diff --git a/plugins/model-apps/agents/genpage-entity-builder.md b/plugins/model-apps/agents/genpage-entity-builder.md index 7e3c721e0..211b9d875 100644 --- a/plugins/model-apps/agents/genpage-entity-builder.md +++ b/plugins/model-apps/agents/genpage-entity-builder.md @@ -27,7 +27,7 @@ You will be invoked by the `/genpage` skill with a prompt that includes: - Path to `genpage-plan.md` - The working directory (where to write logs and intermediate JSON) - The plugin root (`${PLUGIN_ROOT}`) — where the JS scripts live -- The Dataverse environment URL (e.g. `https://aurorabapenv4ab3f.crmtest.dynamics.com`) +- The Dataverse environment URL (e.g. `https://contoso.crm.dynamics.com`) The **Solution unique name** and **Publisher Prefix** are read directly from the plan document's `## Environment` section (the planner always writes them — the @@ -153,7 +153,7 @@ full entity creation specification. All examples below assume you have extracted from the plan's `## Environment`: ```bash -ENV_URL="" # e.g. https://aurorabapenv4ab3f.crmtest.dynamics.com +ENV_URL="" # e.g. https://contoso.crm.dynamics.com SOLUTION="" # e.g. Default PREFIX="" # e.g. new ``` diff --git a/plugins/model-apps/docs/app-builder-staged-flow-design.md b/plugins/model-apps/docs/app-builder-design.md similarity index 70% rename from plugins/model-apps/docs/app-builder-staged-flow-design.md rename to plugins/model-apps/docs/app-builder-design.md index 9a6f60489..8b657167d 100644 --- a/plugins/model-apps/docs/app-builder-staged-flow-design.md +++ b/plugins/model-apps/docs/app-builder-design.md @@ -1,17 +1,36 @@ -# Design — `/app-builder` staged flow architecture +# `/app-builder` — design record + +The two design records the `/app-builder` engine cites, in one file. They were previously +`app-builder-staged-flow-design.md` and `changed-only-design.md`; the content is unchanged. + +- **Part I — staged-flow architecture.** The shipped architecture and the reasoning behind it. + Its **numbered sections are cited from code** (§6, §7, §9, §11, §13, §14), so the numbers are + load-bearing: keep them stable, and never renumber. (The numbering skips §16 and §17 — those + were removed without shifting anything else, for exactly this reason.) +- **Part II — `--changed-only` safe partial apply.** The v1 scope and safety contract for the + partial-apply mode built on top of Part I. Read it before touching that mode. + +Current status of shipped/pending capabilities lives in +[`app-builder-roadmap.md`](./app-builder-roadmap.md) — this file is the "why", the roadmap is the +"what/where". Wiring and flow diagrams live in [`architecture.md`](./architecture.md). + +--- + +# Part I — staged-flow architecture > **Status: IMPLEMENTED / LANDED.** This is the design record for the `/app-builder` staged-flow > architecture that shipped — the engine now runs it (staged flow, design-only author, generate-pages -> stage, structural evals, fail-closed safety). It began as a re-architecture proposal (revised after -> Sol architectural review R1; the four Critical findings were verified against the engine code and -> resolved — see **§17 Sol R1 → resolutions**) and was subsequently implemented, reviewed, and +> stage, structural evals, fail-closed safety). It began as a re-architecture proposal and was +> subsequently implemented, reviewed, and > live-verified. It is retained as the **design rationale** the engine's code cites by section (§6, §7, -> §9, §11, §13, §14); the section numbers are load-bearing — keep them stable. +> §9, §11, §13, §14); the section numbers are load-bearing — keep them stable, and do not renumber +> when trimming (§16 and §17 were removed without shifting anything else, which is why the numbering +> skips them). > > **Current status of shipped/pending capabilities lives in [`app-builder-roadmap.md`](./app-builder-roadmap.md)** > (the canonical roadmap), not here — this doc is the "why", the roadmap is the "what/where". The > `--changed-only` partial-apply mode built ON TOP of this staged flow has its own contract in -> [`changed-only-design.md`](./changed-only-design.md). +> **Part II** below. > > The App Spec **core** contract and the 13 engine phase names are unchanged; the staged flow is an > additive naming/orchestration layer over them. @@ -74,7 +93,7 @@ parity oracle. But the **flow around it** had five concrete problems: --- -## 5. Execution model (corrected — resolves Critical C1) +## 5. Execution model **Why the first draft was wrong.** `runSdkBuild` allocates a **fresh** `result.created` map on every invocation (`sdk-build.js:540`). Downstream phases resolve dependencies from that @@ -141,7 +160,7 @@ invocations. `data` executes in run 1; `generate-pages` in the main loop; `ui`/` **Renamed** from the first draft: the code-gen stage is **`generate-pages`**, never `pages` — the engine phase `pages` (upload, a member of stage `app`) keeps its name, and using one word for -both was ambiguous (reviewer Minor 2). **One namespace** is used for stage names across +both was ambiguous. **One namespace** is used for stage names across narration, journal, and CLI. **`--stage ` sugar** on `build-model-app.js` maps a stage to its phase range internally @@ -153,12 +172,12 @@ stage/phase selectors are **rejected** (today `resolvePhases` silently ignores u --- -## 7. Author redesign & App Spec schema (resolves C2, I1) +## 7. Author redesign & App Spec schema Author stops emitting `.tsx`. It produces a **design-only** spec and freezes it via consent gates (two shape-level design checkpoints + the single plan-mode build approval, R2 unchanged). -### 7.1 Validation profiles (resolves C2) +### 7.1 Validation profiles `validateAppSpec` is currently unconditional and requires every page's `codeFile` (`build-model-app.js:98`, `app-spec.js:431-438`), so an intent-only spec fails before the first @@ -182,7 +201,7 @@ unconditional; `build-model-app.js:97-101`, `teardown-model-app.js:62-65`, `veri | teardown / partial cleanup | `structural` (no `codeFile` required) | | default programmatic build | `deploy` | -### 7.2 Page shape — discriminated source (resolves I1, C4 key ambiguity) +### 7.2 Page shape — discriminated source One `pages[]` collection; implementation state is **explicit**, not a nullable field: @@ -243,7 +262,7 @@ persistence is therefore **foundational**, and the mechanism is **decided** (not --- -## 8. Generate-pages stage & agent contracts (resolves I2) +## 8. Generate-pages stage & agent contracts After run 1 (tables exist), the **main loop**: @@ -271,7 +290,7 @@ After run 1 (tables exist), the **main loop**: --- -## 9. Cross-page navigation & the `PAGEREF_` resolver (resolves C4) +## 9. Cross-page navigation & the `PAGEREF_` resolver Reuse `/genpage`'s navigation contract (`references/rules.md` 299–356): navigate via `Xrm.Navigation.navigateTo({ pageType:'generative', pageId, data })`; custom ids go in `data:` @@ -308,7 +327,7 @@ duplicates and orphans. The `app` stage instead: --- -## 10. Look-and-feel: a **page** design contract (resolves I5) +## 10. Look-and-feel: a **page** design contract `appDef` has no theme field or mutation (`sdk-build.js:516-518`), so a `spec.design` cannot drive the model-driven shell today. Scope honestly: @@ -323,7 +342,7 @@ the model-driven shell today. Scope honestly: --- -## 11. Safety & autopilot — fail-closed destructive ops (resolves C3) +## 11. Safety & autopilot — fail-closed destructive ops Consent is mode-aware; **autopilot mode is also eval mode** (both non-interactive). @@ -373,7 +392,7 @@ working dir when autopilot. The user approves the whole app shape at once. --- -## 13. Verify & evals (resolves I3, I4) +## 13. Verify & evals ### 13.1 Verify extended to pages (I4) @@ -406,7 +425,7 @@ navigation resolution end-to-end. --- -## 14. Engine mechanics & module extraction (resolves I6) +## 14. Engine mechanics & module extraction `sdk-build.js` is ~1099 lines; the precedent for pure modules is `artifact-intent.js`. Extract: @@ -466,55 +485,6 @@ artifact-specific SDK mutator — SDK **T2/T7 pass**). + pipeline id in the CLI/journal; adapted genpage agent contracts + `Task` in the SKILL `allowed-tools`; round-trip metadata in `download-model-app.js`/`hydrate-spec.js`. -## 16. Doc-sync checklist (expanded — Minor 1) - -`SKILL.md`, `references/authoring-flow.md`, `references/app-spec-schema.md`, -`docs/architecture.md` (stage diagram), `AGENTS.md`, `CHANGELOG.md`, the README section, **plus the -contract/round-trip/agent files most affected**: `app-spec.js`, `hydrate-spec.js`, -`download-model-app.js`, `verify-spec.js`, the genpage agent/plan contracts, and the eval docs. - ---- - -## 17. Sol R1 → resolutions - -| # | Finding | Resolution | -|---|---|---| -| **C1** | Phase ranges not composable across invocations (`result.created` per run) | §5 — data pre-build → code-gen → **one full idempotent build**; run 2's discovery rehydrates `result.created` (the proven `create==edit` path). Stages are vocabulary, not write invocations. | -| **C2** | Intent spec fails validation; `--verify`/`--only publish`/data examples are no-ops | §7.1 validation **profiles** (`design`/`plan`/`deploy`); standalone verify via `verify-model-app.js`; §6 correct stage→command mapping with required flags. | -| **C3** | Destructive ops not fail-closed | §11 read-only **`op-diff.js`** planner; hard `--allow-destructive` gate incl. teardown; create fails on collision; approval bound to env/app/spec-hash/op-hash; env var suppresses questions only. | -| **C4** | `PAGEREF_` source mutation not portable/idempotent | §9 stable `key`; **symbolic source preserved**; **staging-file** deployment derivative; fail-closed enumeration + create-absent-first + single resolved upload; pure `pageref-resolver.js`; sitemap-finalize commit; forward-only recovery. | -| **I1** | Bare optional `codeFile` weakens contract | §7.2 discriminated `source:{intent|tsx}` + `schemaVersion` + stable keys + path confinement + duplicate checks; `sitemapSlot` dropped (appShell authoritative). | -| **I2** | Reused agents don't fit the boundary | §8 generate a page-build contract for `page-builder` (owns new+edit); `edit-planner` becomes headless (no consent tools); add `Task` to SKILL `allowed-tools`. | -| **I3** | `.tsx` snapshots are the wrong oracle | §13.2 structural/compile/nav/verified-column/token oracles; new `schema-facts` extractor for data-model. | -| **I4** | Verify doesn't cover page invariants | §13.1 page existence/`GenPageId`/no-unresolved-`PAGEREF_` checks; verify stage mandatory + fail-closed. | -| **I5** | Shell-theme alignment unsupported | §10 scope `spec.design` to a **page** contract + Fluent-token mapping + validation; shell theme deferred. | -| **I6** | Stage state / perf / module ownership | §14 pipeline id + stage-attempt metadata; extract `stages.js`/`pageref-resolver.js`/`op-diff.js`/`schema-facts.js`; reject unknown selectors; **DAG removed** (single full run 2). | -| **Minor 1/2** | Doc-sync gaps; 6-vs-7 stage ambiguity | §16 expanded doc-sync; §6 rename to `generate-pages`, one namespace, author counted as the design stage. | - -### 17.1 Sol R2 (confirming pass) → resolutions - -R2 **verified C1 sound** (run 2 rehydrates every `appDef` dependency, incl. existing pages) and -passed I2/I3/I5 + both Minors. The remaining PARTIALs + new findings are closed as: - -- **New Critical — safe page deployment/recovery** → §9 protocol (staging files; fail-closed - enumeration; create-absent-first + immediate manifest persist; single resolved upload; - sitemap-finalize commit). -- **Execution-semantics contradiction (DAG vs single run 2)** → §14: DAG removed; `--stage` - apply-safe only for `data`; run 1 omits `--sample-data`. -- **Safety-plan realism** → §11: v1 scope limited to detectable ops; reuse reconciliation / - `diffArtifact`; scoped (not blanket) collision; TOCTOU re-check. -- **Stable-key persistence (foundational)** → §7.3: durable `_pagemanifest` web resource + - download reverse-normalization + legacy migration. -- **Full-build retry idempotency** → §14: commands/dashboards become discover-reconcile. -- **Validation/verification boundaries** → §7.1 per-caller profile matrix; §13.1 verify each nav - edge → actual target `GenPageId`. - -**R3 (convergence pass)** confirmed the execution model sound and closed the last two items: the -**page manifest** is now a versioned durable contract with full semantic metadata + an explicit -create/update/add-to-solution/teardown lifecycle + `pageId`s validated against enumeration (§7.3), -and **command/dashboard reconcile** is **additive-only** in v1 with removals deferred to `op-diff` -(§14). - ## 18. SDK-alignment (target after rework) - **T1 canonical desired state** — restored: `sitemapSlot` dropped (single source = `appShell`); @@ -545,3 +515,136 @@ and **command/dashboard reconcile** is **additive-only** in v1 with removals def same deployed state as a single full build. - **Data-driven offline harness** (`evals/model-apps/app-builder/`) with the §13.2 oracles. - **Live tier:** keep `smoke-eval.js`; add a multi-page navigation case. + + +--- + +# Part II — `--changed-only` safe partial apply + +Status: **implemented; PROD-readiness reviewed; live-verified.** The design went through several +adversarial review rounds before implementation (v1 *unsafe* → v7 *safe-to-implement*), and the +implementation was reviewed again with all Critical/High findings fixed and covered by tests. +This is the canonical spec for the multi-deliverable build. + +## v1 scope + contract (READ THIS) +v1 wires exactly ONE convergent shape end-to-end: **page-content re-upload** (a `.tsx` byte edit to an +existing page). Everything else — view-append, sitemap, form, or any data-model/AI/chart/command/ +dashboard/web-resource change — routes to a **full build** (always safe; a full build converges the first +four and additive-SKIPS the rest, which then incur sticky debt). The user-facing contract: +- **`--changed-only` bootstraps its baseline from a FRESH build.** The eligible baseline is written only + when the app did **not** exist when the run started (a first `--apply --changed-only`). Running the + first build with a plain `--apply` (no snapshot) and *then* `--changed-only` yields an **ineligible** + baseline (the additive engine may not have converged a pre-existing app) → every run full-builds until a + clean teardown+rebuild. **Use `--changed-only` from the first build to opt in.** +- **Use `--changed-only` consistently.** A plain `--apply` in between invalidates the snapshot (→ next + `--changed-only` re-baselines via a full build). Mixing the two de-optimizes but never corrupts. +- **Concurrent Maker edit of the SAME changed page is unsupported** (PAC page upload has no CAS) — the + fast path re-uploads only the *changed* pages, so an unchanged page is never clobbered; a page you edit + in both the spec and Maker is overwritten by the spec (the intended deploy). Documented as a follow-up. + +## Deferred to follow-ups (v1 does NOT implement; tracked in the roadmap) +Pre-mutation live page-content drift verifier; `expectedSitemap` population + pre/post sitemap projection +equality (sitemap fast shape unwired in v1); a `clearDebtMatching` production caller (v1 clears debt only +via teardown+rebuild); unifying `contentPath` confinement between hashing and the build; view/form/sitemap +fast submodes. None are reachable on the v1 pages-only fast path. + +## Why this is hard +`/app-builder`'s build engine is **ADDITIVE, not convergent**: existing chart/command/dashboard defs and +web-resource CONTENT are *skipped* on rebuild (`sdk-build.js:685-695,985-996,1114-1153`), and the I1 gate +(`build-model-app.js:149-158`) forbids partial `--apply` because `result.created` (the artifact-id map) +is per-invocation and phases read each other's ids from it. So a naive "run only changed phases" would +silently **bless edits that never deployed**. The whole design is therefore **fail-closed**. + +## Core invariant (replaces I1) +A partial `--apply` is allowed ONLY via `--changed-only`, and ONLY when: an identity-validated snapshot +exists with `eligible:true` and **empty debt**; every consumed id re-resolves live by semantic identity; +the diff is a subset of the four PROVEN-CONVERGENT shapes; no sitemap drift; and — after the apply — an +EXACT projection verifier proves each touched artifact's deployed state equals the spec. **Every** +state-changing op (full apply, fast apply, teardown) first atomically writes `eligible:false` +(write-before-mutate) and aborts if that guard write fails. `eligible:true` is reached again ONLY via a +verified fast run or a proven-fresh/rebaseline full build with empty debt. Anything not provably safe ⇒ +full build / HALT. + +## The four convergent shapes (the ONLY fast-path edits in v1) +1. **page** — `.tsx` byte change to an EXISTING page, unchanged key→pageId map (re-upload only, sitemap + write skipped). +2. **form (explicit layout)** — field-set add(first-section)/remove, placement-exact (all other tab/ + section/cell/isRequired/events/quickView/autoSubgrids/subgrid identical). +3. **view** — pure APPEND of columns (no width/order/filter/sort/removal); default-view enrichment + suppressed. +4. **app-shell sitemap** — structural sitemap change only (app.description/icon excluded). +Additions of NEW top-level artifacts, removals, and charts/commands/dashboards/web-resource-content/AI ⇒ +full build, and they incur sticky **debt**. + +## Snapshot envelope (`/apply-snapshot.json`, schema 3, atomic) +`{ schema, orgId (live WhoAmI), envUrl, appUniqueName, appId, solutionUniqueName, generation (uuid), +eligible (bool), debt:[{artifactType,identity,reason}], priorSpec (canonical, no rows), expectedSitemap +(normalized projection), artifacts:{ pages:{key:{pageId,sourceSha,deployedSha}}, forms:{entity|formType| +name:{formId,projSha}}, views:{entity|name:{viewId,projSha}}, app:{sitemapSha}, webResources, entities, +charts, commands, dashboards } }`. Written temp→fsync→rename, under one build-wide lease + generation CAS, +ONLY after effective success (apply+verify). + +## Eligibility state machine (durable, fail-closed) +- **INVALIDATE (→false) before any write** of every full/unsupported/fast apply and teardown; abort if + the invalidation write fails. +- **debt** accrues on any unsupported change/removal; `eligible:true` requires empty debt; debt clears + ONLY by proven-fresh recreation (artifact absent before build) or an exact verifier — never by a plain + full rebuild that re-skips a stale artifact. +- **teardown TOMBSTONE**: teardown writes `eligible:false` + `teardown-in-progress` debt BEFORE deleting + anything, and deletes the envelope ONLY after teardown success + verified live absence; a partial/ + crashed teardown leaves the tombstone (so a surviving artifact can't be rebaselined). + +## Projection/verifier framework (`scripts/lib/projection.js` — deliverable #1, DONE) +Pure, id-free, normalized projections that serve as the EXACT post-apply verifiers (static classification +alone is not trusted): `formProjection` (placement-exact), `sitemapProjection` (GUID/env-url normalized), +and page source-vs-deployed dual hashing. Adversarial tests: form-placement collision, page dual-hash, +sitemap normalization (`scripts/tests/projection.test.js`). + +## Phase submodes + phase-local publish +On `--changed-only`, phases run in restricted submodes — forms `fieldReconcileOnly` (no promote/ +deactivate/events/quickviews), app-shell `prerequisiteResolveOnly`/`finalizeSitemapOnly` (no icon/ +sitemap-solution re-ensure), pages `selectedKeysOnly` (whole-app read-only safety checks retained, only +changed keys uploaded), views append-only. NO whole-spec publish (`PublishAllXml`) — phase-local publish +of the touched artifacts only. A pre-mutation live-projection equality check refuses to overwrite a +Maker-drifted artifact (PAC page upload has no CAS — the residual concurrent-edit race is documented as +unsupported). + +**CONFIRMED landmine (validated against `sdk-build.js`) — the pages-only fast path MUST skip the sitemap +finalize.** The pages phase's finalize (`sdk-build.js:1428-1437`) rebuilds the WHOLE sitemap via +`appDef(spec, result.created)` and writes `/siteMap` + `components`. In a pages-only run `result.created` +holds only `app`+`pages`, so `appDef` (a) THROWS on any dashboard subarea (`:610-611`, +`result.dashboards` empty) and (b) rebuilds `components` from empty `result.forms/views/charts` (`:635`), +stripping the app's form/view/chart component registrations. A pure page-content re-upload leaves the +key→pageId map unchanged, so the sitemap needs no rewrite — the fast path seeds `result.created.app` from +live discovery, uploads only the changed page(s) to their existing pageIds, SKIPS the finalize, and +publishes just the page. Two sdk-build seams (flag-gated, full-build path byte-identical): seed +`result.created.app` from `opts.resolvedAppId`, and skip `:1428-1437` under the changed-only page submode. + +## Sequenced deliverables +1. ✅ Projection/verifier framework + 3 adversarial tests. 2. ✅ Content hashing + fix the shipped +`phase-diff` foundation (`.tsx`/`contentPath` edits visible to the diff, fail-closed). 3. ✅ Envelope — +pure state machine (`apply-snapshot.js`) + I/O store (`apply-snapshot-store.js`) + `indexArtifacts` +(`apply-snapshot-index.js`), wired into the live build (persist/invalidate) + teardown (tombstone/delete). +4. ✅ Phase submodes (v1) — the pages-only sdk-build seams: seed `result.created.app` from +`opts.changedOnly.resolvedAppId`, skip the sitemap finalize, and `selectedKeysOnly` (upload only changed +keys) + measured `pageDeployedShas`. (view/sitemap/form submodes deferred — see follow-ups.) +5. ✅ Classifier (`classify-changes.js`) + the `build-model-app.js --changed-only` FLOW +(`changed-only-flow.js`): `--changed-only` flag, live identity (WhoAmI orgId + app discovery), +snapshot read→eligibility gate→classify→pages-only fast apply via the seams (or full fallback), +invalidate-before-write + fresh-create-only baseline + verify-gated re-bless + generation fencing, I1-gate +exception. 6. ✅ Full offline tests (919 green). 7. ✅ Implementation review — adversarial, max effort; +all Critical/High findings fixed with tests (see the fix commit + this doc's v1 +contract/deferred sections). 8. ✅ **Live regression PASSED** (a scratch environment, 2026-07-27): (1) fresh +`--apply --changed-only` → full build, verify 9/9, **eligible** snapshot (orgId/appId bound, debt 0); +(2) `.tsx` byte edit → `--apply --changed-only` → **FAST pages-only** (2 steps not 12, same pageId = +UPDATE, new measured deployedSha, verify 9/9, snapshot **re-blessed eligible**); (3) chart edit → +`--apply --changed-only` → **full-build fallback**, verify 9/9, snapshot **INELIGIBLE** with sticky +`chart-edit-not-convergent` + `uncertified-baseline` debt; (4) `teardown --apply` → app cascade removed, +snapshot **tombstoned then deleted** (0 leftovers). 9. ✅ Docs — design + roadmap + CHANGELOG + AGENTS.md. + +## Build-time notes (from the final Sol pass) +- Live-absence verification covers the UNION of prior-snapshot identities, current spec, generated + artifacts, and all debt entries — not merely the current spec. +- Debt-clearing verifiers must match the specific debt reason (placement verify can't clear + event/quick-view debt). +- Keep crash/failure-injection tests + a live PAC page-upload publication/concurrency test. diff --git a/plugins/model-apps/docs/app-builder-roadmap.md b/plugins/model-apps/docs/app-builder-roadmap.md index a8ca53b9e..eb5c4592c 100644 --- a/plugins/model-apps/docs/app-builder-roadmap.md +++ b/plugins/model-apps/docs/app-builder-roadmap.md @@ -71,41 +71,36 @@ are in [`architecture.md`](architecture.md). --- -## 🎯 MVP gaps vs the MDA Agentic Authoring spec +## 🎯 MVP gaps -Evaluated 2026-08-12 against `powerplatform-modelpages-ade/docs/topics/ModelBuilder/MDA-Agentic-Authoring-spec.md` -(§3 MVP definition + §5 stack rank). **9 of 10 core P0 _primitives_ are ✅ complete** — roles + JTBDs as first-class planning outputs, -data model + sample data, auto-number + dedup, Active/Inactive + authored views, main forms, charts, -gen-page landings, custom SVG sitemap icons, in-app agents (`ai.appFeatures` = formFill/nlSearch/nlChart), -Insight Card summaries (`ai.summaries`), and +Assessed 2026-08-12. **9 of 10 core P0 _primitives_ are ✅ complete** — roles + JTBDs as first-class planning outputs, +data model + sample data, auto-number + dedup, Active/Inactive + authored views, main forms, charts, +gen-page landings, custom SVG sitemap icons, in-app agents (`ai.appFeatures` = formFill/nlSearch/nlChart), +Insight Card summaries (`ai.summaries`), and the dedup/verify quality gates. The open MVP items: -- 🔲 **Wave 2 (header/navigation refresh) enabled by default** (spec rank 3, P0) — set the app-module header/nav - refresh flag on new apps, and turn it on when editing an app that isn't on it yet (spec rank 20). Not set today. -- ✅ **Roles + JTBD as first-class planning outputs** (spec rank 1, P0) — the Level-(c) design flow now models +- 🔲 **Wave 2 (header/navigation refresh) enabled by default** (P0) — set the app-module header/nav + refresh flag on new apps, and turn it on when editing an app that isn't on it yet. Not set today. +- ✅ **Roles + JTBD as first-class planning outputs** (P0) — the Level-(c) design flow now models **personas** and their **jobs-to-be-done**: the author declares the entity access each job needs, the builder unions it into one security role per persona, and the plan/preview surfaces the proposed roles for review. Built on the SDK security surface (`createPersonaRole`). JTBD-driven view/summary/sitemap coherence (below) still builds further on this. - 🔲 **Default-on + coherence wiring** — the AI agents, Insight Card summaries, and (once added) Wave 2 exist as primitives, but the SKILL flow must **enable them by default** and author **JTBD-quality** content (entity-specific - summary prompts per the §4 Group G guidelines, the _right_ view columns). A ✅ primitive is necessary but not - sufficient for the spec's coherence bar. + summary prompts, the _right_ view columns). A ✅ primitive is necessary but not sufficient for a coherent app. Important, P1 (not MVP-gating; tracked here for visibility): -- ✅ **Security roles per persona** (spec rank 14, Group N P1) — `personas[]` authors one security role per persona, +- ✅ **Security roles per persona** — `personas[]` authors one security role per persona, sized from its jobs-to-be-done, and grants the app to each role so it **opens for non-admins**, not just sysadmins. Idempotent + converging (replace-privileges), fail-closed on a foreign same-name role, torn down with the app. - **Column-level (field) security** and **access teams / hierarchy security** (Group N P3) remain a tracked SDK + **Column-level (field) security** and **access teams / hierarchy security** remain a tracked SDK follow-up. -- 🔲 **Rich AI descriptions on every artifact** (rank 22) · **quick-find / relevance-search config** (rank 23) · - **custom app theme + logo** (rank 18 — the `design` block styles gen pages, not the app theme). +- 🔲 **Rich AI descriptions on every artifact** · **quick-find / relevance-search config** · + **custom app theme + logo** (the `design` block styles gen pages, not the app theme). -P0.5 stretch (not built): **modern grid visualizations by default** (rank 11, contingent on the SDK grid-customizer) -and **MCP server + Catalyst by default** (rank 12). The spec's §5 stack rank carries a status column annotated from -this roadmap; it is refreshed on `master` in `powerplatform-modelpages-ade` (last synced 2026-08-12, plugin v2.4.3). -**Keep the two in step** — when a row here flips, update that column in the same pass, or the spec silently goes -stale (ranks 1 / 14 / 29 sat wrong for two weeks after the persona work shipped). +P0.5 stretch (not built): **modern grid visualizations by default** (contingent on the SDK grid-customizer) +and **MCP server + Catalyst by default**. --- @@ -138,7 +133,7 @@ stale (ranks 1 / 14 / 29 sat wrong for two weeks after the persona work shipped) (case-sensitive; only bare names are lower-cased); `collectSitemap` skips platform-path icons. So download→edit→rebuild on a real app with custom/OOB nav icons no longer fails validation or silently loses the icon. Adversarially reviewed (Sol + Opus — Opus caught a residual area-icon case-corruption, - fixed); **live-verified on aurorabapenv03468** (entity-subarea `VectorIcon` lands in the deployed sitemap + fixed); **live-verified** (entity-subarea `VectorIcon` lands in the deployed sitemap XML; the reporter's exact OOB `…/CDSEntity` icon rebuilds `ok:true`; round-trip clean). **Follow-up (now resolved — see below):** deploying a `/WebResources//icons/x.svg` reference to a DIFFERENT env originally rendered a broken icon (the WR was assumed pre-existing). @@ -205,7 +200,7 @@ stale (ranks 1 / 14 / 29 sat wrong for two weeks after the persona work shipped) `resolveExistingFormId` for the same identity. `FORM_TYPE_CODE`/`FORM_GUID_RE` live in the shared `app-spec.js`. `artifactIdentityQuery('form')` is also type-scoped. Reviewed adversarially across three rounds (Sol — the name-only-identity flaw was pervasive; all High/Medium findings fixed). Live-repro - `zava_javavendor` (three "Information" forms) unblocked; **live-verified end-to-end on aurora** (build a + `zava_javavendor` (three "Information" forms) unblocked; **live-verified end-to-end** (build a table → it gets 3 same-named "Information" forms → edit the Main without a halt → the edit lands on the Main ONLY, Quick View/Card untouched → teardown resolves the Main type-scoped). Locked by resolver / preflight / verify / teardown / validation unit tests. **Follow-up:** a quick-view control already @@ -217,7 +212,7 @@ stale (ranks 1 / 14 / 29 sat wrong for two weeks after the persona work shipped) - 🔲 **Conditional `updateTable` (If-Match / skip-if-unchanged)** — the SDK's `updateTable` does an unconditional GET→PUT of the whole `EntityDefinitions` row (strips `@odata.etag`, no `If-Match`), so a concurrent Maker edit to another property of the SAME table in the GET→PUT window is last-writer-wins. This is pre-existing (icons/audit already use `updateTable`); the quick-create flag adds one more caller. Follow-ups: preserve the ETag + conditional PUT (retry/surface 412), and skip the PUT when the requested flag is already set (avoids a redundant write on every opted-in rebuild). Same class as the build's `requireSuccessfulPush` 412 posture for artifacts. ### Phase: Forms, views & data-load polish (from the 2026-07-15 V1↔V2 comparison review) -Source: `IMPROVEMENTS-07-15-app-builder.md` (Project Management V1/V2 diff + a sample data-load). **Status 2026-07-27: all 8 addressed — the "Allow quick create" table flag (#8) now ships; auto-GENERATING the Quick Create form's field layout remains a follow-up (see below).** Severity from the source doc. +From a review comparing two generated Project Management apps plus a sample data-load. **Status 2026-07-27: all 8 addressed — the "Allow quick create" table flag (#8) now ships; auto-GENERATING the Quick Create form's field layout remains a follow-up (see below).** Severity as assessed in that review. - ✅ **[High] Validate lookup binds; stop silent data-load lookup failures** — DONE. `validateAppSpec` now validates `$parents` (junction) the same as `$parent` and flags a `$parent`/`$parents.match` that resolves to no parent sample row (the bind would be dropped and the lookup left unset); `buildSeedGroup` THROWS (fail loud) on an unresolvable parent instead of silently skipping. Runs inside `runner.run` (clean phase failure). `app-spec.js` sampleData validation + `entity-provision.js` buildSeedGroup. - ✅ **[Medium] Don't truncate parent lookups in default-view enrichment** — DONE. `defaultViewColumns` now reserves the parent-lookup slots up front and caps *scalar* columns at the remaining budget, then appends every lookup — so a lookup-heavy table never drops a parent link (`sdk-build.js`). Teardown's `{ includeLookups:false }` reset path unchanged. - ✅ **[Medium] Normalize N:N relationship schema-name ordering** — DONE. `manyToManySchemaName` sorts the two entity logical names alphabetically before composing, so the N:N name is stable regardless of `entity1`/`entity2` declaration order (`app-spec.js`). 1:N keeps its semantic `referenced_referencing` order; explicit `schemaName` still wins. @@ -234,12 +229,27 @@ Source: `IMPROVEMENTS-07-15-app-builder.md` (Project Management V1/V2 diff + a s - 🔲 **Spec templates** — domain starters (support desk, CRM, asset tracking) as one-shot scaffolds. ### Phase: Quality & docs +- ✅ **Security-role and jobs-to-be-done verification (2026-08-14)** — both **metadata-only**: + - **`verify` now proves what a persona role GRANTS, not just that it exists.** The `role` check + only asserted a row carrying the SDK marker, so a role built 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. Deliberately a **subset** check (`lib/role-privileges.js` explains why equality + would false-fail on `appAccess` injection, max-scope union, and shared privileges), and + **fail-closed** on an unreadable role or table. + - **`surfaces[]` is no longer documentary.** `lib/surface-resolver.js` resolves every + `personas[].jobs[].surfaces[]` entry against the spec's own views/forms/pages/dashboards/tables + /sitemap titles; `spec-lint` **warns** when one matches nothing (a warning, not an error — a + surface may legitimately name an OOB artifact), and `verify` adds a `job-surface` rollup that + reports a *deployed* failure as the job it broke ("persona P can no longer do job J") rather + than only "view X is missing". - ✅ **Sample-run UX fixes (2026-07-27, from a live Property-Listings build).** - ✅ **#1 live build status.** A long build now writes `/.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. - ✅ **#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. - ✅ **#3 (foundation) track the diff.** After a successful full apply the CLI persists `last-applied.json`; a later dry-run prints `▸ Changed since last apply: ` (pure `phase-diff.js`, stable-stringify deep-equal). **Still open (tracked below): actually running ONLY the changed phases on `--apply`.** - ✅ **#5 auto-number reseed lint.** `spec-lint` now WARNS that sampleData for a table whose primary is auto-numbered with no single-column alternate key will DUPLICATE on every re-run (and blocks refreshing data with `--sample-data` on an edit) — nudging a natural alternate key. -- 🔄 **#3 execution / #7 — SAFE `--changed-only` partial apply (Preview; implemented + review-hardened + LIVE-VERIFIED).** Design **Sol-approved (7 rounds)** + implementation **adversarially reviewed (GPT-5.6 Sol + Claude Opus, max effort; all Critical/High fixed with tests)** + **live regression PASSED on aurorabapenv03468** (baseline→eligible, page edit→fast pages-only, chart edit→full fallback+debt, teardown→snapshot deleted). Canonical spec [`docs/changed-only-design.md`](./changed-only-design.md) (read its **v1 scope + contract**). Because the engine is ADDITIVE-not-convergent, the partial apply is **fail-closed**: identity-bound schema-3 snapshot (live orgId/env/app match, appId required), a durable eligibility state machine + sticky debt + teardown tombstone + generation-fenced CAS. **v1 wires ONE shape end-to-end — page-content re-upload**: `--apply --changed-only` after a FRESH baseline runs ONLY the pages phase for a `.tsx` byte edit (uploads just the changed keys — never clobbers an unchanged page — skips the sitemap finalize, records the measured deployed hash, re-blesses only when page verify passes); anything else falls back to a full build. Deliverables ①–⑨ ✅ (919 offline tests green + live regression). Off by default; opt in with `--changed-only`. **Follow-ups (deferred): pre-mutation live page-content drift verifier; view/sitemap/form fast submodes + `expectedSitemap` gating; `clearDebtMatching` production caller; unify `contentPath` confinement.** +- 🔄 **#3 execution / #7 — SAFE `--changed-only` partial apply (Preview; implemented + review-hardened + LIVE-VERIFIED).** Design **design-reviewed** + implementation **adversarially reviewed** all Critical/High fixed with tests + **live regression PASSED** (baseline→eligible, page edit→fast pages-only, chart edit→full fallback+debt, teardown→snapshot deleted). Canonical spec [`docs/app-builder-design.md`](./app-builder-design.md) (read its **v1 scope + contract**). Because the engine is ADDITIVE-not-convergent, the partial apply is **fail-closed**: identity-bound schema-3 snapshot (live orgId/env/app match, appId required), a durable eligibility state machine + sticky debt + teardown tombstone + generation-fenced CAS. **v1 wires ONE shape end-to-end — page-content re-upload**: `--apply --changed-only` after a FRESH baseline runs ONLY the pages phase for a `.tsx` byte edit (uploads just the changed keys — never clobbers an unchanged page — skips the sitemap finalize, records the measured deployed hash, re-blesses only when page verify passes); anything else falls back to a full build. Deliverables ①–⑨ ✅ (919 offline tests green + live regression). Off by default; opt in with `--changed-only`. **Follow-ups (deferred): pre-mutation live page-content drift verifier; view/sitemap/form fast submodes + `expectedSitemap` gating; `clearDebtMatching` production caller; unify `contentPath` confinement.** - 🔲 **#4 page visual preview before deploy.** The generated `.tsx` is only structurally grep-checked; the user never *sees* the page (they hit the double-render live). Add a page preview/screenshot gate before the page is uploaded. - 🔲 **#6 lifecycle status modeling.** The skill authored "Listing/Offer/Showing Status" as plain Choices, so the record `statecode` (Active/Inactive) is disconnected from business status (a "Sold" row is still an "active record", and `activeOnly` views need an extra Choice filter to compensate). Steer lifecycle fields toward Dataverse `statusReasons` (statecode/statuscode), which the App Spec already supports. - 🔲 **#8 approval gate shows artifacts, not counts.** The plan gate presents a phase-count table; the user approves counts, not the reviewable data model / wireframes. Fold the wireframe (#2) + a data-model summary into the approval gate. @@ -247,7 +257,7 @@ Source: `IMPROVEMENTS-07-15-app-builder.md` (Project Management V1/V2 diff + a s - 🔲 **Workspace reuse** — load `.maker-workspace/` metadata to skip re-discovery on iterative runs. - 🔲 **Worked samples** — a Form-JS spec (web resource + onchange handler) and a dashboard spec in `samples/`. - 🔲 **Refresh `authoring-flow.md`** Level (a) column-type list (still shows the pre-Tier-1 short list). -- 🔲 **KNOWN BEHAVIOR — an authored view named identically to a stock default view unions onto it.** An authored `views[]` entry whose name equals the Dataverse stock default ("Active/Inactive <PluralName>") is matched by `findArtifact('view', {name,entity})` and **reconciled (unioned) onto that stock default** rather than created as a new view. `reconcileView` (`sdk-build.js`) updates only `/columns`, so the authored **filters and sort are silently ignored** and the stock `createdon` is kept. Live-observed on `aurorabapenv03468` (2026-07). **Mitigated:** `spec-lint` now WARNS when a view name matches the stock-default pattern (recommending a distinct name). **Still to decide (design call):** whether to also (a) fully reconcile a detected default view (columns + filters + sort) or (b) hard-reject the collision — deferred pending a decision; the warning makes it loud in the meantime. +- 🔲 **KNOWN BEHAVIOR — an authored view named identically to a stock default view unions onto it.** An authored `views[]` entry whose name equals the Dataverse stock default ("Active/Inactive <PluralName>") is matched by `findArtifact('view', {name,entity})` and **reconciled (unioned) onto that stock default** rather than created as a new view. `reconcileView` (`sdk-build.js`) updates only `/columns`, so the authored **filters and sort are silently ignored** and the stock `createdon` is kept. Live-observed on `a scratch environment` (2026-07). **Mitigated:** `spec-lint` now WARNS when a view name matches the stock-default pattern (recommending a distinct name). **Still to decide (design call):** whether to also (a) fully reconcile a detected default view (columns + filters + sort) or (b) hard-reject the collision — deferred pending a decision; the warning makes it loud in the meantime. - 🔲 **Review follow-ups deferred from the 2026-07-27 Sol PROD-readiness review (pre-prod-acceptable, documented here so they aren't lost).** - **Existing old-style sub-grid migration.** `addSubgrids` is idempotent by relationship (`hasSubgrid`), so a rebuild does NOT move a sub-grid that a *previous, pre-#5 build* placed as a half-width cell into the new full-width section, nor re-apply a changed label/view. No deployed apps predate #5 (the skill is pre-prod and the edit path is teardown+rebuild-fresh), so there is nothing to migrate today; this is consistent with the documented additive-build limitation (edits aren't re-applied in place). Revisit if in-place convergence lands. - **Self-referential / cyclic sample-data binds.** A sample row that `$parent`-binds to its own entity can't be seeded (the record-graph seeder resolves `@odata.bind` before the current group's ids exist). Not yet rejected at lint — needs SDK confirmation before a hard reject so a case the SDK *can* handle isn't blocked. Rare; document + revisit. @@ -269,7 +279,7 @@ SDK doesn't package. - 🔴 **Power Fx command on-click** — same component-library blocker (A). **JavaScript** on-click **ships**. - 🟡 **Titled command groups** — a titled group needs a parent command-bar row the adapter doesn't synthesize for from-scratch commands (re-confirmed live: Dataverse 400 "Group button must have parentappactionid"). Buttons emit as loose controls; **flyout / split-button menus do work**. **Unblock:** SDK synthesis of the parent group rows. - 🟡 **Interactive (type 10) dashboards** — different formxml machinery (streams/tiles keyed by cell id); the tile generator targets Standard (type 0). **Unblock:** an interactive-dashboard tile generator. -- ⚠ **Business-rule validation** — org-gated on the Aurora test orgs (missing the `*ProcessWithWfomJson` action) so it can't be live-verified here, and the modern path is Power-Fx-flavored. Build behind a capability flag once an org supports it. +- ⚠ **Business-rule validation** — org-gated on the available test orgs (missing the `*ProcessWithWfomJson` action) so it can't be live-verified here, and the modern path is Power-Fx-flavored. Build behind a capability flag once an org supports it. - 🟡 **Explicit app-component re-pin on an app EDIT** — a NEW chart added to an ALREADY-DEPLOYED app on an edit rebuild is not re-pinned as an explicit app component: the SDK's generic surface can't add a missing `components` object to a fetched app (`setAppDefinition` was retired). Low impact — the chart is still added to the solution and shows on its table's chart pane; rebuild the app fresh, or surface the chart via a dashboard/sitemap subarea, if it must be an explicit component. **Unblock:** an SDK component-set API for a fetched app, or fetch populating `components`. --- diff --git a/plugins/model-apps/docs/changed-only-design.md b/plugins/model-apps/docs/changed-only-design.md deleted file mode 100644 index 795a38aff..000000000 --- a/plugins/model-apps/docs/changed-only-design.md +++ /dev/null @@ -1,130 +0,0 @@ -# `--changed-only` safe partial apply — design (Sol-approved, 7 review rounds) - -Status: **implemented; PROD-readiness reviewed; live-verified** (design: adversarial GPT-5.6 Sol, 7 rounds -v1 *unsafe* → v7 *safe-to-implement*; implementation: adversarial GPT-5.6 Sol + Claude Opus, both max -effort — all Critical/High findings fixed with tests; **live regression PASSED on aurorabapenv03468**). -This is the canonical spec for the multi-deliverable build; the round-by-round design history lives in the -session workspace (`design-changed-only-v1..v7.md`). - -## v1 scope + contract (READ THIS) -v1 wires exactly ONE convergent shape end-to-end: **page-content re-upload** (a `.tsx` byte edit to an -existing page). Everything else — view-append, sitemap, form, or any data-model/AI/chart/command/ -dashboard/web-resource change — routes to a **full build** (always safe; a full build converges the first -four and additive-SKIPS the rest, which then incur sticky debt). The user-facing contract: -- **`--changed-only` bootstraps its baseline from a FRESH build.** The eligible baseline is written only - when the app did **not** exist when the run started (a first `--apply --changed-only`). Running the - first build with a plain `--apply` (no snapshot) and *then* `--changed-only` yields an **ineligible** - baseline (the additive engine may not have converged a pre-existing app) → every run full-builds until a - clean teardown+rebuild. **Use `--changed-only` from the first build to opt in.** -- **Use `--changed-only` consistently.** A plain `--apply` in between invalidates the snapshot (→ next - `--changed-only` re-baselines via a full build). Mixing the two de-optimizes but never corrupts. -- **Concurrent Maker edit of the SAME changed page is unsupported** (PAC page upload has no CAS) — the - fast path re-uploads only the *changed* pages, so an unchanged page is never clobbered; a page you edit - in both the spec and Maker is overwritten by the spec (the intended deploy). Documented as a follow-up. - -## Deferred to follow-ups (v1 does NOT implement; tracked in the roadmap) -Pre-mutation live page-content drift verifier; `expectedSitemap` population + pre/post sitemap projection -equality (sitemap fast shape unwired in v1); a `clearDebtMatching` production caller (v1 clears debt only -via teardown+rebuild); unifying `contentPath` confinement between hashing and the build; view/form/sitemap -fast submodes. None are reachable on the v1 pages-only fast path. - -## Why this is hard -`/app-builder`'s build engine is **ADDITIVE, not convergent**: existing chart/command/dashboard defs and -web-resource CONTENT are *skipped* on rebuild (`sdk-build.js:685-695,985-996,1114-1153`), and the I1 gate -(`build-model-app.js:149-158`) forbids partial `--apply` because `result.created` (the artifact-id map) -is per-invocation and phases read each other's ids from it. So a naive "run only changed phases" would -silently **bless edits that never deployed**. The whole design is therefore **fail-closed**. - -## Core invariant (replaces I1) -A partial `--apply` is allowed ONLY via `--changed-only`, and ONLY when: an identity-validated snapshot -exists with `eligible:true` and **empty debt**; every consumed id re-resolves live by semantic identity; -the diff is a subset of the four PROVEN-CONVERGENT shapes; no sitemap drift; and — after the apply — an -EXACT projection verifier proves each touched artifact's deployed state equals the spec. **Every** -state-changing op (full apply, fast apply, teardown) first atomically writes `eligible:false` -(write-before-mutate) and aborts if that guard write fails. `eligible:true` is reached again ONLY via a -verified fast run or a proven-fresh/rebaseline full build with empty debt. Anything not provably safe ⇒ -full build / HALT. - -## The four convergent shapes (the ONLY fast-path edits in v1) -1. **page** — `.tsx` byte change to an EXISTING page, unchanged key→pageId map (re-upload only, sitemap - write skipped). -2. **form (explicit layout)** — field-set add(first-section)/remove, placement-exact (all other tab/ - section/cell/isRequired/events/quickView/autoSubgrids/subgrid identical). -3. **view** — pure APPEND of columns (no width/order/filter/sort/removal); default-view enrichment - suppressed. -4. **app-shell sitemap** — structural sitemap change only (app.description/icon excluded). -Additions of NEW top-level artifacts, removals, and charts/commands/dashboards/web-resource-content/AI ⇒ -full build, and they incur sticky **debt**. - -## Snapshot envelope (`/apply-snapshot.json`, schema 3, atomic) -`{ schema, orgId (live WhoAmI), envUrl, appUniqueName, appId, solutionUniqueName, generation (uuid), -eligible (bool), debt:[{artifactType,identity,reason}], priorSpec (canonical, no rows), expectedSitemap -(normalized projection), artifacts:{ pages:{key:{pageId,sourceSha,deployedSha}}, forms:{entity|formType| -name:{formId,projSha}}, views:{entity|name:{viewId,projSha}}, app:{sitemapSha}, webResources, entities, -charts, commands, dashboards } }`. Written temp→fsync→rename, under one build-wide lease + generation CAS, -ONLY after effective success (apply+verify). - -## Eligibility state machine (durable, fail-closed) -- **INVALIDATE (→false) before any write** of every full/unsupported/fast apply and teardown; abort if - the invalidation write fails. -- **debt** accrues on any unsupported change/removal; `eligible:true` requires empty debt; debt clears - ONLY by proven-fresh recreation (artifact absent before build) or an exact verifier — never by a plain - full rebuild that re-skips a stale artifact. -- **teardown TOMBSTONE**: teardown writes `eligible:false` + `teardown-in-progress` debt BEFORE deleting - anything, and deletes the envelope ONLY after teardown success + verified live absence; a partial/ - crashed teardown leaves the tombstone (so a surviving artifact can't be rebaselined). - -## Projection/verifier framework (`scripts/lib/projection.js` — deliverable #1, DONE) -Pure, id-free, normalized projections that serve as the EXACT post-apply verifiers (static classification -alone is not trusted): `formProjection` (placement-exact), `sitemapProjection` (GUID/env-url normalized), -and page source-vs-deployed dual hashing. Adversarial tests: form-placement collision, page dual-hash, -sitemap normalization (`scripts/tests/projection.test.js`). - -## Phase submodes + phase-local publish -On `--changed-only`, phases run in restricted submodes — forms `fieldReconcileOnly` (no promote/ -deactivate/events/quickviews), app-shell `prerequisiteResolveOnly`/`finalizeSitemapOnly` (no icon/ -sitemap-solution re-ensure), pages `selectedKeysOnly` (whole-app read-only safety checks retained, only -changed keys uploaded), views append-only. NO whole-spec publish (`PublishAllXml`) — phase-local publish -of the touched artifacts only. A pre-mutation live-projection equality check refuses to overwrite a -Maker-drifted artifact (PAC page upload has no CAS — the residual concurrent-edit race is documented as -unsupported). - -**CONFIRMED landmine (validated against `sdk-build.js`) — the pages-only fast path MUST skip the sitemap -finalize.** The pages phase's finalize (`sdk-build.js:1428-1437`) rebuilds the WHOLE sitemap via -`appDef(spec, result.created)` and writes `/siteMap` + `components`. In a pages-only run `result.created` -holds only `app`+`pages`, so `appDef` (a) THROWS on any dashboard subarea (`:610-611`, -`result.dashboards` empty) and (b) rebuilds `components` from empty `result.forms/views/charts` (`:635`), -stripping the app's form/view/chart component registrations. A pure page-content re-upload leaves the -key→pageId map unchanged, so the sitemap needs no rewrite — the fast path seeds `result.created.app` from -live discovery, uploads only the changed page(s) to their existing pageIds, SKIPS the finalize, and -publishes just the page. Two sdk-build seams (flag-gated, full-build path byte-identical): seed -`result.created.app` from `opts.resolvedAppId`, and skip `:1428-1437` under the changed-only page submode. - -## Sequenced deliverables -1. ✅ Projection/verifier framework + 3 adversarial tests. 2. ✅ Content hashing + fix the shipped -`phase-diff` foundation (`.tsx`/`contentPath` edits visible to the diff, fail-closed). 3. ✅ Envelope — -pure state machine (`apply-snapshot.js`) + I/O store (`apply-snapshot-store.js`) + `indexArtifacts` -(`apply-snapshot-index.js`), wired into the live build (persist/invalidate) + teardown (tombstone/delete). -4. ✅ Phase submodes (v1) — the pages-only sdk-build seams: seed `result.created.app` from -`opts.changedOnly.resolvedAppId`, skip the sitemap finalize, and `selectedKeysOnly` (upload only changed -keys) + measured `pageDeployedShas`. (view/sitemap/form submodes deferred — see follow-ups.) -5. ✅ Classifier (`classify-changes.js`) + the `build-model-app.js --changed-only` FLOW -(`changed-only-flow.js`): `--changed-only` flag, live identity (WhoAmI orgId + app discovery), -snapshot read→eligibility gate→classify→pages-only fast apply via the seams (or full fallback), -invalidate-before-write + fresh-create-only baseline + verify-gated re-bless + generation fencing, I1-gate -exception. 6. ✅ Full offline tests (919 green). 7. ✅ Implementation review — adversarial GPT-5.6 Sol + -Claude Opus (max effort); all Critical/High findings fixed with tests (see the fix commit + this doc's v1 -contract/deferred sections). 8. ✅ **Live regression PASSED** (aurorabapenv03468, 2026-07-27): (1) fresh -`--apply --changed-only` → full build, verify 9/9, **eligible** snapshot (orgId/appId bound, debt 0); -(2) `.tsx` byte edit → `--apply --changed-only` → **FAST pages-only** (2 steps not 12, same pageId = -UPDATE, new measured deployedSha, verify 9/9, snapshot **re-blessed eligible**); (3) chart edit → -`--apply --changed-only` → **full-build fallback**, verify 9/9, snapshot **INELIGIBLE** with sticky -`chart-edit-not-convergent` + `uncertified-baseline` debt; (4) `teardown --apply` → app cascade removed, -snapshot **tombstoned then deleted** (0 leftovers). 9. ✅ Docs — design + roadmap + CHANGELOG + AGENTS.md. - -## Build-time notes (from the final Sol pass) -- Live-absence verification covers the UNION of prior-snapshot identities, current spec, generated - artifacts, and all debt entries — not merely the current spec. -- Debt-clearing verifiers must match the specific debt reason (placement verify can't clear - event/quick-view debt). -- Keep crash/failure-injection tests + a live PAC page-upload publication/concurrency test. diff --git a/plugins/model-apps/references/app-spec-schema.md b/plugins/model-apps/references/app-spec-schema.md index 3d904cf4b..04875d7ea 100644 --- a/plugins/model-apps/references/app-spec-schema.md +++ b/plugins/model-apps/references/app-spec-schema.md @@ -605,7 +605,7 @@ privilege removes it — the role converges to the spec). **Field reference** - `persona` (**required**) — the security role's display name; also its idempotency key. Must be unique across `personas[]`. - `jobs[]` (**required**, ≥1) — `{ name, description?, surfaces?, privileges[] }`. `privileges[]` is required and non-empty per job. -- `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 gap — nothing in the app demonstrably lets that persona do that job. +- `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". - `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. - `privileges[].access` (**required**) — one or more of `read · create · write · delete · append · appendTo · assign · share`. - `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 first's) — give personas distinct names, or a distinct `businessUnitId`, if you need separate roles. In `--changed-only` mode a persona change forces a **full build** (there is no partial security apply yet). +**Verification.** `verify-model-app` proves the role **row** exists carrying the SDK ownership marker +(`role`) *and* — when the reader supplies role/entity privilege access — that the role actually +**grants** every declared privilege at **at least** the declared depth (`role-privileges`). The depth +comparison is a **subset** check by design: extra privileges are never a finding, because `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). It fails +**closed** — an unreadable role, or a table whose privilege metadata cannot be read, is reported +rather than skipped. + **Validation rules** (`validateAppSpec`): `persona` required + unique; each job needs a `name` and a non-empty `privileges[]`; `access` values and `scope` must be valid tokens; `appAccess` must be a boolean; `businessUnitId` and `assignTo` ids must be GUIDs. Two apply-time checks need live metadata and diff --git a/plugins/model-apps/references/troubleshooting.md b/plugins/model-apps/references/troubleshooting.md index e9df3c1b6..5d5728ca8 100644 --- a/plugins/model-apps/references/troubleshooting.md +++ b/plugins/model-apps/references/troubleshooting.md @@ -63,6 +63,34 @@ This plugin creates **pages within existing** model-driven apps — it cannot cr --- +## Generative Page Fails to Render + +Dialog: **"Failed to load generative page"**, carrying a Dataverse OData error such as: + +> A binary operator with incompatible types was detected. Found operand types 'Edm.Guid' and 'Edm.String' for operator kind 'Equal'. + +The page itself deployed correctly — this is a **runtime data** failure, so `pac model genpage list` and the sitemap both still look right. Diagnose from the failing request, never from the dialog: + +- **Capture the query first**: F12 → Network → filter `api/data` → find the 4xx response. Copy its `$filter` (or the request body for an `Execute`/action call). That one string decides which cause below applies; the dialog alone cannot distinguish them. + +**Cause 1 — a GUID compared as a string.** Dataverse types record ids as `Edm.Guid`, and OData v4 GUID literals are **unquoted**. `accountid eq ''` raises the error above; `accountid eq ` succeeds. + +- Prefer `dataApi.retrieveRecord({ id })` over a hand-built `$filter` — it types the id for you (see `samples/10-detail-with-pageinput.tsx`) +- The same applies to lookup columns (`_ownerid_value`), which are also `Edm.Guid` + +**Cause 2 — a Custom API parameter whose declared kind is missing or wrong.** The host stamps the OData type from the *declared* kind, so an undeclared or mistyped parameter is serialized untyped and fails the same way. + +- Declare every parameter in `parameterKinds` (see `references/custom-api.md`) +- Pass a plain GUID-formatted string for a `Guid` kind — the platform wraps it into its typed form; do **not** wrap it yourself + +**Cause 3 — a GUID carrying braces.** Some Dataverse surfaces return `{xxxxxxxx-xxxx-…}`. Strip the braces before using the value in a filter or as a parameter. + +If the failing request comes from the page-**load** path rather than from code in the `.tsx`, this is a platform issue: capture the request and report it rather than editing the page. + +See: + +--- + ## RuntimeTypes Issues - Generate schema BEFORE uploading: `pac model genpage generate-types --data-sources "entity1" --output-file RuntimeTypes.ts` diff --git a/plugins/model-apps/scripts/add-page-to-solution.js b/plugins/model-apps/scripts/add-page-to-solution.js index 064e0da8a..d7bd84884 100644 --- a/plugins/model-apps/scripts/add-page-to-solution.js +++ b/plugins/model-apps/scripts/add-page-to-solution.js @@ -1,7 +1,7 @@ #!/usr/bin/env node // Adds a connector-bound GenPage and its connection references to a solution so it -// travels cross-environment. Verified live 2026-07-10 on AuroraBAPEnv03468: +// travels cross-environment. Verified live 2026-07-10 on a Dataverse test environment: // - The appmodule (type 80, AddRequiredComponents=true) pulls the sitemap (62) and // appmodulecomponent (10097) but does NOT pull the GenPage — so the GenPage's // uxagentproject row MUST be added explicitly (type 10372); adding it pulls its @@ -43,14 +43,14 @@ function connectionRefsToAdd(refs, connectorsEnabled) { const APPMODULE_COMPONENT_TYPE = 80; // Solution component type for the GenPage itself. uxagentproject IS a registered -// component type (10372 = its ObjectTypeCode), verified live 2026-07-10 on -// AuroraBAPEnv03468. It does NOT auto-travel with the appmodule, so it is added +// component type (10372 = its ObjectTypeCode), verified live 2026-07-10 on a +// Dataverse test environment. It does NOT auto-travel with the appmodule, so it is added // explicitly; AddRequiredComponents=true then pulls its uxagentprojectfile rows // (10373: page.tsx, page.compiled, config.json, firstPrompt.json). const UXAGENTPROJECT_COMPONENT_TYPE = 10372; // Solution component type for connectionreference. Verified live (2026-07-10) by // reading solutioncomponent.componenttype for an existing connection reference in the -// Default/Active solutions on AuroraBAPEnv03468 (= 10158). Note: 371 is "Connector" +// Default/Active solutions of a test environment (= 10158). Note: 371 is "Connector" // (msdyn_Connector), NOT a connection reference — AddSolutionComponent with 371 fails // with "entity ... 'msdyn_Connector' ... not found in MetadataCache". const CONNECTION_REFERENCE_COMPONENT_TYPE = 10158; diff --git a/plugins/model-apps/scripts/build-model-app.js b/plugins/model-apps/scripts/build-model-app.js index 9fe88c0d8..50014fedf 100644 --- a/plugins/model-apps/scripts/build-model-app.js +++ b/plugins/model-apps/scripts/build-model-app.js @@ -55,7 +55,11 @@ function makeSdk(env, spec, workspaceDir) { const cleanup = () => { fs.rmSync(sdkTempDir, { recursive: true, force: true }); }; - return { sdk, provisionSdk, cleanup }; + // `httpClient` is returned so the caller can wire verify's role-privilege reader, which needs the + // raw client (and the org URL) to compose an absolute `EntityDefinitions(...)?$select=Privileges` + // request — the SDK's entity metadata projects `Privileges` away. Returning the SAME instance + // rather than constructing a second one keeps token acquisition and retry state shared. + return { sdk, provisionSdk, httpClient, cleanup }; } // Turn engine progress events into a phase-grouped, status-marked build log: @@ -431,7 +435,7 @@ async function main() { }; // Construct for both dry-run and apply: proves the vendored bundle + adapter wire up // (offline), and apply needs it. A spec validation error short-circuits before any write. - const { sdk, provisionSdk, cleanup } = makeSdk(env, spec, workspaceDir); + const { sdk, provisionSdk, httpClient, cleanup } = makeSdk(env, spec, workspaceDir); // Durable build journal (apply runs only): a per-run record of steps + where a run halted, // written to /build-log.jsonl. Resume = re-run the same command (idempotent). const journal = opts.apply @@ -484,7 +488,13 @@ async function main() { const deps = { log: (m) => process.stderr.write(m + '\n'), sdk, provisionSdk, journal, - verify: (s) => verifySpec(s, readerFor(provisionSdk, appUniqueName(s), { genpageCli: makeGenpageCli(env), workspaceDir })), + // `httpClient` + `envUrl` are threaded through so the role-privileges check actually RUNS + // here. verify-spec skips it unless BOTH `rolePrivileges` and `entityPrivileges` readers are + // present, and `entityPrivileges` needs the raw client and the org URL to compose an absolute + // EntityDefinitions request. Omitting them degraded silently: `--apply --verify` reported a + // clean PASS having never checked what any persona's role actually grants. Caught live — + // standalone verify ran 10 checks against the same app where the build's inline verify ran 8. + verify: (s) => verifySpec(s, readerFor(provisionSdk, appUniqueName(s), { genpageCli: makeGenpageCli(env), workspaceDir, httpClient, envUrl: env })), }; if (changedOnly && opts.apply) { // #changed-only: the flow decides fast (pages-only via the sdk-build seams) vs full, gated on the diff --git a/plugins/model-apps/scripts/lib/app-spec.js b/plugins/model-apps/scripts/lib/app-spec.js index 4c8a9555b..7703fa94e 100644 --- a/plugins/model-apps/scripts/lib/app-spec.js +++ b/plugins/model-apps/scripts/lib/app-spec.js @@ -355,7 +355,7 @@ function resolveChoiceValue(byLabel, v, isMulti) { // Valid validation profiles. `deploy` (default) is the strictest — every page must be implemented // (a real .tsx). `design`/`plan` allow intent-only pages (author designs pages before generate-pages // writes their .tsx). `structural` ignores page implementation (teardown/cleanup only cares about refs). -// See docs/app-builder-staged-flow-design.md §7.1. +// See docs/app-builder-design.md §7.1. const VALIDATION_PROFILES = ['design', 'plan', 'deploy', 'structural']; // Normalize a page's implementation source into a discriminated shape: @@ -1202,7 +1202,7 @@ function canonicalPersonaName(persona) { // - wrap a legacy top-level `codeFile` into `source: { kind: 'tsx', codeFile }` // - rewrite name-based references (appShell page subareas + navigatesTo.targetKey) to keys // Idempotent: a spec already at schemaVersion >= 2 is returned as-is. Runs on load before validate, -// so downstream code only ever sees the v2 shape. See docs/app-builder-staged-flow-design.md §7.3. +// so downstream code only ever sees the v2 shape. See docs/app-builder-design.md §7.3. // // Two-pass design: pass 1 mints ALL keys first so nameToKey is fully populated before any // rewrite. A single rewrite pass (pass 2) then replaces every name-ref exactly once, preventing diff --git a/plugins/model-apps/scripts/lib/content-hash.js b/plugins/model-apps/scripts/lib/content-hash.js index 56dedcdf9..025698733 100644 --- a/plugins/model-apps/scripts/lib/content-hash.js +++ b/plugins/model-apps/scripts/lib/content-hash.js @@ -6,7 +6,7 @@ // inline `content`/`contentBase64`. So editing the .tsx (or the referenced web-resource file) while // leaving the spec JSON identical was INVISIBLE to diffPhases — a real content change reported as "no // change since last apply". That false negative is exactly what makes a naive partial apply unsafe, so -// the changed-only design (docs/changed-only-design.md) fixes it by hashing the on-disk content and +// the changed-only design (docs/app-builder-design.md) fixes it by hashing the on-disk content and // folding the hash into the diffed slice. // // annotateContentHashes returns a SHALLOW-CLONED spec (the pages/webResources arrays are re-mapped; diff --git a/plugins/model-apps/scripts/lib/dataverse-auth.js b/plugins/model-apps/scripts/lib/dataverse-auth.js index 220e39022..8363e166e 100644 --- a/plugins/model-apps/scripts/lib/dataverse-auth.js +++ b/plugins/model-apps/scripts/lib/dataverse-auth.js @@ -9,7 +9,7 @@ const { execFileSync } = require('child_process'); /** * Gets an Azure CLI access token for the given Dataverse environment URL. * Returns null if `az` is missing, the user isn't logged in, or the resource is unreachable. - * @param {string} envUrl - e.g. "https://aurorabapenv4ab3f.crmtest.dynamics.com" + * @param {string} envUrl - e.g. "https://contoso.crm.dynamics.com" * @returns {string|null} */ function getAuthToken(envUrl) { diff --git a/plugins/model-apps/scripts/lib/hydrate-spec.js b/plugins/model-apps/scripts/lib/hydrate-spec.js index e08c3a147..3a8a55606 100644 --- a/plugins/model-apps/scripts/lib/hydrate-spec.js +++ b/plugins/model-apps/scripts/lib/hydrate-spec.js @@ -124,7 +124,7 @@ async function hydrateSpec(read) { // a rebuild reuses this id (reconcilePageIds authority #1) instead of minting a new one, even for // a page the user added in Maker that our manifest never knew about. A portable fresh-authored // spec has no pageIds; a downloaded edit-snapshot does — that is the intended distinction. - // See references/app-spec-schema.md (pages[].pageId) and docs/changed-only-design.md. + // See references/app-spec-schema.md (pages[].pageId) and docs/app-builder-design.md. ...(p.pageId ? { pageId: p.pageId } : {}), ...(p.purpose !== undefined ? { purpose: p.purpose } : {}), ...(p.dataSources && p.dataSources.length ? { dataSources: p.dataSources } : {}), diff --git a/plugins/model-apps/scripts/lib/op-diff.js b/plugins/model-apps/scripts/lib/op-diff.js index 8a3946d49..0c9945a65 100644 --- a/plugins/model-apps/scripts/lib/op-diff.js +++ b/plugins/model-apps/scripts/lib/op-diff.js @@ -5,7 +5,7 @@ // and — for teardown — the pure teardown plan, it classifies the ops an apply WOULD perform and // surfaces the DESTRUCTIVE ones so the caller can fail closed unless --allow-destructive is set. This // module performs NO I/O and holds NO SDK handle: the caller fetches state read-only and passes it in, -// keeping classification pure and offline-testable. See docs/app-builder-staged-flow-design.md §11 +// keeping classification pure and offline-testable. See docs/app-builder-design.md §11 // (safety/autopilot) and §14 (additive commands/dashboards). // // It REUSES the existing single sources of truth rather than reimplementing them: diff --git a/plugins/model-apps/scripts/lib/page-manifest.js b/plugins/model-apps/scripts/lib/page-manifest.js index f101d6bc5..f49a33c7f 100644 --- a/plugins/model-apps/scripts/lib/page-manifest.js +++ b/plugins/model-apps/scripts/lib/page-manifest.js @@ -3,7 +3,7 @@ // carries the FULL design-time page semantics — { schemaVersion, pages:[{ key, name, pageId, purpose, // dataSources, navigatesTo, pageInput, source }], design } — so a download→edit→rebuild round-trip // restores intent + navigation that pac's page download (name + resolved-GUID source only) drops. It -// travels inside the solution and survives export/import. See docs/app-builder-staged-flow-design.md §7.3. +// travels inside the solution and survives export/import. See docs/app-builder-design.md §7.3. // PURE: the engine reads/writes the web-resource bytes; this module only shapes/parses strings (no I/O, // no SDK handle). diff --git a/plugins/model-apps/scripts/lib/pageref-resolver.js b/plugins/model-apps/scripts/lib/pageref-resolver.js index 7ce56a9ff..c9281a55f 100644 --- a/plugins/model-apps/scripts/lib/pageref-resolver.js +++ b/plugins/model-apps/scripts/lib/pageref-resolver.js @@ -3,7 +3,7 @@ // symbolic token — pageId: "PAGEREF_" — because the real GenPageId is minted by the server at // deploy time and differs per environment (SDK opaque-identity rule T5: never bake a resolved GUID into // canonical source, or a cross-env recreate ships a dead link). See references/rules.md "Generative Page -// Navigation" and docs/app-builder-staged-flow-design.md §9. The ENGINE reads/writes files; this module +// Navigation" and docs/app-builder-design.md §9. The ENGINE reads/writes files; this module // only shapes/parses strings (pure, offline). // // THE SINGLE NAV ORACLE. `extractNavTargets` parses the ACTUAL diff --git a/plugins/model-apps/scripts/lib/role-privileges.js b/plugins/model-apps/scripts/lib/role-privileges.js new file mode 100644 index 000000000..f5584dc72 --- /dev/null +++ b/plugins/model-apps/scripts/lib/role-privileges.js @@ -0,0 +1,119 @@ +// plugins/model-apps/scripts/lib/role-privileges.js +// PURE: what privileges a persona's security role MUST hold, and whether a deployed role holds them. +// +// WHY this exists. `verifySpec`'s role check proved only that a role ROW exists carrying the SDK +// ownership marker. It never looked at the role's privileges — so a role created with the wrong +// access, or one whose privilege write silently failed after the row was created, verified clean. +// That is a metadata read, which is why it is cheap enough to run on every verify. +//// SUBSET, not equality. We assert the role holds AT LEAST every declared privilege at AT LEAST the +// declared depth. Extra privileges are never a finding, and that is deliberate — three legitimate +// sources add privileges the spec does not literally list: +// 1. `appAccess` injects `appmodule` read (see personaRoleSpecFor). +// 2. Several jobs unioned together escalate a shared entity+access to the MAX declared scope. +// 3. Distinct entities can share ONE Dataverse privilege, and a role holds one depth per +// privilege — so the SDK raises that privilege to the highest scope any of them asked for. +// An equality check would fail on all three while telling us nothing true. +// +// Dataverse reference — privilege depth (`Depth` on ReplacePrivilegesRole, `RolePrivilegeDepth`): +// Basic (user) < Local (business unit) < Deep (parent/child) < Global (organization). +// https://learn.microsoft.com/en-us/power-apps/developer/data-platform/security-model +'use strict'; + +// App Spec scope -> Dataverse depth name, and its ORDER. The order is what makes this a subset +// check: a role holding Global satisfies a declared Basic. Mirrors the vendored SDK's own mapping +// (verified against scripts/vendor/cds-maker-sdk.cjs) so a comparison cannot disagree with the write. +const SCOPE_DEPTH = { user: 'Basic', businessUnit: 'Local', parentChild: 'Deep', organization: 'Global' }; +const DEPTH_RANK = { basic: 1, local: 2, deep: 3, global: 4 }; +// App Spec access token -> Dataverse PrivilegeType, again mirroring the SDK. +const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' }; + +const rankOf = (depth) => DEPTH_RANK[String(depth == null ? '' : depth).trim().toLowerCase()] || 0; + +// `roleprivileges.privilegedepthmask` -> the depth NAME the comparison speaks in. +// +// The column is a BITMASK, not an enum: 1 Basic, 2 Local, 4 Deep, 8 Global. A single value can +// therefore carry SEVERAL bits — 3 (Basic|Local), 7 (Basic|Local|Deep), 15 (all four) are all legal +// — because privilege depth is CUMULATIVE: a role granted Global can also do everything Basic +// allows. The effective depth is therefore the HIGHEST bit set, never an exact match. +// +// An exact-match lookup (`{1:'Basic',2:'Local',4:'Deep',8:'Global'}[mask]`) mapped every combined +// value to '', which ranks 0, which made `compareRolePrivileges` report a CORRECTLY configured role +// as missing or too shallow — the cry-wolf failure this check exists to avoid. +// +// Fail closed on 0, a negative, a non-number, or a value carrying only bits we do not recognise: +// an undecodable mask is not evidence of a grant. +// https://learn.microsoft.com/en-us/power-apps/developer/data-platform/reference/entities/roleprivileges +function depthFromMask(mask) { + const m = Number(mask); + if (!Number.isFinite(m) || m <= 0) return ''; + if (m & 8) return 'Global'; + if (m & 4) return 'Deep'; + if (m & 2) return 'Local'; + if (m & 1) return 'Basic'; + return ''; +} + +// Flatten a persona to the (entity, access, scope) triples its role must satisfy, taking the MAX +// scope per (entity, access) exactly as the builder's union does. `appAccess` is folded in here so +// the expectation matches what the build actually writes rather than what the author typed. +function declaredPrivileges(persona) { + const byKey = new Map(); // "|" -> { entity, access, scope } + const addAll = (list) => { + for (const pr of list || []) { + if (!pr || !pr.entity) continue; + const entity = String(pr.entity).trim().toLowerCase(); + const scope = pr.scope || 'user'; + for (const a of pr.access || []) { + const access = String(a).trim(); + if (!access) continue; + const key = `${entity}|${access.toLowerCase()}`; + const prev = byKey.get(key); + // Max scope wins — the same rule the builder applies when unioning jobs into one role. + if (!prev || rankOf(SCOPE_DEPTH[scope]) > rankOf(SCOPE_DEPTH[prev.scope])) byKey.set(key, { entity, access, scope }); + } + } + }; + for (const j of (persona && persona.jobs) || []) addAll(j && j.privileges); + addAll(persona && persona.additionalPrivileges); + // Mirrors personaRoleSpecFor: unless the persona opts out, the build grants appmodule read so the + // app actually opens for them. Verifying it matters — without it the role exists but the app does not. + if (!persona || persona.appAccess !== false) addAll([{ entity: 'appmodule', access: ['read'], scope: 'organization' }]); + return [...byKey.values()]; +} + +// Compare declared privileges against what the role actually holds. +// `entityPrivileges`: Map — from +// EntityDefinitions(LogicalName='x')?$select=Privileges, the SAME source the +// SDK resolves against when it writes. +// `actualByPrivilegeId`: Map +// Returns { ok, missing:[{ entity, access, scope, reason, privilegeName? }] }. +// An entity whose metadata could not be read is reported as a finding, never skipped — a read +// failure must not read as "nothing missing" (fail closed). +function compareRolePrivileges(declared, entityPrivileges, actualByPrivilegeId) { + const missing = []; + for (const d of declared) { + const privs = entityPrivileges.get(d.entity); + if (!privs) { + missing.push({ ...d, reason: `could not read privilege metadata for '${d.entity}'` }); + continue; + } + const type = ACCESS_TYPE[d.access.toLowerCase()]; + const p = type && privs.find((x) => x && x.PrivilegeType === type); + if (!p) { + missing.push({ ...d, reason: `'${d.entity}' exposes no '${d.access}' privilege` }); + continue; + } + const held = actualByPrivilegeId.get(String(p.PrivilegeId || '').trim().toLowerCase()); + if (!held) { + missing.push({ ...d, privilegeName: p.Name, reason: `role does not hold ${p.Name}` }); + continue; + } + const want = SCOPE_DEPTH[d.scope] || 'Basic'; + if (rankOf(held) < rankOf(want)) { + missing.push({ ...d, privilegeName: p.Name, reason: `role holds ${p.Name} at ${held}, below the declared ${want}` }); + } + } + return { ok: missing.length === 0, missing }; +} + +module.exports = { declaredPrivileges, compareRolePrivileges, depthFromMask, SCOPE_DEPTH, ACCESS_TYPE, DEPTH_RANK }; diff --git a/plugins/model-apps/scripts/lib/schema-facts.js b/plugins/model-apps/scripts/lib/schema-facts.js index c39cf085e..6b8cead65 100644 --- a/plugins/model-apps/scripts/lib/schema-facts.js +++ b/plugins/model-apps/scripts/lib/schema-facts.js @@ -5,7 +5,7 @@ // createColumn / createRelationship — entity-provision.js:167-291) into stable, comparable facts the // offline eval harness diffs per build. Pure (no I/O, no SDK): it derives everything deterministically // from the App Spec, reusing the SAME naming/value rules the engine uses, so a fact equals what WOULD -// be provisioned — not a naive spec echo. See docs/app-builder-staged-flow-design.md §13.2, §14. +// be provisioned — not a naive spec echo. See docs/app-builder-design.md §13.2, §14. const { columnTypeMap, choiceValueMap, relationshipSchemaName, manyToManySchemaName, quickCreateEnabledFor } = require('./app-spec.js'); const lc = (s) => String(s || '').toLowerCase(); diff --git a/plugins/model-apps/scripts/lib/sdk-build.js b/plugins/model-apps/scripts/lib/sdk-build.js index 0a5ad7960..45d6f8311 100644 --- a/plugins/model-apps/scripts/lib/sdk-build.js +++ b/plugins/model-apps/scripts/lib/sdk-build.js @@ -1842,7 +1842,7 @@ async function runSdkBuild(spec, opts = {}) { result.created.roles[roleName] = rr; // Ensure the role is in the app's solution on EVERY run so an export/import carries it. // AddSolutionComponent is idempotent server-side (re-adding an existing component returns 200 — - // live-verified on aurora), so this ALSO repairs a role that missed membership on a prior run + // live-verified), so this ALSO repairs a role that missed membership on a prior run // (created, then the add failed, then reused — a `!reused` gate would skip the repair forever). // NOT swallowed: a real failure means the exported solution would omit the role and silently break // access in the target env, so it fails the phase fail-closed — same as the app/sitemap adds. diff --git a/plugins/model-apps/scripts/lib/sitemap-pages.js b/plugins/model-apps/scripts/lib/sitemap-pages.js index 76a52b213..aa4338e5b 100644 --- a/plugins/model-apps/scripts/lib/sitemap-pages.js +++ b/plugins/model-apps/scripts/lib/sitemap-pages.js @@ -149,7 +149,7 @@ async function fetchSitemap(sdk, appUnique) { } // Env-wide MEMBERSHIP scan (Imp5): which apps' sitemaps reference each id in `pageIds`. GROUNDED by a live -// Dataverse probe (aurorabapenv03468): a generative page has NO `appmodulecomponent` row — +// Dataverse probe (test environment): a generative page has NO `appmodulecomponent` row — // `appmodulecomponents?$filter=objectid eq ` returns 0 rows. So a genpage's app membership lives // ONLY inside the sitemap XML (`GenPageId="…"`), with NO direct genpage→apps join. The ONLY way to find a // page shared across apps is to scan every OTHER app's sitemap XML for the id. diff --git a/plugins/model-apps/scripts/lib/spec-lint.js b/plugins/model-apps/scripts/lib/spec-lint.js index 2d4fefef1..55c313cd5 100644 --- a/plugins/model-apps/scripts/lib/spec-lint.js +++ b/plugins/model-apps/scripts/lib/spec-lint.js @@ -4,6 +4,7 @@ // relationship schema-name vs lookup-name collision Dataverse rejects. const { relationshipSchemaName, relationshipFor, invalidChoiceSampleTokens, isPlatformIconRef } = require('./app-spec.js'); const { normalizeSpecShape } = require('./spec-shape.js'); +const { resolveSurfaces, unresolvedSurfaceMessage } = require('./surface-resolver.js'); const CHOICE_OPTION_WARN = 12; const SEQNUM_RE = /\{SEQNUM(:\d+)?\}/i; @@ -390,6 +391,11 @@ function lintAppSpec(spec) { 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.`); } } + // A surface that names nothing this spec builds is the NEXT failure after "no surfaces at all": + // the job claims coverage that does not exist. Only a warning, because a surface may legitimately + // name an out-of-the-box artifact this spec never authors (the same reason app-spec.js validates + // surfaces as shape-only) — see lib/surface-resolver.js. + for (const u of resolveSurfaces(spec).unresolved) W(unresolvedSurfaceMessage(u)); if (!(Array.isArray(spec.pages) && spec.pages.length)) { 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.'); } diff --git a/plugins/model-apps/scripts/lib/stages.js b/plugins/model-apps/scripts/lib/stages.js index 0ecadcac3..37ca32608 100644 --- a/plugins/model-apps/scripts/lib/stages.js +++ b/plugins/model-apps/scripts/lib/stages.js @@ -5,7 +5,7 @@ // load-bearing: phases run top-to-bottom, and downstream phases depend on earlier ones (e.g. the // app phase consumes forms/views/charts built earlier). The `security` phase runs AFTER app-shell // because a persona role grants read on (and is associated to) the app module created there, so the -// generated app opens for that persona. See docs/app-builder-staged-flow-design.md §6. +// generated app opens for that persona. See docs/app-builder-design.md §6. const PHASES = ['solution', 'data-model', 'sample-data', 'web-resources', 'views', 'charts', 'forms', 'commands', 'dashboards', 'app-shell', 'pages', 'ai-features', 'security', 'publish']; // User-facing stages, each a contiguous range of engine phases. Stages are the vocabulary the diff --git a/plugins/model-apps/scripts/lib/surface-resolver.js b/plugins/model-apps/scripts/lib/surface-resolver.js new file mode 100644 index 000000000..16e60dc86 --- /dev/null +++ b/plugins/model-apps/scripts/lib/surface-resolver.js @@ -0,0 +1,95 @@ +// plugins/model-apps/scripts/lib/surface-resolver.js +// PURE: resolve each `personas[].jobs[].surfaces[]` entry to the App Spec artifact that satisfies it. +// +// WHY this exists. `surfaces[]` is the only machine-readable claim in the spec that says *this job is +// satisfied by these screens*. Until now it was documentary — `app-spec.js` validates that each entry +// is a non-empty string and stops, and `spec-lint.js` warns only when the array is EMPTY. So a job +// could name "My Open Work Orders" when no such view existed anywhere in the spec and every gate +// passed. That is a coherence defect the build cannot catch either: the builder never reads +// `surfaces[]`, so nothing downstream fails. +// +// Deliberately a WARNING, not an error. The shape-only validation in `app-spec.js` is loose on +// purpose — its comment says a surface "may legitimately name an artifact this spec doesn't author +// (a stock view)". A spec that names the OOB "Active Accounts" view is correct and must keep +// validating, so an unresolved surface is reported as a design smell for a human to judge, never a +// hard failure. +'use strict'; + +const norm = (s) => String(s == null ? '' : s).trim().toLowerCase(); + +// Every artifact kind a surface may legitimately name, with the field(s) a human would write. +// Sitemap subareas are included because an author frequently names the NAV ENTRY ("Work Orders") +// rather than the underlying view — that is a real answer to "what lets them do the job", so +// treating it as unresolved would produce noise instead of signal. +function buildIndex(spec) { + const index = new Map(); // normalized name -> [{ kind, name, entity? }] + const add = (name, entry) => { + const k = norm(name); + if (!k) return; + if (!index.has(k)) index.set(k, []); + // De-dupe identical (kind,name,entity) triples — e.g. a view named the same as its subarea title + // would otherwise report twice and read like two separate matches. + const list = index.get(k); + if (!list.some((e) => e.kind === entry.kind && norm(e.name) === norm(entry.name) && norm(e.entity || '') === norm(entry.entity || ''))) list.push(entry); + }; + + 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 + // either — an author writing the plan will reach for whichever they saw last. + add(p && p.name, { kind: 'page', name: p.name, key: p.key }); + add(p && p.key, { kind: 'page', name: p.name, key: p.key }); + } + for (const e of spec.entities || []) { + // A table itself is a surface when the app exposes it as a nav entry (grid + form). Accept the + // display name and the schema name. + add(e && e.displayName, { kind: 'entity', name: e.displayName || e.schemaName, entity: e.schemaName }); + add(e && e.schemaName, { kind: 'entity', name: e.displayName || e.schemaName, entity: e.schemaName }); + } + for (const a of (spec.appShell && spec.appShell.areas) || []) { + for (const g of (a && a.groups) || []) { + for (const sa of (g && g.subAreas) || []) add(sa && sa.title, { kind: 'subarea', name: sa.title }); + } + } + return index; +} + +// Flatten every (persona, job, surface) triple the spec declares. Jobs with no `surfaces[]` are NOT +// reported here — `spec-lint.js` already warns about an unmapped job, and duplicating it would +// double-report the same design gap. +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() }); + } + } + } + return out; +} + +// Resolve every declared surface against the spec's own artifacts. +// Returns { resolved: [{ persona, job, surface, matches:[{kind,...}] }], unresolved: [{ persona, job, surface }] }. +function resolveSurfaces(spec) { + const resolved = []; + const unresolved = []; + if (!spec || typeof spec !== 'object') return { resolved, unresolved }; + const index = buildIndex(spec); + for (const d of declaredSurfaces(spec)) { + const matches = index.get(norm(d.surface)); + if (matches && matches.length) resolved.push({ ...d, matches: matches.slice() }); + else unresolved.push({ ...d }); + } + return { resolved, unresolved }; +} + +// The lint/verify message for an unresolved surface. Shared so the two gates cannot word the same +// finding differently — a maker who sees it twice should see it identically. +function unresolvedSurfaceMessage(u) { + return `persona "${u.persona}" job "${u.job}": surface "${u.surface}" does not match any view, form, page, dashboard, table or sitemap entry in this spec — either it names an out-of-the-box artifact (fine) or the job is not actually covered by anything this app builds.`; +} + +module.exports = { resolveSurfaces, declaredSurfaces, unresolvedSurfaceMessage }; diff --git a/plugins/model-apps/scripts/lib/verify-spec.js b/plugins/model-apps/scripts/lib/verify-spec.js index 9318bb74c..ee5bfad76 100644 --- a/plugins/model-apps/scripts/lib/verify-spec.js +++ b/plugins/model-apps/scripts/lib/verify-spec.js @@ -9,6 +9,8 @@ const { normalizePageSource, relationshipSchemaName, manyToManySchemaName, SDK_R const { resolveExistingFormId, resolveRoleBusinessUnit, roleBuClause, appUniqueName } = require('./sdk-build.js'); const { extractNavTargets } = require('./pageref-resolver.js'); const { AI_APP_SETTING, resolveAiFlags, featureWantValue, sameSettingValue, resolveAppModuleId, proveAppOverride } = require('./ai-app-settings.js'); +const { declaredPrivileges, compareRolePrivileges } = require('./role-privileges.js'); +const { resolveSurfaces } = require('./surface-resolver.js'); // The PER-APP setting each AI feature writes now lives in ./ai-app-settings.js, together with the // flag-resolution and override-proof helpers the BUILD uses — see that module for why one source of @@ -261,6 +263,37 @@ async function verifySpec(spec, read, opts = {}) { } } catch { row = undefined; } add('role', roleName, row, row ? '' : 'persona security role not found (or its business unit could not be resolved)'); + + // Privilege depth check — reader-gated (see `entityRelationships` / `commandBar` above for the + // same pattern), so an existence-only reader behaves exactly as before. Proving the role ROW + // exists says nothing about what it GRANTS: a role created with the wrong access, or one whose + // privilege write failed after the row landed, verified clean until this check existed. + // SUBSET semantics — see lib/role-privileges.js for why equality would be wrong. + if (row && typeof read.rolePrivileges === 'function' && typeof read.entityPrivileges === 'function') { + const declared = declaredPrivileges(p); + let actual = null; + try { + actual = await read.rolePrivileges(row.roleid); + } catch { actual = null; } + if (!Array.isArray(actual)) { + // Fail CLOSED: an unreadable role is not a role we can call correct. + add('role-privileges', roleName, false, 'could not read the role\'s privileges'); + } else { + const actualByPrivilegeId = new Map(actual.map((a) => [String((a && a.privilegeId) || '').trim().toLowerCase(), a && a.depth])); + const entityPrivileges = new Map(); + for (const entity of new Set(declared.map((d) => d.entity))) { + try { + const privs = await read.entityPrivileges(entity); + if (Array.isArray(privs)) entityPrivileges.set(entity, privs); + } catch { /* left absent → reported as a finding by compareRolePrivileges */ } + } + const cmp = compareRolePrivileges(declared, entityPrivileges, actualByPrivilegeId); + const detail = cmp.ok + ? `${declared.length} declared privilege(s) held` + : cmp.missing.map((m) => `${m.entity}.${m.access}: ${m.reason}`).join('; '); + add('role-privileges', roleName, cmp.ok, detail); + } + } } // AI app features. The verifier previously had NO awareness of `spec.ai` at all, so a build whose @@ -331,6 +364,38 @@ async function verifySpec(spec, read, opts = {}) { } } + // JTBD rollup — translates a technical failure into the business impact it caused. Every other + // check answers "is this artifact deployed?"; this one answers "can this persona still do this + // job?". + // + // Deliberately a PURE ROLLUP over checks already computed — no extra reads, so it costs nothing + // and cannot fail independently. A job fails when a surface it names resolves to a spec artifact + // whose own check failed. An UNRESOLVED surface is NOT failed here: it may name an out-of-the-box + // artifact this spec never authors (see lib/surface-resolver.js), and spec-lint already warns at + // authoring time — failing it here would turn a plan-time smell into a deploy-time error. + const failedNames = new Set(checks.filter((c) => !c.present).map((c) => String(c.name).toLowerCase())); + if (failedNames.size) { + // Each check kind names itself differently (a view is `.`, a form/page/subarea is + // its bare name, an entity is its schemaName), so a resolved surface is mapped to the candidate + // check-name(s) its kind would have produced. Derived here rather than in the resolver because + // the naming convention belongs to THIS file — a resolver that guessed it would silently rot + // the moment a check kind renamed itself. + const candidatesFor = (m) => { + const name = String(m.name || ''); + if (m.kind === 'view') return [`${String(m.entity || '').toLowerCase()}.${name}`]; + if (m.kind === 'entity') return [String(m.entity || name)]; + return [name]; // form · page · subarea · dashboard + }; + for (const r of resolveSurfaces(spec).resolved) { + const broken = r.matches + .flatMap(candidatesFor) + .filter((n) => n && failedNames.has(n.toLowerCase())); + if (broken.length) { + add('job-surface', `${r.persona} → ${r.job}`, false, `surface "${r.surface}" is not deployed (${[...new Set(broken)].join(', ')})`); + } + } + } + const missing2 = checks.filter((c) => !c.present); // Keep unableToRun absent (undefined) on the normal path so existing callers and tests are unaffected. return { ok: missing2.length === 0 && !unableToRun, checks, missing: missing2, unableToRun: unableToRun || undefined }; diff --git a/plugins/model-apps/scripts/tests/build-model-app.test.js b/plugins/model-apps/scripts/tests/build-model-app.test.js index 297b95bd5..e88b8f697 100644 --- a/plugins/model-apps/scripts/tests/build-model-app.test.js +++ b/plugins/model-apps/scripts/tests/build-model-app.test.js @@ -488,3 +488,26 @@ test('unableToRun is propagated from verifySpec into r.verify (RECONCILIATION 1) assert.strictEqual(r.verify.ok, false); assert.strictEqual(r.verify.unableToRun, true, 'unableToRun propagated from verifySpec result'); }); + +// -- verify wiring ----------------------------------------------------------------------------- +// Asserted against SOURCE because the wiring lives inside main(), which is not exported, and the +// failure mode is SILENT: verify-spec skips role-privileges unless BOTH readers are present, so +// omitting httpClient/envUrl made --apply --verify report a clean PASS having never checked what +// any persona role actually grants. Found live -- the standalone verifier ran 10 checks against the +// same app where the build inline verify ran 8. A behavioural test would need a live SDK; this pins +// the exact regression at zero cost. +test('build --verify wires the role-privilege readers (httpClient + envUrl)', () => { + const src = fs.readFileSync(path.join(__dirname, '..', 'build-model-app.js'), 'utf8'); + const call = src.split(/\r?\n/).find((l) => l.includes('verify: (s) => verifySpec')); + assert.ok(call, 'expected the deps.verify wiring line'); + assert.match(call, /httpClient/, 'entityPrivileges needs the raw client'); + assert.match(call, /envUrl: env/, 'entityPrivileges needs the org url to build an absolute request'); +}); + +test('makeSdk returns the httpClient so the caller can wire verify', () => { + // Returning the SAME instance rather than constructing a second one keeps token acquisition and + // retry state shared; a second client would re-acquire a token per verify run. + const src = fs.readFileSync(path.join(__dirname, '..', 'build-model-app.js'), 'utf8'); + assert.match(src, /return \{ sdk, provisionSdk, httpClient, cleanup \}/, 'makeSdk must expose httpClient'); + assert.match(src, /const \{ sdk, provisionSdk, httpClient, cleanup \} = makeSdk\(/, 'main must destructure it'); +}); diff --git a/plugins/model-apps/scripts/tests/genpage-cli.test.js b/plugins/model-apps/scripts/tests/genpage-cli.test.js index db79abe21..eff331739 100644 --- a/plugins/model-apps/scripts/tests/genpage-cli.test.js +++ b/plugins/model-apps/scripts/tests/genpage-cli.test.js @@ -6,7 +6,7 @@ const { makeGenpageCli, parsePageId, parseList, quoteArg, buildPacInvocation, cl const GUID = '6e0c28a2-cdbf-41ec-9186-d10fd5de6e35'; // REAL `pac model genpage list` output — a fixed-width TABLE (header + GUID/Name/Published rows), captured -// LIVE from env aurorabapenv03468 (2026-07). Columns auto-size to the longest name. `listText` reproduces +// LIVE from a Dataverse test environment (2026-07). Columns auto-size to the longest name. `listText` reproduces // that exact shape; `countOverride` lets a test set the "Found N" summary independently of the rows (to // exercise the count-mismatch fail-closed path). NOTE (live): pac lists only SITEMAP-reachable pages. function listText(rows, countOverride) { diff --git a/plugins/model-apps/scripts/tests/role-privileges.test.js b/plugins/model-apps/scripts/tests/role-privileges.test.js new file mode 100644 index 000000000..5bbdfacc7 --- /dev/null +++ b/plugins/model-apps/scripts/tests/role-privileges.test.js @@ -0,0 +1,208 @@ +'use strict'; +// `verifySpec`'s role check proved only that a role ROW exists carrying the SDK marker, never what +// it GRANTS. A role created with the wrong access — or one whose privilege write failed after the +// row landed — verified clean. This is a metadata read, not a runtime check. +const { test } = require('node:test'); +const assert = require('node:assert'); +const { declaredPrivileges, compareRolePrivileges, depthFromMask } = require('../lib/role-privileges.js'); +const { verifySpec } = require('../lib/verify-spec.js'); + +// A realistic privilege set as EntityDefinitions returns it. +const privsFor = (entity, id) => [ + { Name: `prvRead${entity}`, PrivilegeId: `${id}-read`, PrivilegeType: 'Read' }, + { Name: `prvWrite${entity}`, PrivilegeId: `${id}-write`, PrivilegeType: 'Write' }, + { Name: `prvCreate${entity}`, PrivilegeId: `${id}-create`, PrivilegeType: 'Create' }, +]; + +const persona = (over = {}) => ({ + persona: 'Field Technician', + jobs: [{ name: 'Complete work orders', privileges: [{ entity: 'msdyn_workorder', access: ['read', 'write'], scope: 'businessUnit' }] }], + ...over, +}); + +test('declaredPrivileges flattens jobs and folds in the appmodule read appAccess injects', () => { + const d = declaredPrivileges(persona()); + assert.deepStrictEqual(d.sort((a, b) => `${a.entity}${a.access}`.localeCompare(`${b.entity}${b.access}`)), [ + { entity: 'appmodule', access: 'read', scope: 'organization' }, + { entity: 'msdyn_workorder', access: 'read', scope: 'businessUnit' }, + { entity: 'msdyn_workorder', access: 'write', scope: 'businessUnit' }, + ]); +}); + +test('appAccess:false drops the appmodule privilege (a data-only role)', () => { + assert.ok(!declaredPrivileges(persona({ appAccess: false })).some((d) => d.entity === 'appmodule')); +}); + +// Mirrors the builder's union rule, so the expectation matches what is actually written. +test('the MAX scope wins when several jobs declare the same entity+access', () => { + const p = persona({ jobs: [ + { name: 'a', privileges: [{ entity: 'account', access: ['read'], scope: 'user' }] }, + { name: 'b', privileges: [{ entity: 'account', access: ['read'], scope: 'organization' }] }, + ] }); + const acct = declaredPrivileges(p).filter((d) => d.entity === 'account'); + assert.deepStrictEqual(acct, [{ entity: 'account', access: 'read', scope: 'organization' }]); +}); + +test('additionalPrivileges are folded in like a job\'s privileges', () => { + const d = declaredPrivileges(persona({ additionalPrivileges: [{ entity: 'product', access: ['read'], scope: 'organization' }] })); + assert.ok(d.some((x) => x.entity === 'product' && x.access === 'read')); +}); + +// --- comparison --------------------------------------------------------------------------------- + +const entityPrivs = () => new Map([['msdyn_workorder', privsFor('Workorder', 'wo')], ['appmodule', privsFor('AppModule', 'app')]]); +const heldAll = () => new Map([['wo-read', 'Local'], ['wo-write', 'Local'], ['app-read', 'Global']]); + +test('a role holding every declared privilege at the declared depth passes', () => { + const r = compareRolePrivileges(declaredPrivileges(persona()), entityPrivs(), heldAll()); + assert.deepStrictEqual(r, { ok: true, missing: [] }); +}); + +// SUBSET semantics: a deeper grant satisfies a shallower declaration. Equality would false-fail on +// the builder's own max-scope union and on shared Dataverse privileges. +test('a DEEPER grant than declared still passes', () => { + const held = new Map([['wo-read', 'Global'], ['wo-write', 'Global'], ['app-read', 'Global']]); + assert.strictEqual(compareRolePrivileges(declaredPrivileges(persona()), entityPrivs(), held).ok, true); +}); + +test('extra privileges the spec never declared are NOT a finding', () => { + const held = heldAll(); + held.set('some-other-privilege', 'Global'); + assert.strictEqual(compareRolePrivileges(declaredPrivileges(persona()), entityPrivs(), held).ok, true); +}); + +// THE regression this check exists for. +test('a privilege the role does not hold is reported by name', () => { + const held = heldAll(); + held.delete('wo-write'); + const r = compareRolePrivileges(declaredPrivileges(persona()), entityPrivs(), held); + assert.strictEqual(r.ok, false); + assert.strictEqual(r.missing.length, 1); + assert.strictEqual(r.missing[0].access, 'write'); + assert.match(r.missing[0].reason, /role does not hold prvWriteWorkorder/); +}); + +test('a privilege held at too SHALLOW a depth is reported with both depths', () => { + const held = heldAll(); + held.set('wo-write', 'Basic'); // declared businessUnit => Local + const r = compareRolePrivileges(declaredPrivileges(persona()), entityPrivs(), held); + assert.strictEqual(r.ok, false); + assert.match(r.missing[0].reason, /at Basic, below the declared Local/); +}); + +// Fail closed: an unreadable table must not read as "nothing missing". +test('an entity whose privilege metadata could not be read is a finding, not a skip', () => { + const r = compareRolePrivileges(declaredPrivileges(persona()), new Map(), heldAll()); + assert.strictEqual(r.ok, false); + assert.ok(r.missing.every((m) => /could not read privilege metadata/.test(m.reason))); +}); + +test('an access level the table does not expose is reported', () => { + const p = persona({ jobs: [{ name: 'a', privileges: [{ entity: 'msdyn_workorder', access: ['delete'] }] }], appAccess: false }); + const r = compareRolePrivileges(declaredPrivileges(p), entityPrivs(), heldAll()); + assert.strictEqual(r.ok, false); + assert.match(r.missing[0].reason, /exposes no 'delete' privilege/); +}); + +// --- verifySpec wiring -------------------------------------------------------------------------- + +const SDK_MARKER = 'Authored by @maker-studio/cds-maker-sdk (persona/security role).'; +const specWith = (p) => ({ solution: { uniqueName: 'S' }, app: { name: 'A' }, entities: [], views: [], charts: [], forms: [], appShell: { areas: [] }, personas: [p] }); + +function readerWith({ rolePrivileges, entityPrivileges } = {}) { + const base = { + findTable: async () => null, + findColumns: async () => [], + sitemapXml: async () => '', + queryRecords: async (set) => { + if (set === 'businessunit') return [{ businessunitid: '11111111-1111-1111-1111-111111111111' }]; + if (set === 'role') return [{ roleid: 'role-1', description: SDK_MARKER, ismanaged: false }]; + return []; + }, + }; + if (rolePrivileges) base.rolePrivileges = rolePrivileges; + if (entityPrivileges) base.entityPrivileges = entityPrivileges; + return base; +} + +// Reader-gated, exactly like entityRelationships / commandBar: an existence-only reader must behave +// as it did before this check existed. +test('verifySpec does NOT emit a role-privileges check when the reader lacks the methods', async () => { + const r = await verifySpec(specWith(persona()), readerWith()); + assert.ok(r.checks.some((c) => c.kind === 'role'), 'the existing role check still runs'); + assert.ok(!r.checks.some((c) => c.kind === 'role-privileges'), 'no privilege check without the readers'); +}); + +test('verifySpec PASSES the privilege check when the role holds everything declared', async () => { + const r = await verifySpec(specWith(persona()), readerWith({ + rolePrivileges: async () => [{ privilegeId: 'wo-read', depth: 'Local' }, { privilegeId: 'wo-write', depth: 'Local' }, { privilegeId: 'app-read', depth: 'Global' }], + entityPrivileges: async (e) => entityPrivs().get(e) || null, + })); + const c = r.checks.find((x) => x.kind === 'role-privileges'); + assert.ok(c && c.present, JSON.stringify(c)); +}); + +test('verifySpec FAILS when the role is missing a declared privilege', async () => { + const r = await verifySpec(specWith(persona()), readerWith({ + rolePrivileges: async () => [{ privilegeId: 'wo-read', depth: 'Local' }, { privilegeId: 'app-read', depth: 'Global' }], + entityPrivileges: async (e) => entityPrivs().get(e) || null, + })); + const c = r.checks.find((x) => x.kind === 'role-privileges'); + assert.ok(c && !c.present); + assert.match(c.detail, /msdyn_workorder\.write/); + assert.strictEqual(r.ok, false); +}); + +test('verifySpec fails CLOSED when the role privileges cannot be read', async () => { + const r = await verifySpec(specWith(persona()), readerWith({ + rolePrivileges: async () => { throw new Error('429 throttled'); }, + entityPrivileges: async (e) => entityPrivs().get(e) || null, + })); + const c = r.checks.find((x) => x.kind === 'role-privileges'); + assert.ok(c && !c.present); + assert.match(c.detail, /could not read/); +}); + +// -- depthFromMask ---------------------------------------------------------------------------- +// Review finding: privilegedepthmask is a BITMASK, not an enum. It can carry several bits at once, +// and an exact-match lookup mapped every combined value to '' -- which ranks 0, which made +// compareRolePrivileges report a CORRECTLY configured role as missing or too shallow. + +test('depthFromMask: single bits map to their depth', () => { + assert.strictEqual(depthFromMask(1), 'Basic'); + assert.strictEqual(depthFromMask(2), 'Local'); + assert.strictEqual(depthFromMask(4), 'Deep'); + assert.strictEqual(depthFromMask(8), 'Global'); +}); + +test('depthFromMask: COMBINED masks resolve to the highest bit, not to empty', () => { + // Depth is cumulative -- a role granted Global can also do everything Basic allows -- so the + // effective depth is the deepest bit present. + assert.strictEqual(depthFromMask(3), 'Local'); // Basic|Local + assert.strictEqual(depthFromMask(7), 'Deep'); // Basic|Local|Deep + assert.strictEqual(depthFromMask(15), 'Global'); // all four + assert.strictEqual(depthFromMask(12), 'Global'); // Deep|Global + assert.strictEqual(depthFromMask(6), 'Deep'); // Local|Deep +}); + +test('depthFromMask: a combined mask satisfies a declared scope instead of false-failing', () => { + // The end-to-end point of the fix: a role holding mask 15 must satisfy a declared organization + // scope. Before, it decoded to '' and was reported as too shallow. + const declared = [{ entity: 'account', access: 'read', scope: 'organization' }]; + const entityPrivileges = new Map([['account', [{ Name: 'prvReadAccount', PrivilegeId: 'p1', PrivilegeType: 'Read' }]]]); + const actual = new Map([['p1', depthFromMask(15)]]); + const cmp = compareRolePrivileges(declared, entityPrivileges, actual); + assert.strictEqual(cmp.ok, true, JSON.stringify(cmp.missing)); +}); + +test('depthFromMask: fails closed on an undecodable mask', () => { + // An unreadable mask is not evidence of a grant. + for (const bad of [0, -1, null, undefined, '', 'abc', NaN, 16]) { + assert.strictEqual(depthFromMask(bad), '', String(bad) + ' should not decode'); + } +}); + +test('depthFromMask: tolerates a numeric string', () => { + assert.strictEqual(depthFromMask('8'), 'Global'); + assert.strictEqual(depthFromMask('7'), 'Deep'); +}); diff --git a/plugins/model-apps/scripts/tests/surface-resolver.test.js b/plugins/model-apps/scripts/tests/surface-resolver.test.js new file mode 100644 index 000000000..c6b04c30a --- /dev/null +++ b/plugins/model-apps/scripts/tests/surface-resolver.test.js @@ -0,0 +1,148 @@ +'use strict'; +// `surfaces[]` was documentary only, so a job could claim a screen that exists nowhere in the spec +// and every gate stayed green. These tests pin the resolution and — just as importantly — pin that +// an unresolved surface stays a WARNING, because a surface may legitimately name an out-of-the-box +// artifact this spec never authors. +const { test } = require('node:test'); +const assert = require('node:assert'); +const { resolveSurfaces, declaredSurfaces, unresolvedSurfaceMessage } = require('../lib/surface-resolver.js'); +const { lintAppSpec } = require('../lib/spec-lint.js'); +const { validateAppSpec } = require('../lib/app-spec.js'); + +const base = () => ({ + schemaVersion: 2, + solution: { uniqueName: 'S', publisherPrefix: 'new' }, + app: { name: 'Field Service' }, + entities: [{ + schemaName: 'new_workorder', + displayName: 'Work Order', + primaryAttribute: { schemaName: 'new_name', displayName: 'Name' }, + columns: [{ schemaName: 'new_status', displayName: 'Status', type: 'Text' }], + }], + views: [{ entity: 'new_workorder', name: 'My Open Work Orders', columns: ['new_name'] }], + forms: [{ entity: 'new_workorder', type: 'main', name: 'Work Order' }], + pages: [{ key: 'overview', name: 'Overview', purpose: 'KPIs', source: { kind: 'tsx', codeFile: 'overview.tsx' } }], + dashboards: [], + appShell: { areas: [{ label: 'Main', groups: [{ label: 'Work', subAreas: [{ entity: 'new_workorder', title: 'Work Orders' }, { page: 'overview', title: 'Overview' }] }] }] }, + personas: [], +}); + +const withJob = (surfaces) => { + const s = base(); + s.personas = [{ persona: 'Technician', jobs: [{ name: 'Complete work orders', surfaces, privileges: [{ entity: 'new_workorder', access: ['read', 'write'] }] }] }]; + return s; +}; + +test('declaredSurfaces flattens every (persona, job, surface) triple', () => { + const spec = withJob(['My Open Work Orders', 'Work Order']); + assert.deepStrictEqual(declaredSurfaces(spec), [ + { persona: 'Technician', job: 'Complete work orders', surface: 'My Open Work Orders' }, + { persona: 'Technician', job: 'Complete work orders', surface: 'Work Order' }, + ]); +}); + +test('a surface resolves against every artifact kind an author might name', () => { + for (const [surface, kind] of [ + ['My Open Work Orders', 'view'], + ['Overview', 'page'], // page by display name + ['overview', 'page'], // page by stable key + ['Work Order', 'form'], // also matches the entity display name — see the multi-match test + ['Work Orders', 'subarea'], // the NAV entry is a legitimate answer to "what lets them do the job" + ['new_workorder', 'entity'], + ]) { + const r = resolveSurfaces(withJob([surface])); + assert.deepStrictEqual(r.unresolved, [], `${surface} should resolve`); + assert.ok(r.resolved[0].matches.some((m) => m.kind === kind), `${surface} should match kind ${kind}, got ${JSON.stringify(r.resolved[0].matches)}`); + } +}); + +// Precedence is deliberately NOT invented: a name that legitimately identifies two artifacts reports +// both, so a human sees the ambiguity instead of the resolver silently picking one. +test('a name matching several artifacts reports every match rather than picking one', () => { + const r = resolveSurfaces(withJob(['Work Order'])); + const kinds = r.resolved[0].matches.map((m) => m.kind).sort(); + assert.deepStrictEqual(kinds, ['entity', 'form']); +}); + +test('matching is case- and whitespace-insensitive', () => { + const r = resolveSurfaces(withJob([' mY oPeN wOrK oRdErS '])); + assert.deepStrictEqual(r.unresolved, []); + assert.strictEqual(r.resolved[0].matches[0].kind, 'view'); +}); + +// THE case this check exists for. +test('a surface naming nothing in the spec is reported as unresolved', () => { + const r = resolveSurfaces(withJob(['Overdue Escalations'])); + assert.deepStrictEqual(r.resolved, []); + assert.strictEqual(r.unresolved.length, 1); + assert.strictEqual(r.unresolved[0].surface, 'Overdue Escalations'); + assert.match(unresolvedSurfaceMessage(r.unresolved[0]), /surface "Overdue Escalations" does not match/); +}); + +test('a job with no surfaces[] contributes nothing (spec-lint already warns about it)', () => { + const spec = base(); + spec.personas = [{ persona: 'Tech', jobs: [{ name: 'j', privileges: [{ entity: 'new_workorder', access: ['read'] }] }] }]; + assert.deepStrictEqual(resolveSurfaces(spec), { resolved: [], unresolved: [] }); +}); + +test('resolveSurfaces tolerates a malformed spec without throwing', () => { + for (const bad of [null, undefined, 'x', 42, []]) { + assert.doesNotThrow(() => resolveSurfaces(bad)); + } + assert.deepStrictEqual(resolveSurfaces({ personas: [null, { jobs: [null, { surfaces: [null, '', 'x'] }] }] }).unresolved.length, 1); +}); + +// --- wiring ------------------------------------------------------------------------------------- + +test('spec-lint WARNS on an unresolved surface and does not block the spec', () => { + const spec = withJob(['Overdue Escalations']); + const r = lintAppSpec(spec); + assert.strictEqual(r.errors.filter((e) => /Overdue Escalations/.test(e)).length, 0, 'must not be a blocking error'); + assert.strictEqual(r.warnings.filter((w) => /surface "Overdue Escalations" does not match/.test(w)).length, 1); +}); + +// An out-of-the-box artifact is the documented reason validation is loose; it must keep validating. +test('a spec naming an OOB surface still VALIDATES (warning only)', () => { + const spec = withJob(['Active Accounts']); + assert.strictEqual(validateAppSpec(spec).ok, true, 'an unresolved surface is never a hard error'); + assert.ok(lintAppSpec(spec).warnings.some((w) => /Active Accounts/.test(w))); +}); + +test('spec-lint stays silent when every surface resolves', () => { + const r = lintAppSpec(withJob(['My Open Work Orders', 'Overview'])); + assert.deepStrictEqual(r.warnings.filter((w) => /does not match any view/.test(w)), []); +}); + +// --- verifySpec job-surface rollup --------------------------------------------------------------- +// A PURE rollup over checks already computed: it translates a technical failure ("view X missing") +// into the business impact it caused ("Technician cannot Complete work orders"). +const { verifySpec } = require('../lib/verify-spec.js'); + +const reader = (present) => ({ + findTable: async () => ({ LogicalName: 'new_workorder' }), + findColumns: async () => [{ logicalName: 'new_status' }], + // The view exists only when `present`; that is the failure the rollup must attribute to the job. + queryRecords: async (set) => (set === 'savedquery' && present ? [{ savedqueryid: 'v1' }] : []), + sitemapXml: async () => '', +}); + +test('a job whose surface failed to deploy is reported as a job-surface failure', async () => { + const spec = withJob(['My Open Work Orders']); + const r = await verifySpec(spec, reader(false)); + const c = r.checks.find((x) => x.kind === 'job-surface'); + assert.ok(c && !c.present, JSON.stringify(r.checks.filter((x) => x.kind === 'job-surface'))); + assert.strictEqual(c.name, 'Technician → Complete work orders'); + assert.match(c.detail, /surface "My Open Work Orders" is not deployed/); +}); + +test('no job-surface check is emitted when every surface deployed', async () => { + const r = await verifySpec(withJob(['My Open Work Orders']), reader(true)); + assert.deepStrictEqual(r.checks.filter((x) => x.kind === 'job-surface'), []); +}); + +// An unresolved surface may name an out-of-the-box artifact, so it must NOT become a deploy-time +// failure — spec-lint already warns about it at authoring time. +test('an UNRESOLVED surface does not fail verify', async () => { + const r = await verifySpec(withJob(['Active Accounts']), reader(true)); + assert.deepStrictEqual(r.checks.filter((x) => x.kind === 'job-surface'), []); +}); diff --git a/plugins/model-apps/scripts/tests/verify-model-app.test.js b/plugins/model-apps/scripts/tests/verify-model-app.test.js index 1d7404017..7b60f5d06 100644 --- a/plugins/model-apps/scripts/tests/verify-model-app.test.js +++ b/plugins/model-apps/scripts/tests/verify-model-app.test.js @@ -279,6 +279,14 @@ function loadVerifyCli({ parseResult, validateResult = { ok: true }, verifyResul }, }; } + // Fallback for any require the intercepts above do not cover. Relative ids must resolve against + // the SCRIPT's directory, not this test file's — the sandbox runs verify-model-app.js source, so + // `./lib/x.js` means `scripts/lib/x.js`. Resolving it here (rather than adding an intercept per + // module) means a new `./lib/...` import in the script does not silently break every CLI test + // with "Cannot find module". + if (id.startsWith('./') || id.startsWith('../')) { + return require(path.resolve(path.dirname(scriptPath), id)); + } return require(id); }; customRequire.main = invokeAsMain ? mod : {}; @@ -399,3 +407,98 @@ test('verify CLI entrypoint converts SDK startup errors into emitResult failures assert.strictEqual(emitted.ok, false); assert.match(emitted.payload.message, /SDK unavailable/); }); + +// ── entityPrivileges reader ────────────────────────────────────────────────────────────────────── +// These tests exist because the original role-privileges tests injected a FAKE `entityPrivileges` +// into verifySpec, which meant the real reader was never executed by any test. It shipped building a +// RELATIVE url (`/EntityDefinitions(...)`). `createAzHttpClient` is the raw transport the SDK drives, +// so it takes FULL request urls and validates them with `new URL(url)` for its same-origin guard — a +// relative path throws there. verify-spec catches that per entity, so the failure was silent: every +// entity would have reported "privileges unreadable" and the check would have failed on every live +// run. A reader is only as tested as its narrowest untested seam. + +test('entityPrivileges builds an ABSOLUTE url with the /api/data prefix', async () => { + const seen = []; + const httpClient = { get: async (url) => { seen.push(url); return { status: 200, body: { Privileges: [{ PrivilegeId: 'p1', Name: 'prvReadAccount' }] } }; } }; + const read = readerFor(stubSdk(), 'app', { httpClient, envUrl: 'https://contoso.crm.dynamics.com' }); + + const privs = await read.entityPrivileges('Account'); + + assert.strictEqual(seen.length, 1); + assert.strictEqual( + seen[0], + "https://contoso.crm.dynamics.com/api/data/v9.2/EntityDefinitions(LogicalName='account')?$select=LogicalName,Privileges", + ); + assert.deepStrictEqual(privs, [{ PrivilegeId: 'p1', Name: 'prvReadAccount' }]); +}); + +test('entityPrivileges survives the REAL createAzHttpClient same-origin guard', async () => { + // The regression test that matters: drive the reader through the actual transport rather than a + // hand-written stub, so the url has to satisfy the same guard it failed in production. + const { createAzHttpClient } = require('../lib/sdk-http-client.js'); + const requests = []; + const request = async (o) => { + requests.push(o); + return { statusCode: 200, headers: {}, body: JSON.stringify({ Privileges: [{ PrivilegeId: 'p9' }] }) }; + }; + const httpClient = createAzHttpClient('https://contoso.crm.dynamics.com', { getToken: () => 'TOK', request }); + const read = readerFor(stubSdk(), 'app', { httpClient, envUrl: 'https://contoso.crm.dynamics.com' }); + + const privs = await read.entityPrivileges('co_ticket'); + + assert.deepStrictEqual(privs, [{ PrivilegeId: 'p9' }]); + assert.strictEqual(requests.length, 1, 'the request must reach the transport, not be rejected by the guard'); +}); + +test('entityPrivileges tolerates a trailing slash on the org url', async () => { + const seen = []; + const httpClient = { get: async (url) => { seen.push(url); return { status: 200, body: { Privileges: [] } }; } }; + const read = readerFor(stubSdk(), 'app', { httpClient, envUrl: 'https://contoso.crm.dynamics.com/' }); + await read.entityPrivileges('account'); + assert.ok(!seen[0].includes('.com//api'), `double slash in ${seen[0]}`); +}); + +test('entityPrivileges escapes a quote in the logical name', async () => { + // odataLit doubles a single quote; without it the OData path literal would be malformed. + const seen = []; + const httpClient = { get: async (url) => { seen.push(url); return { status: 200, body: { Privileges: [] } }; } }; + const read = readerFor(stubSdk(), 'app', { httpClient, envUrl: 'https://contoso.crm.dynamics.com' }); + await read.entityPrivileges("o'brien"); + assert.match(seen[0], /LogicalName='o''brien'/); +}); + +test('entityPrivileges returns null on a non-2xx rather than an empty grant', async () => { + // null is the fail-closed signal: verify-spec reports it as a finding. An empty array would read + // as "this table exposes no privileges" and could pass the subset comparison vacuously. + const httpClient = { get: async () => ({ status: 404, body: {} }) }; + const read = readerFor(stubSdk(), 'app', { httpClient, envUrl: 'https://contoso.crm.dynamics.com' }); + assert.strictEqual(await read.entityPrivileges('account'), null); +}); + +test('entityPrivileges is ABSENT (not broken) when the client or org url is missing', async () => { + // verify-spec skips role-privileges unless both readers are functions, so an unwired reader must + // not exist at all. A present-but-throwing reader would report a false failure on every entity. + assert.strictEqual(typeof readerFor(stubSdk(), 'app', {}).entityPrivileges, 'undefined'); + assert.strictEqual(typeof readerFor(stubSdk(), 'app', { httpClient: { get: async () => ({}) } }).entityPrivileges, 'undefined'); + assert.strictEqual(typeof readerFor(stubSdk(), 'app', { envUrl: 'https://contoso.crm.dynamics.com' }).entityPrivileges, 'undefined'); +}); + +test('rolePrivileges paginates and never caps with top', async () => { + // Found by a LIVE run: with the previous op: 5000 a System Administrator role returned + // EXACTLY 5000 rows -- silently truncated. Paginated it returns 7119, so 2119 privileges were + // being dropped. A truncated page is the worst shape for this check: a declared privilege that + // fell off the end reads as NOT HELD, so verify reports a correctly configured role as missing. + // Dataverse honors as a hard cap and omits @odata.nextLink, and the SDK rejects + // paginate+top, so asserting the ABSENCE of top matters as much as the presence of paginate. + const calls = []; + const sdk = stubSdk(); + sdk.queryRecords = async (set, options) => { calls.push({ set, options }); return []; }; + const read = readerFor(sdk, 'app', { httpClient: { get: async () => ({ status: 200, body: {} }) }, envUrl: 'https://contoso.crm.dynamics.com' }); + + await read.rolePrivileges('00000000-0000-0000-0000-000000000001'); + + const q = calls.find((c) => c.set === 'roleprivileges'); + assert.ok(q, 'expected a roleprivileges query'); + assert.strictEqual(q.options.paginate, true, 'must follow @odata.nextLink to completion'); + assert.strictEqual('top' in q.options, false, 'must NOT cap with top -- Dataverse treats it as a hard cap and drops nextLink'); +}); diff --git a/plugins/model-apps/scripts/verify-model-app.js b/plugins/model-apps/scripts/verify-model-app.js index 13b60e51f..a0ce490dd 100644 --- a/plugins/model-apps/scripts/verify-model-app.js +++ b/plugins/model-apps/scripts/verify-model-app.js @@ -17,6 +17,7 @@ const { appUniqueName } = require('./lib/sdk-build.js'); const { validateAppSpec, migrateAppSpec } = require('./lib/app-spec.js'); const { odataLit } = require('./lib/odata.js'); const { makeGenpageCli } = require('./lib/genpage-cli.js'); +const { depthFromMask } = require('./lib/role-privileges.js'); function makeProvision(env, workspaceDir) { const { createMakerSdk } = require('./vendor/cds-maker-sdk.cjs'); @@ -24,7 +25,11 @@ function makeProvision(env, workspaceDir) { fs.mkdirSync(workspaceDir, { recursive: true }); const sdk = createMakerSdk({ workspacePath: workspaceDir, instanceUrl: env, httpClient }); sdk.initWorkspace(); - return sdk; + // The httpClient is returned alongside the SDK because one read has no SDK surface: the role + // privilege check needs `EntityDefinitions(...)?$select=Privileges`, and `fetchEntityMetadata` + // projects that field away (see the `entityPrivileges` reader below). The caller must also pass + // the org URL to `readerFor` — this client takes FULL absolute request URLs, not paths. + return { sdk, httpClient }; } // Resolve the app's sitemap XML: appmodule (by unique name) -> appmodulecomponents (type 62) -> @@ -89,6 +94,35 @@ function readerFor(sdk, appUnique, opts) { const items = await sdk.resolveArtifact('command', { entity: String(entity).toLowerCase() }); return !!(items && items[0] && items[0].id); }, + // rolePrivileges(roleId): what the deployed role actually GRANTS, as [{ privilegeId, depth }]. + // The `roleprivileges` intersect row carries `privilegedepthmask` — a BITMASK (1 Basic / + // 2 Local / 4 Deep / 8 Global), which is NOT the same encoding as the `Depth` name the SDK + // writes via ReplacePrivilegesRole, so it is translated here into the depth NAME the pure + // comparison speaks in. A role with no privileges legitimately returns []; a READ FAILURE + // must propagate (verify-spec turns a non-array into a fail-closed finding) rather than look + // like an empty grant. + // See: https://learn.microsoft.com/en-us/power-apps/developer/data-platform/reference/entities/roleprivileges + rolePrivileges: async (roleId) => { + const rows = await sdk.queryRecords('roleprivileges', { + select: ['privilegeid', 'privilegedepthmask'], + filter: `roleid eq ${roleId}`, + // Follow @odata.nextLink to completion rather than capping with `top`. Verified live: a + // System Administrator role returned EXACTLY 5000 rows against the previous `top: 5000`, + // i.e. it was silently truncated at the cap. A truncated page is the worst shape for this + // check — a declared privilege that simply fell off the end reads as NOT HELD, so verify + // reports a correctly configured role as missing privileges. Not combined with `top`: + // Dataverse honors $top as a hard cap and omits @odata.nextLink, and the SDK rejects + // paginate+top for exactly that reason. + paginate: true, + }); + return (rows || []).map((r) => ({ + privilegeId: String((r && r.privilegeid) || ''), + // `privilegedepthmask` is a BITMASK and can carry several bits at once (7, 15, …), so the + // depth is the HIGHEST bit set — see depthFromMask in lib/role-privileges.js for why an + // exact-match lookup made correctly configured roles report as too shallow. + depth: depthFromMask(r && r.privilegedepthmask), + })); + }, // retrieveSetting(name, { appUniqueName }): the EFFECTIVE app-scoped value of a Dataverse setting, // for the AI app-feature reconcile. Wired here (not in the pure core) so the reader stays injectable // and existence-only callers skip the check entirely. Errors PROPAGATE: verifySpec catches them per @@ -100,6 +134,52 @@ function readerFor(sdk, appUnique, opts) { sitemapXml: async () => { const r = await memoSitemap(); return r.ok ? r.xml : ''; }, }; + // entityPrivileges(logical): the privilege set a table exposes, as [{ PrivilegeId, PrivilegeType, ... }]. + // Read from the SAME source the SDK resolves against when it WRITES the role — + // `EntityDefinitions(LogicalName='x')?$select=Privileges` — so the comparison cannot disagree with + // the write about which PrivilegeId means "Read on account". + // + // Deliberately NOT `sdk.fetchEntityMetadata`: that returns a PROJECTED, camelCased shape + // ({logicalName, displayName, entitySetName, attributes, relationships}) which drops `Privileges` + // entirely, so routing through it would silently report every privilege as unreadable. + // + // The projection's omission is PERMANENT by design — its `$select` never asks for `Privileges`, + // and the SDK pins that with a guardrail test asserting the projection must not surface them even + // when the server returns them. That projection is disk-cached and documents its enrichments as + // best-effort, which is the wrong contract for a security read. Verified against the vendored + // bundle: feeding it a response that DOES carry `Privileges` still yields a projection without them. + // + // TODO: the SDK is gaining a dedicated `getEntityPrivileges(logicalName)` — the privilege READ it + // previously lacked (it could create/delete roles but never read what a table exposes). Switch to + // it once the vendored bundle carries it, and drop this raw read. Note the SDK returns camelCased + // `{ name, privilegeId, privilegeType }` and throws when a table exposes none, where this returns + // PascalCase rows and `null`; `compareRolePrivileges` reads the PascalCase shape today, and + // verify-spec already treats a throw as a per-entity finding, so the swap is a mapping change plus + // a bundle bump — not a behaviour change. + // + // The URL must be ABSOLUTE and carry the `/api/data/v9.2` prefix. `createAzHttpClient` is the raw + // transport the SDK drives, so it receives full request URLs and enforces a same-origin check by + // parsing the argument with `new URL(url)` — a relative path throws there ("Refusing to send the + // Dataverse token to a non-absolute URL") rather than resolving against the org. That failure is + // caught per-entity in verify-spec, so a relative URL would not crash: it would silently report + // EVERY entity's privileges as unreadable and fail the role-privileges check on every live run. + // + // Wired only when BOTH the org URL and the raw client are available. When they are not, the reader + // is ABSENT rather than broken, which makes verify-spec skip the role-privileges check entirely + // (it requires both `rolePrivileges` and `entityPrivileges` to be functions) instead of reporting a + // false failure. + if (opts.httpClient && opts.envUrl) { + const apiRoot = `${String(opts.envUrl).replace(/\/+$/, '')}/api/data/v9.2`; + // Returns null on any non-2xx — verify-spec turns that into a finding rather than a pass. + base.entityPrivileges = async (logical) => { + const name = String(logical).toLowerCase(); + const url = `${apiRoot}/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; + }; + } + // Only expose page-authority readers when a genpageCli is wired — absent it, verifySpec fails closed // for a page-bearing spec (Imp7/C6: missing methods → unableToRun). if (genpageCli) { @@ -179,9 +259,9 @@ async function main() { const v = validateAppSpec(spec, { profile: 'deploy' }); if (!v.ok) { emitResult(false, { ok: false, errors: v.errors }); return; } const workspaceDir = workspaceArg || path.join(path.dirname(specPath), '.maker-workspace'); - const sdk = makeProvision(env, workspaceDir); + const { sdk, httpClient } = makeProvision(env, workspaceDir); const genpageCli = makeGenpageCli(env); - const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir })); + const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient, envUrl: env })); // Show `detail` on a failing check. Without it a READ that failed (throttling, auth expiry, a 5xx) // is indistinguishable from an artifact that is genuinely absent — verifySpec records the cause // but the operator saw only "✗ view: Active Orders" and would chase a phantom deployment drift. diff --git a/scripts/tests/validate-no-real-environments.test.js b/scripts/tests/validate-no-real-environments.test.js new file mode 100644 index 000000000..73d415017 --- /dev/null +++ b/scripts/tests/validate-no-real-environments.test.js @@ -0,0 +1,189 @@ +// Guards the guard. The point of validate-no-real-environments.js is to fail when a +// real environment identifier is committed to this PUBLIC repository, so the tests +// that matter most are the ones asserting it does NOT stay silent on real inputs. +// Every "real" case below is an identifier that was genuinely committed to this repo +// and removed, or a shape the Dataverse service actually generates. + +const test = require('node:test'); +const assert = require('node:assert'); + +const { + isPlaceholder, + isPlaceholderEmailDomain, + scanText, +} = require('../validate-no-real-environments.js'); + +test('placeholder subdomains are accepted', () => { + for (const value of [ + 'contoso', + 'contoso-dev', + 'contosobapenv0001', + 'fabrikamenv001', + 'example', + 'test', + 'your-env', + 'org', + 'org1', + 'ORG2', + 'other', + 'x', + ]) { + assert.equal(isPlaceholder(value), true, `${value} should be treated as a placeholder`); + } +}); + +test('real environment names are rejected', () => { + for (const value of [ + // Realistic non-placeholder names (invented, not values that were ever committed here). + 'zenithbapenv77c1', + 'zenithbapenv22d4', + 'qtxbapenv31a9', + // Shapes seen elsewhere in the repo / generated by Dataverse. + 'org1e98cc97', + 'pascalepipelineshost', + '1841communityv2fresh', + ]) { + assert.equal(isPlaceholder(value), false, `${value} should be rejected`); + } +}); + +test('the org<8hex> shape is rejected even though it starts with the allowed word "org"', () => { + // Regression guard for the ordering inside isPlaceholder: the hex-shape check must + // run BEFORE the placeholder-root and length checks, otherwise a real auto-generated + // Dataverse org is rescued by the generic "org" allowance. + assert.equal(isPlaceholder('org1e98cc97'), false); + assert.equal(isPlaceholder('orgc4f78248'), false); + // ...while short generic org stand-ins still pass. + assert.equal(isPlaceholder('org123'), true); +}); + +test('a real Dataverse host in text is reported', () => { + const violations = scanText('- URL: https://zenithbapenv77c1.crmtest.dynamics.com/'); + assert.ok(violations.length > 0, 'expected at least one violation'); + assert.ok( + violations.some((v) => /non-placeholder Dataverse host/.test(v.detail)), + `expected a non-placeholder violation, got ${JSON.stringify(violations)}`, + ); +}); + +test('a real tenant in a UPN is reported', () => { + const violations = scanText('user: someone@northwindcorp42.onmicrosoft.com'); + assert.ok( + violations.some((v) => /non-placeholder tenant/.test(v.detail)), + `expected a non-placeholder violation, got ${JSON.stringify(violations)}`, + ); +}); + +test('a previously-unseen real org host is reported by shape alone', () => { + // Caught by the shape rule, which is now the only mechanism — there is no denylist. + // rule. Without that rule the guard would only ever re-catch yesterday's leak. + const violations = scanText('envUrl: https://org4a2942d9.crm17.dynamics.com'); + assert.equal(violations.length, 1, JSON.stringify(violations)); + assert.match(violations[0].detail, /non-placeholder Dataverse host/); +}); + +test('placeholder-only content produces no violations', () => { + const clean = [ + 'https://contoso.crm.dynamics.com', + 'maker@contoso.onmicrosoft.com', + 'https://contosobapenv0001.crmtest.dynamics.com/', + 'tester@fabrikamtenant01.onmicrosoft.com', + 'https://org.crm.dynamics.com', + ].join('\n'); + assert.deepEqual(scanText(clean), []); +}); + +test('violations carry the 1-based line number', () => { + const violations = scanText(['clean line', '', 'https://org9cf0ed45.crm.dynamics.com'].join('\n')); + assert.equal(violations.length, 1); + assert.equal(violations[0].line, 3); +}); + +test('CRLF content is scanned with correct line numbers', () => { + // Plugin scripts are CRLF while eval fixtures are LF; a split on "\n" alone would + // leave a trailing "\r" and could shift or mangle reported lines. + const violations = scanText('clean\r\nhttps://org5fbe4359.crm5.dynamics.com\r\n'); + assert.equal(violations.length, 1); + assert.equal(violations[0].line, 2); +}); + +test('a real environment whose name STARTS with a generic word is rejected', () => { + // Review finding: PLACEHOLDER_ROOTS is matched as a prefix, so generic English words like + // `test`/`demo`/`sample` waved through an unbounded family of names — and environments genuinely + // called `TestEnv01` or `demo-prod-01` are common. A false negative here is a leak, which is the + // failure direction that matters, so those roots were removed. + for (const value of [ + 'testenv12345', + 'TestEnv01', + 'demo-prod-01', + 'demoorg9931', + 'sampleorg99', + 'my-real-tenant', + 'samplecorp-prod', + ]) { + assert.equal(isPlaceholder(value), false, `${value} must NOT be treated as a placeholder`); + } +}); + +test('bare generic stand-ins still pass, via the length rule rather than a prefix', () => { + // These are what the in-tree fixtures actually use; they are short enough that the length ceiling + // covers them without needing a prefix entry that would also cover `testenv12345`. + for (const value of ['test', 'demo', 'dev', 'stg', 'uat', 'x']) { + assert.equal(isPlaceholder(value), true, `${value} should still be accepted`); + } +}); + +test('the fictional brands still match as prefixes', () => { + // Contoso/Fabrikam are Microsoft's documented sample organizations, so a name built on them reads + // as obviously fake even when long. + for (const value of ['contosobapenv0001', 'fabrikamtenant01', 'exampleorg', 'your-env']) { + assert.equal(isPlaceholder(value), true, `${value} should be accepted`); + } +}); + +test('a real-looking tenant beginning with a generic word is reported', () => { + const violations = scanText('user: someone@testtenant0042.onmicrosoft.com'); + assert.equal(violations.length, 1, JSON.stringify(violations)); + assert.match(violations[0].detail, /non-placeholder tenant/); +}); + +// -- e-mail rule --------------------------------------------------------------------------------- +// Added after review found a real employee work UPN surviving in three committed fixtures. A +// hostname-only scrub misses it precisely because an address does not look like infrastructure. + +test('a real employee e-mail domain is reported', () => { + const violations = scanText('[1] UNIVERSAL someone@microsoft.com Public OperatingSystem'); + assert.equal(violations.length, 1, JSON.stringify(violations)); + assert.match(violations[0].detail, /real-looking e-mail domain/); +}); + +test('placeholder e-mail domains are accepted', () => { + for (const line of [ + 'maker@contoso.onmicrosoft.com', + 'tester@contoso.com', + 'user@fabrikam.com', + 'someone@example.org', + 'your.email@company.com', + ]) { + assert.deepEqual(scanText(line), [], line); + } +}); + +test('OData annotations are not mistaken for e-mail addresses', () => { + // These parse as addresses but are payload syntax; flagging them would make the guard unusable + // against the eval fixtures, which are full of them. + const odata = [ + '_ownerid_value@OData.Community.Display.V1.FormattedValue', + 'publisherid@odata.bind', + 'IconWebResourceId@odata.bind', + ].join('\n'); + assert.deepEqual(scanText(odata), []); +}); + +test('isPlaceholderEmailDomain: real vs illustrative', () => { + assert.equal(isPlaceholderEmailDomain('microsoft.com'), false); + assert.equal(isPlaceholderEmailDomain('gmail.com'), false); + assert.equal(isPlaceholderEmailDomain('contoso.com'), true); + assert.equal(isPlaceholderEmailDomain('contosotest1.onmicrosoft.com'), true); + assert.equal(isPlaceholderEmailDomain('odata.bind'), true); +}); diff --git a/scripts/validate-no-real-environments.js b/scripts/validate-no-real-environments.js new file mode 100644 index 000000000..8395820bd --- /dev/null +++ b/scripts/validate-no-real-environments.js @@ -0,0 +1,227 @@ +#!/usr/bin/env node + +// Fails the build when model-apps source, docs, or eval fixtures reference a REAL +// Dataverse environment, tenant, or user account instead of a placeholder. +// +// WHY THIS EXISTS +// This repository is public (see the "This Repo Is PUBLIC" section in the root +// AGENTS.md). The genpage eval fixtures are *captured* agent transcripts, so they +// faithfully record whatever live environment the eval was run against — including +// `pac auth list` output with the operator's UPN, tenant, and environment URL. That +// is internal infrastructure detail with no value to an external reader, and it +// accumulated to 53 occurrences across 28 files before it was scrubbed. This guard +// exists so the scrub sticks: re-pasting a fresh live transcript is the realistic +// regression path, and it is not something review reliably catches by eye. +// +// WHAT IT CHECKS +// Two independent rules, both scoped to the model-apps plugin and its evals: +// 1. Shape rule — every Dataverse host (`.crm*.dynamics.com`) and every +// `.onmicrosoft.com` must look like a placeholder. +// 2. Token rule — specific identifiers that were previously committed and removed +// are permanently banned, so the exact same environments cannot come back. +// +// KNOWN GAP (deliberate, not an oversight) +// The scan is limited to `plugins/model-apps/**` and `evals/model-apps/**`. Other +// plugins have their own pre-existing references of this class (for example real +// `org<8hex>` orgs cited in power-pages provenance comments). Widening the scan +// today would fail the build on those pre-existing hits, which would either block +// unrelated PRs or force a rushed cross-plugin edit. Widening is a separate change +// that must scrub those plugins first. A guard that silently skipped them while +// claiming repo-wide coverage would be worse than one that states its limits. +// +// `scripts/**` is outside the scan too, and cannot simply be added: this guard's own +// tests must contain non-placeholder hosts and tenants BY CONSTRUCTION — they are the +// negative cases proving rejection works — so scanning them would report the guard +// against itself. Those fixtures are invented values, deliberately NOT the identifiers +// this repo actually scrubbed; committing the real ones as test data (or as a denylist) +// would make this file the durable public copy of exactly what the scrub removed. + +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); + +const REPO_ROOT = path.resolve(__dirname, '..'); + +const SCAN_PATHS = ['plugins/model-apps', 'evals/model-apps']; + +// Subdomain roots that are unambiguously fictional. Microsoft documentation uses +// Contoso/Fabrikam as its standard sample organizations, so they read as obviously +// fake to any external reader. +// https://learn.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/term-collections/fictitious-names +// +// Kept deliberately SHORT. These are matched as PREFIXES, so every entry here waves through an +// unbounded family of names — and a generic English word is exactly the kind of prefix a real +// environment carries. `test`, `demo`, `sample` and `my-` were removed for that reason: +// environments genuinely named `TestEnv01`, `demo-prod-01` or `sampleorg99` are common, and a prefix +// rule would have declared each of them a placeholder and let a live URL through. That is the +// failure direction that matters here — a false negative is a leak, while a false positive is a +// one-line fix by whoever hits it. Bare `test`/`demo` still pass via the length rule below, which is +// what the in-tree fixtures actually use. +const PLACEHOLDER_ROOTS = [ + 'contoso', + 'fabrikam', + 'example', + 'your-', +]; + +// Short generic stand-ins used throughout the unit tests (`org`, `x`, `a`, `b`, +// `stg`, `other`, ...). Real Dataverse environment names are never this short: the +// service auto-generates `org<8 hex>` and human-named envs carry a team or product +// word. A length ceiling is therefore a reliable discriminator, and it is +// intentionally paired with the hex check below so `org1a2b3c4d` cannot slip +// through merely because it starts with the allowed word "org". +const MAX_GENERIC_LENGTH = 6; + +// Dataverse auto-generates organization hostnames as `org` + 8 hex characters, e.g. +// `https://org1a2b3c4d.crm.dynamics.com` (that example is invented — this file must not become a +// durable copy of a real org id). That shape is always a real environment. +// https://learn.microsoft.com/en-us/power-platform/admin/determine-org-id-name +const REAL_ORG_SHAPE = /^org[0-9a-f]{8}$/i; + +// Previously-committed identifiers are deliberately NOT listed here as plaintext. A denylist of +// private values republishes exactly what this guard exists to suppress — the file would become the +// durable copy of every environment and tenant we scrubbed, in the same public repo. It is also +// unnecessary: the shape rules below already reject every one of them, because a real environment +// name is neither a documented fictitious brand nor short enough to be a generic stand-in. + +// Email domains that are obviously illustrative. Anything else in a scanned file is treated as a +// real person's address. A work UPN in a captured transcript is the leak most likely to survive a +// hostname-only scrub, precisely because it does not look like infrastructure — this guard missed +// one until review caught it. +const PLACEHOLDER_EMAIL_DOMAIN_ROOTS = ['contoso', 'fabrikam', 'example', 'company', 'your']; + +// OData annotations parse as e-mail addresses but are not: `_ownerid_value@OData.Community.Display. +// V1.FormattedValue`, `publisherid@odata.bind`. Both begin their "domain" with `odata.`, which is +// not a registrable domain, so skipping them costs no real coverage. +const ODATA_ANNOTATION = /^odata\./i; + +const HOST_RE = /\b([a-z0-9][a-z0-9-]*)\.crm[a-z0-9]*\.dynamics\.com\b/gi; +const TENANT_RE = /\b([a-z0-9][a-z0-9-]*)\.onmicrosoft\.com\b/gi; +const EMAIL_RE = /\b[A-Za-z0-9._%+-]+@([A-Za-z0-9.-]+\.[A-Za-z]{2,})\b/g; + +// `contosotest1.onmicrosoft.com` -> `contosotest1`; `contoso.com` -> `contoso`. +function isPlaceholderEmailDomain(domain) { + const value = String(domain).toLowerCase(); + if (ODATA_ANNOTATION.test(value)) return true; + // `*.onmicrosoft.com` is already judged by TENANT_RE with the same placeholder logic. Returning + // true here means "not the e-mail rule's business", not "safe" — reporting one identifier twice + // would make the operator's output noisier without adding coverage. + if (value.endsWith('.onmicrosoft.com')) return true; + const firstLabel = value.split('.')[0] || ''; + return PLACEHOLDER_EMAIL_DOMAIN_ROOTS.some((root) => firstLabel.startsWith(root)); +} + +function isPlaceholder(subdomain) { + const value = subdomain.toLowerCase(); + // Checked before the placeholder roots so a real `org<8hex>` is never rescued by + // some future root that happens to prefix-match it. + if (REAL_ORG_SHAPE.test(value)) return false; + if (PLACEHOLDER_ROOTS.some((root) => value.startsWith(root))) return true; + return value.length <= MAX_GENERIC_LENGTH; +} + +function listTrackedFiles() { + // `git ls-files` rather than a directory walk so the guard sees exactly what is + // committed — untracked scratch files are the author's business, not the public + // repository's. + const out = execFileSync('git', ['ls-files', '-z', '--', ...SCAN_PATHS], { + cwd: REPO_ROOT, + encoding: 'utf8', + maxBuffer: 32 * 1024 * 1024, + }); + return out.split('\0').filter(Boolean); +} + +function scanText(content) { + const violations = []; + const lines = content.split(/\r?\n/); + lines.forEach((line, index) => { + const lineNo = index + 1; + + for (const [, subdomain] of line.matchAll(HOST_RE)) { + if (!isPlaceholder(subdomain)) { + violations.push({ + line: lineNo, + detail: `non-placeholder Dataverse host "${subdomain}"`, + }); + } + } + + for (const [, tenant] of line.matchAll(TENANT_RE)) { + if (!isPlaceholder(tenant)) { + violations.push({ line: lineNo, detail: `non-placeholder tenant "${tenant}"` }); + } + } + + for (const [, domain] of line.matchAll(EMAIL_RE)) { + if (!isPlaceholderEmailDomain(domain)) { + violations.push({ line: lineNo, detail: `real-looking e-mail domain "${domain}"` }); + } + } + }); + return violations; +} + +function main() { + const violations = []; + + for (const relPath of listTrackedFiles()) { + const absPath = path.join(REPO_ROOT, relPath); + let content; + try { + content = fs.readFileSync(absPath, 'utf8'); + } catch { + // Unreadable or binary content cannot carry a readable identifier. + continue; + } + // A cheap pre-filter: most files contain none of these markers, and skipping them avoids running + // the regexes over the whole plugin tree. `@` covers the e-mail rule — broad, but the files that + // contain no `@` at all are exactly the ones with nothing to find. + const lower = content.toLowerCase(); + if ( + !lower.includes('dynamics.com') && + !lower.includes('onmicrosoft.com') && + !lower.includes('@') + ) { + continue; + } + + for (const v of scanText(content)) { + violations.push({ file: relPath, ...v }); + } + } + + if (violations.length > 0) { + console.error('Real environment / tenant identifiers found in a PUBLIC repository:\n'); + for (const v of violations) { + console.error(` ${v.file}:${v.line} — ${v.detail}`); + } + console.error( + [ + '', + `${violations.length} violation(s).`, + '', + 'This repository is public. Replace live environment URLs, tenants, and user', + 'accounts with placeholders such as https://contoso.crm.dynamics.com or', + 'maker@contoso.onmicrosoft.com. For eval fixtures captured from a live run,', + 'prefer an EQUAL-LENGTH placeholder so fixed-width `pac auth list` tables stay', + 'aligned. For provenance comments ("verified live on X"), generalise the claim', + 'to "a Dataverse test environment" rather than substituting a fake name, which', + 'would make the claim misleading.', + '', + 'See the "This Repo Is PUBLIC" section in the root AGENTS.md.', + ].join('\n'), + ); + process.exit(1); + } + + console.log( + `OK: no real environment identifiers in ${SCAN_PATHS.join(', ')} (scanned tracked files).`, + ); +} + +if (require.main === module) { + main(); +} + +module.exports = { isPlaceholder, isPlaceholderEmailDomain, scanText, PLACEHOLDER_ROOTS };