Skip to content

Commit cb68f86

Browse files
tyaginidhiclaude
andcommitted
plan-alm: fix 5 issues surfaced by an EDM-site end-to-end run
All five confirmed against source (and PAC 2.8.1) during a data-model/EDM site run of plan-alm: 1. getEnvironmentUrl() grepped "Environment URL:" but `pac env who` on PAC 2.8.x prints the URL under "Org URL:" → returned null, so every caller relying on the pac-env-who fallback (verify-alm-prerequisites without --envUrl, the datamodel / solution / permissions validators, and the #204 declarative-site path) silently failed. Match either label. Extracted parseEnvironmentUrl() (pure) + tests. 2. `pac env list --output json` is INVALID on PAC 2.8.1 (env list accepts only --filter), so ENV_LIST pre-fill never worked. New list-environments.js parses the plain `pac env list` table into JSON {displayName, environmentId, environmentUrl, uniqueName, active}; verified against live PAC (237 rows). Updated all call sites (plan-alm, setup-pipeline, ensure-pipelines-host, cicd-pipeline-patterns.md) + AGENTS.md. Pure parseEnvList() + tests. 3. estimate-solution-size.js hardcoded siteType:'code-site', mislabeling every EDM/data-model site. Now resolves via new --siteType arg (plan-alm passes SITE_TYPE from Phase 1) with a local marker fallback, emitting canonical 'code' | 'data-model' | 'unknown'. resolveSiteType() + tests. 4. plan-alm risk rule #9 emitted a spurious "pipeline host resolution did not run" warning whenever a pipeline already exists (rawDiscovery.hostResolution is legitimately null then, per the Phase 1 Step 12 skip rule). Added a PIPELINE_DONE carve-out so the warning only fires for fresh-pipeline projects. 5. NEW env-match guard (plan-alm Phase 1 step 6b, gate plan-alm:1.env-match, warn+prompt): cross-checks `pac env who` against the project's recorded env URL (powerpages.config.json / .solution-manifest.json) and a websiteRecordId existence probe, so discovery can't silently run against the wrong environment. Catalogued in approval-gates.md §6.1; alm-lint 0 findings. 1279 tests pass (+10). Riding the existing 2.6.0 bump on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1df51b3 commit cb68f86

12 files changed

Lines changed: 322 additions & 17 deletions

File tree

plugins/power-pages/AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ Shared lib modules live at `scripts/lib/` and are imported by other scripts via
233233

234234
#### PP Pipelines
235235

236+
- `scripts/lib/list-environments.js`: Enumerates the Dataverse environments the signed-in PAC user can access, as JSON, for `ENV_LIST` pre-fill (plan-alm Phase 1 Step 5, setup-pipeline, ensure-pipelines-host "Other (paste URL)" prompts). **Why it exists:** the skills used to run `pac env list --output json`, which is INVALID on current PAC CLI (verified 2.8.1 — `pac env list` accepts only `--filter` and errors on `--output`), so the JSON pre-fill silently never worked. This helper runs the plain `pac env list` and parses its table (anchored on the env GUID + https URL + unique-name tokens, so display names with spaces survive). `pac admin list --json` was rejected as the source — it's admin-only and tenant-wide, the wrong scope for a per-user pre-fill. Exports `parseEnvList(stdout)` (pure, tested) + `listEnvironments()`. CLI prints a JSON array of `{ displayName, environmentId, environmentUrl, uniqueName, active }`; prints `[]` and exits 0 on any failure (unauthenticated PAC, parse miss) so callers degrade to manual entry. Match envs by `environmentUrl` origin.
236237
- `scripts/lib/discover-pipelines-host.js`: Discovers the tenant-level default Power Platform Pipelines host environment URL by calling `RetrieveSetting('DefaultCustomPipelinesHostEnvForTenant')` on the dev/source environment. Args: `--envUrl`, `--token`, `--userId`. Output: `{ found, hostEnvUrl }`. Exit 0 (including when not found); exit 1 on error.
237238
- `scripts/lib/create-deployment-environment.js`: Creates a `deploymentenvironments` record in the Pipelines host environment using the **unprefixed** field schema (`name`, `environmentid`, `environmenttype`), then polls `validationstatus` until Succeeded (`200000001`) or Failed (`200000002`). Args: `--hostEnvUrl`, `--token`, `--name`, `--bapEnvId`, `--environmentType` (`200000000` Dev / `200000001` Target), `--environmentUrl` (opt, only echoed in output marker). Idempotent: if a record already exists for the same `environmentid`, returns it with `reused: true`. Output: `{ deploymentEnvironmentId, name, bapEnvId, environmentUrl, environmentType, validationStatus, reused }`.
238239
- `scripts/lib/create-deployment-pipeline.js`: Creates a `deploymentpipelines` record, associates the source environment via `$ref` (relative path + `@odata.context`), and creates `deploymentstages` records for each target environment. Args: `--hostEnvUrl`, `--token`, `--pipelineName`, `--description`, `--sourceDeploymentEnvironmentId`, `--stagesJson` (JSON array of `{ name, targetDeploymentEnvironmentId, order }`). Output: `{ pipelineId, pipelineName, stages[] }`.

plugins/power-pages/references/approval-gates.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke
256256
257257
---
258258

259-
### 6.1 `plan-alm` (15 calls; planner)
259+
### 6.1 `plan-alm` (16 calls; planner)
260260

261261
> `plan-alm` is a **planner** — it produces an approved/draft HTML plan and never executes. The execution gates that used to live in Phases 5–8 (deploy-failure, post-deploy activation, manual export/import checkpoint) now belong to the individual ALM skills the user runs afterward; they are catalogued under those skills' sections, not here.
262262
@@ -265,6 +265,7 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke
265265
| `plan-alm:1.deferral` | gate | progress | 1 | `.alm-deferred` marker present — *"Continue with deferral / remove and proceed / cancel"* | `deferral-marker` |
266266
| `plan-alm:1.approve-draft` | gate | plan | 1 (0b) | Existing **Draft** plan found — *"Approve this draft now (no re-plan) / re-plan from scratch / cancel"*. Approve writes status via `set-plan-status.js` and exits | nothing |
267267
| `plan-alm:1.completeness` | gate | progress | 1 | Completeness check found gaps — *"Sync first / plan with gaps / cancel"* | nothing |
268+
| `plan-alm:1.env-match` | gate | progress | 1 (6b) | `pac env who` env ≠ project's (recorded-URL mismatch or `websiteRecordId` not found in connected env) — *"Switch PAC env & re-run / continue against connected env (degraded) / cancel"*. Only fires on a detected mismatch | nothing |
268269
| `plan-alm:2.q1-existing` | gate | plan | 2 (Q1) | `SOLUTION_DONE=true`*"Use existing solution **{name}**?"* | nothing |
269270
| `plan-alm:2.q1-fresh` | gate | plan | 2 (Q1) | `SOLUTION_DONE=false`*"Include solution setup in plan?"* | nothing |
270271
| `plan-alm:2.q1b-split` | gate | plan | 2 (Q1b) | `RECOMMEND_SPLIT=true`*"Follow recommended {strategy} split?"* | nothing |

plugins/power-pages/references/cicd-pipeline-patterns.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,13 +513,13 @@ Accept: application/json
513513

514514
Returns `{ "SettingValue": "{BAP-environment-GUID}" }` or empty/null if no default is configured.
515515

516-
Cross-reference the GUID with `pac env list` output to find the host environment URL:
516+
Cross-reference the GUID with the environment list to find the host environment URL:
517517

518518
```bash
519-
pac env list --output json 2>/dev/null
519+
node "${PLUGIN_ROOT}/scripts/lib/list-environments.js"
520520
```
521521

522-
Match on `EnvironmentId` field. If no match, probe each environment from `pac env list` with:
522+
This emits a JSON array of `{ displayName, environmentId, environmentUrl, uniqueName, active }`. (It parses `pac env list`; the older `pac env list --output json` is invalid on current PAC CLI, which only accepts `--filter` on `env list`.) Match the GUID on the `environmentId` field. If no match, probe each environment from `pac env list` with:
523523

524524
```
525525
GET {envUrl}/api/data/v9.1/deploymentpipelines?$top=0

plugins/power-pages/scripts/lib/estimate-solution-size.js

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ function parseArgs(argv) {
6565
datamodelManifest: null,
6666
solutionId: null,
6767
projectRoot: null,
68+
siteType: null,
6869
};
6970
for (let i = 0; i < args.length; i++) {
7071
if (args[i] === '--envUrl' && args[i + 1]) out.envUrl = args[++i];
@@ -75,10 +76,35 @@ function parseArgs(argv) {
7576
else if (args[i] === '--datamodelManifest' && args[i + 1]) out.datamodelManifest = args[++i];
7677
else if (args[i] === '--solutionId' && args[i + 1]) out.solutionId = args[++i];
7778
else if (args[i] === '--projectRoot' && args[i + 1]) out.projectRoot = args[++i];
79+
else if (args[i] === '--siteType' && args[i + 1]) out.siteType = args[++i];
7880
}
7981
return out;
8082
}
8183

84+
// Resolve the build-axis site type for the estimator's diagnostic `siteType`
85+
// output field. Prefer the caller-supplied value (plan-alm resolves this in
86+
// Phase 1 via detect-project-context.js, the authoritative source), and fall
87+
// back to a lightweight local probe of the same markers documented in CLAUDE.md:
88+
// - `powerpages.config.json` → code / SPA site
89+
// - `.powerpages-site/.portalconfig/` → declarative design-studio (data-model/EDM) site
90+
// Returns the canonical values ('code' | 'data-model') to match
91+
// detect-project-context.js — NOT the old hardcoded 'code-site', which mislabeled
92+
// every EDM/data-model site as a code site. Returns 'unknown' when neither marker
93+
// is present (e.g. running outside a project root).
94+
function resolveSiteType(explicitSiteType, projectRoot) {
95+
if (explicitSiteType) return explicitSiteType;
96+
if (!projectRoot) return 'unknown';
97+
const fs = require('fs');
98+
const path = require('path');
99+
try {
100+
if (fs.existsSync(path.join(projectRoot, 'powerpages.config.json'))) return 'code';
101+
if (fs.existsSync(path.join(projectRoot, '.powerpages-site', '.portalconfig'))) return 'data-model';
102+
} catch {
103+
// Filesystem probe is best-effort — a diagnostic label must never be fatal.
104+
}
105+
return 'unknown';
106+
}
107+
82108
// Page size for paginated OData queries. Dataverse caps `Prefer: odata.maxpagesize`
83109
// at 5000 — requesting more is silently downgraded. Using the cap minimizes
84110
// roundtrips for large sites.
@@ -738,7 +764,7 @@ async function countSolutionMembership(envUrl, solutionId, token, sitePpcIdSet =
738764
};
739765
}
740766

741-
async function estimateSolutionSize({ envUrl, websiteRecordId, token, publisherPrefix, siteName, datamodelManifest, solutionId, projectRoot }) {
767+
async function estimateSolutionSize({ envUrl, websiteRecordId, token, publisherPrefix, siteName, datamodelManifest, solutionId, projectRoot, siteType }) {
742768
if (!envUrl || !websiteRecordId) {
743769
throw new Error('--envUrl and --websiteRecordId are required');
744770
}
@@ -1122,7 +1148,10 @@ async function estimateSolutionSize({ envUrl, websiteRecordId, token, publisherP
11221148
// scope so reviewers can spot the divergence.
11231149
envVarCountTenantWide,
11241150
mediaRatio: Math.round(webMeasure.mediaRatio * 100) / 100,
1125-
siteType: 'code-site',
1151+
// Build-axis label: 'code' | 'data-model' | 'unknown' (was hardcoded
1152+
// 'code-site', which mislabeled every declarative/EDM site). Prefers the
1153+
// caller-supplied --siteType (plan-alm Phase 1), falls back to a local marker probe.
1154+
siteType: resolveSiteType(siteType, projectRoot),
11261155
tables: tables.map((t) => ({ logicalName: t.logicalName, attributeCount: t.attributeCount || 0 })),
11271156
// Dependency edges among the scoped tables ([a,b], lowercased, a<b) — consumed
11281157
// by compute-split-plan.js to cluster related tables into the same solution.
@@ -1183,5 +1212,7 @@ module.exports = {
11831212
classifyPPCs,
11841213
countSolutionMembership,
11851214
isProbablyBundleChunk,
1215+
resolveSiteType,
1216+
parseArgs,
11861217
BYTES_PER,
11871218
};
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#!/usr/bin/env node
2+
'use strict';
3+
4+
// list-environments.js — enumerate the Dataverse environments the signed-in PAC
5+
// user can access, as JSON, for ENV_LIST pre-fill (plan-alm / setup-pipeline /
6+
// ensure-pipelines-host prompt the user with known env URLs).
7+
//
8+
// Why this exists:
9+
// The skills historically ran `pac env list --output json`. That is INVALID on
10+
// current PAC CLI (verified on 2.8.1): `pac env list` accepts only `--filter`
11+
// and errors with "An unknown argument --output was passed", so the JSON
12+
// pre-fill silently never worked. `pac env list` DOES emit a plain table with
13+
// an "Environment URL" column, so this helper runs the plain command and parses
14+
// that table into JSON. (`pac admin list --json` also yields JSON but is
15+
// admin-only and enumerates the WHOLE tenant — wrong scope for a per-user
16+
// pre-fill — so we deliberately parse `pac env list` instead.)
17+
//
18+
// Usage:
19+
// node list-environments.js -> prints JSON array to stdout
20+
//
21+
// Output (JSON array; empty [] when PAC is unauthenticated / the command fails —
22+
// the pre-fill is best-effort and callers degrade gracefully to manual entry):
23+
// [ { "displayName": "...", "environmentId": "...", "environmentUrl": "https://…",
24+
// "uniqueName": "...", "active": true|false }, ... ]
25+
//
26+
// Exit 0 always (callers parse stdout; [] means "no pre-fill available").
27+
28+
const { execSync } = require('child_process');
29+
30+
// Parse the plain `pac env list` table. Pure + exported for unit testing.
31+
// Example real output (PAC 2.8.1) — note the header row, the "Connected as" banner
32+
// line, and that the active env is flagged with `*` in the leading "Active" column:
33+
//
34+
// Connected as admin@contoso.onmicrosoft.com
35+
// Active Display Name Environment ID Environment URL Unique Name
36+
// * Contoso Dev d664a1f5-5c5b-efbf-9cc9-c1923c437109 https://contosodev.crm.dynamics.com/ unq78bd16d6e4baf01189f56045bd003
37+
// Contoso Prod e8ccb697-db78-e2d6-b721-ef23eedbc302 https://contosoprod.crm4.dynamics.com/ unqe4574a3ea1bff01195c56045bd03c
38+
//
39+
// Display names contain spaces and variable padding, so we anchor on the three
40+
// unambiguous tokens that always appear in order — the 36-char environment GUID,
41+
// the https URL, and the trailing unique name — and treat everything before the
42+
// GUID as `[activeMarker] + displayName`.
43+
function parseEnvList(stdout) {
44+
if (!stdout || typeof stdout !== 'string') return [];
45+
const rows = [];
46+
for (const rawLine of stdout.split(/\r?\n/)) {
47+
const line = rawLine.replace(/\s+$/, '');
48+
if (!line.trim()) continue;
49+
// Skip the "Connected as ..." banner and the column header row.
50+
if (/^Connected as\b/i.test(line.trim())) continue;
51+
if (/^Active\s+Display Name\b/i.test(line.trim())) continue;
52+
53+
// prefix = (optional `*` active marker) + display name; then GUID, URL, uniqueName.
54+
const m = line.match(
55+
/^(.*?)\s+([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\s+(https:\/\/\S+)\s+(\S+)\s*$/i,
56+
);
57+
if (!m) continue;
58+
const prefix = m[1];
59+
// The active env is flagged with a leading `*` in the "Active" column.
60+
const active = /^\s*\*/.test(prefix);
61+
const displayName = prefix.replace(/^\s*\*?\s*/, '').trim();
62+
rows.push({
63+
displayName,
64+
environmentId: m[2],
65+
environmentUrl: m[3].replace(/\/+$/, ''),
66+
uniqueName: m[4],
67+
active,
68+
});
69+
}
70+
return rows;
71+
}
72+
73+
function listEnvironments() {
74+
let stdout = '';
75+
try {
76+
stdout = execSync('pac env list', { encoding: 'utf8', timeout: 20000 });
77+
} catch (e) {
78+
// Best-effort: an unauthenticated / failing PAC CLI yields no pre-fill, not an
79+
// error — callers (plan-alm Phase 1, setup-pipeline) fall back to manual entry.
80+
// `pac` writes its table to stdout even on some non-zero exits, so try to parse
81+
// whatever was captured before giving up.
82+
stdout = (e && (e.stdout || '')) || '';
83+
}
84+
return parseEnvList(stdout);
85+
}
86+
87+
if (require.main === module) {
88+
// Never throw to the caller — emit [] on any failure so the consumer always
89+
// receives parseable JSON.
90+
let result = [];
91+
try { result = listEnvironments(); } catch { result = []; }
92+
process.stdout.write(JSON.stringify(result) + '\n');
93+
process.exit(0);
94+
}
95+
96+
module.exports = { parseEnvList, listEnvironments };

plugins/power-pages/scripts/lib/validation-helpers.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,25 @@ function getAuthToken(resourceUrl) {
160160
* Gets the environment URL from `pac env who`.
161161
* @returns {string|null} Environment URL, or null
162162
*/
163+
// Pure parser (exported for unit testing — getEnvironmentUrl() shells out, so the
164+
// regex itself is tested here against raw banner text rather than through execSync).
165+
// PAC CLI labels the environment URL differently across versions / commands:
166+
// `pac env who` on 2.8.x prints it under "Org URL:" (inside "Organization
167+
// Information"); older/other builds emit "Environment URL:". Match EITHER — with
168+
// only the "Environment URL:" form this returned null on 2.8.x and every caller
169+
// relying on the pac-env-who fallback (verify-alm-prerequisites when --envUrl is
170+
// omitted, the datamodel / solution / permissions validators) silently failed.
171+
// Example 2.8.1 line: ` Org URL: https://org4a2942d9.crm17.dynamics.com/`
172+
function parseEnvironmentUrl(whoOutput) {
173+
if (!whoOutput) return null;
174+
const match = whoOutput.match(/(?:Org URL|Environment URL):\s*(https:\/\/[^\s]+)/i);
175+
return match ? match[1].replace(/\/+$/, '') : null;
176+
}
177+
163178
function getEnvironmentUrl() {
164179
try {
165180
const output = execSync('pac env who', { encoding: 'utf8', timeout: 15000 });
166-
const match = output.match(/Environment URL:\s*(https:\/\/[^\s]+)/i);
167-
return match ? match[1].replace(/\/+$/, '') : null;
181+
return parseEnvironmentUrl(output);
168182
} catch {
169183
return null;
170184
}
@@ -326,6 +340,7 @@ module.exports = {
326340
odataGet,
327341
odataGetAll,
328342
getEnvironmentUrl,
343+
parseEnvironmentUrl,
329344
getPacAuthInfo,
330345
CLOUD_TO_API,
331346
CLOUD_TO_SITE_DOMAIN,

plugins/power-pages/scripts/tests/estimate-solution-size.test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,42 @@ test('estimateSolutionSize tableCountScope is "unavailable" with no local signal
789789
assert.equal(result.tableCountScope, 'unavailable');
790790
assert.deepEqual(result.tableRelationships, []);
791791
});
792+
793+
// --- resolveSiteType: correct build-axis label (was hardcoded 'code-site') -----
794+
795+
test('resolveSiteType prefers the explicit caller value (plan-alm Phase 1 detection)', () => {
796+
const { resolveSiteType } = require('../lib/estimate-solution-size');
797+
assert.equal(resolveSiteType('data-model', '/whatever'), 'data-model');
798+
assert.equal(resolveSiteType('code', null), 'code');
799+
});
800+
801+
test('resolveSiteType falls back to local markers: powerpages.config.json => code, .portalconfig => data-model', () => {
802+
const fs = require('fs');
803+
const os = require('os');
804+
const path = require('path');
805+
const { resolveSiteType } = require('../lib/estimate-solution-size');
806+
807+
const codeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'est-stype-code-'));
808+
const edmRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'est-stype-edm-'));
809+
const bareRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'est-stype-bare-'));
810+
try {
811+
fs.writeFileSync(path.join(codeRoot, 'powerpages.config.json'), '{}');
812+
assert.equal(resolveSiteType(null, codeRoot), 'code');
813+
814+
fs.mkdirSync(path.join(edmRoot, '.powerpages-site', '.portalconfig'), { recursive: true });
815+
assert.equal(resolveSiteType(null, edmRoot), 'data-model', 'EDM/declarative site must NOT be mislabeled code');
816+
817+
// No markers and no projectRoot → 'unknown', never a wrong guess.
818+
assert.equal(resolveSiteType(null, bareRoot), 'unknown');
819+
assert.equal(resolveSiteType(null, null), 'unknown');
820+
} finally {
821+
for (const d of [codeRoot, edmRoot, bareRoot]) fs.rmSync(d, { recursive: true, force: true });
822+
}
823+
});
824+
825+
test('parseArgs captures --siteType', () => {
826+
const { parseArgs } = require('../lib/estimate-solution-size');
827+
const a = parseArgs(['node', 'x', '--siteType', 'data-model', '--envUrl', 'https://x']);
828+
assert.equal(a.siteType, 'data-model');
829+
assert.equal(parseArgs(['node', 'x']).siteType, null);
830+
});

0 commit comments

Comments
 (0)