diff --git a/.github/workflows/power-pages-alm-lint.yml b/.github/workflows/power-pages-alm-lint.yml index e8ccbbbf8..31b4f956a 100644 --- a/.github/workflows/power-pages-alm-lint.yml +++ b/.github/workflows/power-pages-alm-lint.yml @@ -1,3 +1,9 @@ +# Deliberately separate from `power-pages-script-tests.yml`. +# This workflow runs policy enforcement (gate markers, ALM-aware-by-default, +# discovery coverage) — pure-Node, OS-independent, completes in seconds, so +# it runs on a single OS to surface red checks fast. +# The sibling workflow runs functional unit tests across ubuntu × windows × macos. +# See PLUGIN_DEVELOPMENT_GUIDE.md and references/approval-gates.md. name: power-pages-alm-lint on: diff --git a/.github/workflows/power-pages-script-tests.yml b/.github/workflows/power-pages-script-tests.yml index 293b7db39..f34734a4a 100644 --- a/.github/workflows/power-pages-script-tests.yml +++ b/.github/workflows/power-pages-script-tests.yml @@ -1,3 +1,8 @@ +# Deliberately separate from `power-pages-alm-lint.yml`. +# This workflow runs functional unit tests across ubuntu × windows × macos to +# catch OS-specific regressions in Node scripts. Slow (~30s on macOS). +# The sibling workflow runs fast policy enforcement on a single OS so red +# checks for gate-marker / catalog drift surface before this matrix finishes. name: power-pages-script-tests on: diff --git a/plugins/power-pages/AGENTS.md b/plugins/power-pages/AGENTS.md index bdd42034f..78f5fe917 100644 --- a/plugins/power-pages/AGENTS.md +++ b/plugins/power-pages/AGENTS.md @@ -257,7 +257,7 @@ Shared reference documents live at `references/` and are referenced by multiple - `solution-api-patterns.md`: OData body templates for publisher POST, solution POST, `AddSolutionComponent`, `ExportSolutionAsync`, `DownloadSolutionExportData`, `ImportSolutionAsync`, `StageSolution`. Also documents `.solution-manifest.json` format. Used by `setup-solution`, `export-solution`, and `import-solution`. - `deployment-error-catalog.md`: Catalog of 10 known deployment failure patterns (stale manifest, blocked JS, missing websiteRecordId, auth expiry, empty build output, solution missing dependencies, solution timeout, PAC CLI not installed, environment mismatch, duplicate component). Each entry includes root cause, severity, auto-fix availability, and fix procedure. Used by `diagnose-deployment`. - `cicd-pipeline-patterns.md`: PAC CLI service principal auth syntax; complete ADO `azure-pipelines.yml` template; complete GitHub Actions `deploy.yml` template; commented solution export/import blocks; secrets/variables setup tables; manual steps that cannot be automated; **Power Platform Pipelines API patterns** (HAR-confirmed): host env discovery via `RetrieveSetting`, `deploymentenvironments` create + `validationstatus` poll, `deploymentpipelines` create, `$ref` associate source (relative path format), `deploymentstages` create, `RetrieveDeploymentPipelineInfo`, stage run create + `ValidatePackageAsync` (204) + `operation` poll, `deploymentsettingsjson` PATCH, `DeployPackageAsync`, `stagerunstatus` terminal values, `docs/alm/last-pipeline.json` and `docs/alm/last-deploy.json` formats. Used by `setup-pipeline` and `deploy-pipeline`. -- `approval-gates.md`: Canonical terminology, marker syntax, and catalog of every user-confirmation point ("Approval Gate") across the ALM skill family. Defines six categories (`intent` / `plan` / `progress` / `consent` / `final` / `pause`), an explicit-pairing marker (`` + human `> 🚦 Gate (...)` block), the `cancel-leaves` vocabulary, and proposed lint rules (`GATE-must-have-marker`, `GATE-id-must-be-unique`, `GATE-must-be-in-catalog`, `GATE-intent-must-call-helper`, `GATE-cancel-leaves-known-vocab`). Currently scoped to the 12 ALM skills (`plan-alm`, `setup-solution`, `setup-pipeline`, `deploy-pipeline`, `export-solution`, `import-solution`, `configure-env-variables`, `ensure-pipelines-host`, `force-link-environment`, `activate-site`, `test-site`, `diagnose-deployment`). **The catalog will be extended to non-ALM skills in a follow-up.** New skills authoring any `AskUserQuestion` block should follow §3 (categories), §4 (marker syntax), and add their gates to §6 (catalog). +- `approval-gates.md`: Canonical terminology, marker syntax, and catalog of every user-confirmation point ("Approval Gate") across the **entire power-pages skill set** (12 ALM + 12 non-ALM). Defines six categories (`intent` / `plan` / `progress` / `consent` / `final` / `pause`), an explicit-pairing marker (`` + human `> 🚦 Gate (...)` block), the `cancel-leaves` vocabulary, and the seven gate-related lint rules enforced by `scripts/lint-skills-alm.js` at hard-fail severity: `GATE-must-have-marker`, `GATE-id-must-be-unique`, `GATE-must-be-in-catalog`, `GATE-intent-must-call-helper`, `GATE-cancel-leaves-known-vocab`, `GATE-prose-block-required` (marker must be followed by a 🚦 prose block within 10 lines, outside any code fence), and `CATALOG-row-must-have-marker` (reverse of `GATE-must-be-in-catalog` — every `kind: gate` catalog row must have a SKILL.md marker). §6.1–§6.12 catalogue the ALM skills; §6.13–§6.24 catalogue the non-ALM skills (`create-site`, `deploy-site`, `add-server-logic`, `add-cloud-flow`, `setup-auth`, `integrate-webapi`, `setup-datamodel`, `add-sample-data`, `add-seo`, `create-webroles`, `audit-permissions`, `integrate-backend`). `report-issue` is excluded because its workflow lives in the cross-plugin shared file. **New skills must extend §6 in the same PR they introduce an `AskUserQuestion`** — lint will block the PR otherwise. Skill-specific reference docs (e.g., `skills/setup-datamodel/references/odata-api-patterns.md`) contain only patterns unique to that skill and point to the shared docs via `${CLAUDE_PLUGIN_ROOT}/references/` paths for common content. @@ -355,7 +355,7 @@ This runs a lightweight check comparing the local plugin version against `origin ### Key Patterns -- **Approval Gates** — Every load-bearing `AskUserQuestion` is an **Approval Gate**. Pause at minimum after gathering requirements, after presenting a plan, after implementation, and before deployment (Three-Point Approval Pattern). For ALM skills, every gate must (a) be catalogued in `references/approval-gates.md` §6 with a stable `gate-id`, and (b) be marked in SKILL.md with the explicit-pairing comment `` followed by a human-readable `> 🚦 **Gate (...)**` block. New ALM skills must extend the catalog in the same PR that introduces the skill. Non-ALM skills should follow the same convention as the catalog is extended in a follow-up; lint runs warn-only on non-ALM skills until then. Do not coin alternative terms ("review gate", "approval checkpoint", "manual step" etc.) — the canonical term is **Approval Gate**. +- **Approval Gates** — Every load-bearing `AskUserQuestion` is an **Approval Gate**. Pause at minimum after gathering requirements, after presenting a plan, after implementation, and before deployment (Three-Point Approval Pattern). **Every skill in this plugin** (ALM and non-ALM alike) must (a) catalogue each gate in `references/approval-gates.md` §6 with a stable `gate-id`, and (b) mark it in SKILL.md with the explicit-pairing comment `` followed by a human-readable `> 🚦 **Gate (...)**` block. Pure data-gathering prompts (free-text fallbacks, configuration sub-prompts) take a `` comment instead. `scripts/lint-skills-alm.js` enforces this at **hard-fail** severity across the whole plugin — there is no warn-only carve-out for any skill class. **When you add a new skill that introduces an `AskUserQuestion`, you must extend `references/approval-gates.md` §6 with the new gate-id(s) in the same PR; CI will block the PR otherwise.** Do not coin alternative terms ("review gate", "approval checkpoint", "manual step" etc.) — the canonical term is **Approval Gate**. - **Deployment prompt** — Skills that modify site artifacts should end by asking "Ready to deploy?" and invoke `/deploy-site` if yes. - **Lifecycle hooks** — Hook registration is centralized in `hooks/hooks.json`; `scripts/lib/powerpages-hook-utils.js` derives tracked skills from `skills/*/SKILL.md` and discovers optional `scripts/validate*.js` validators. Do not define hook registration in individual `SKILL.md` files. - **Graceful failure** — Track API call results, never auto-rollback, report failures clearly, continue with remaining items. diff --git a/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md b/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md index 45400a0ed..792ae51a0 100644 --- a/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md +++ b/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md @@ -270,7 +270,7 @@ Every skill pauses for user approval at three junctures: Between checkpoints, skills work **autonomously** — no mid-analysis questions. -> **Approval Gates — canonical catalog.** Every individual `AskUserQuestion` that meets the gate test (would Cancel leave partial or complete-but-wrong state behind?) is an **Approval Gate**. See `references/approval-gates.md` for the canonical terminology, the six categories (`intent` / `plan` / `progress` / `consent` / `final` / `pause`), the marker syntax (`` + human-readable `> 🚦 **Gate (...)**` block), the per-skill catalog, and the lint rules (`GATE-must-have-marker`, `GATE-id-must-be-unique`, `GATE-must-be-in-catalog`, `GATE-intent-must-call-helper`, `GATE-cancel-leaves-known-vocab`). ALM skills enforce these rules with `severity: 'error'`; non-ALM skills currently warn-only until the catalog extends. +> **Approval Gates — canonical catalog.** Every individual `AskUserQuestion` that meets the gate test (would Cancel leave partial or complete-but-wrong state behind?) is an **Approval Gate**. See `references/approval-gates.md` for the canonical terminology, the six categories (`intent` / `plan` / `progress` / `consent` / `final` / `pause`), the marker syntax (`` + human-readable `> 🚦 **Gate (...)**` block), the per-skill catalog, and the seven gate-related lint rules: `GATE-must-have-marker`, `GATE-id-must-be-unique`, `GATE-must-be-in-catalog`, `GATE-intent-must-call-helper`, `GATE-cancel-leaves-known-vocab`, `GATE-prose-block-required` (marker followed by 🚦 prose block within 10 lines, outside any code fence), and `CATALOG-row-must-have-marker` (every `kind: gate` catalog row must have a corresponding marker in some SKILL.md — the reverse of `GATE-must-be-in-catalog`). **Every skill in this plugin** is enforced at `severity: 'error'` — there is no ALM vs non-ALM carve-out. When you add a new skill that introduces an `AskUserQuestion`, you must extend `references/approval-gates.md` §6 with the new gate-id(s) in the same PR; CI will block otherwise. Data-gathering prompts (free-text fallbacks, configuration sub-prompts) take a `` comment instead. ### Approval in Practice diff --git a/plugins/power-pages/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index c33d141f0..c4dc67c24 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -1,10 +1,12 @@ -# Approval Gates — ALM Skill Catalog (Draft v2) +# Approval Gates — Power Pages Skill Catalog (v3) -> **Status: DRAFT v2.** Addresses review feedback on v1. +> **Status: v3 — extended to non-ALM skills.** v2 introduced the marker/lint design and catalogued the 12 ALM skills (§6.1–§6.12). v3 extends coverage to the 12 non-ALM skills (§6.13–§6.24), flips lint severity from warn-only to hard-fail across the plugin, and updates `AGENTS.md` so any new skill must add its gates here in the same PR. > -> **Scope: ALM skills only.** §6 enumerates every `AskUserQuestion` in the 12 ALM skills (`plan-alm`, `setup-solution`, `setup-pipeline`, `deploy-pipeline`, `export-solution`, `import-solution`, `configure-env-variables`, `ensure-pipelines-host`, `force-link-environment`, `activate-site`, `test-site`, `diagnose-deployment`). Non-ALM skills (`create-site`, `deploy-site`, `add-cloud-flow`, `add-server-logic`, `add-seo`, `add-sample-data`, `audit-permissions`, `create-webroles`, `integrate-backend`, `integrate-webapi`, `setup-auth`, `setup-datamodel`) are intentionally **deferred** — see §10. Catalog completeness is asserted only for ALM. +> **Scope: all power-pages skills.** §6 enumerates every `AskUserQuestion` across the 24 user-invocable skills (12 ALM + 12 non-ALM). `report-issue` is a cross-plugin shared workflow — its wrapper SKILL.md contains no prompts (the workflow file at `shared/skills/report-issue/report-issue-workflow.md` lives outside the per-plugin lint scope) and is excluded from this catalog. > -> **Not yet applied to SKILL.md files.** This document defines terminology + marker + lint design. The follow-up PR will add the markers to each ALM SKILL.md and ship the lint rule. Run the decisions in §9 first. +> **Markers applied across all SKILL.md files.** Each gate has both a machine-readable `` HTML comment and a human-readable `> 🚦 **Gate (...)**` block. Each pure data-gathering prompt has a `` comment. +> +> **Lint is hard-fail for every skill.** `scripts/lint-skills-alm.js` enforces seven gate-related rules at error severity on every SKILL.md under `plugins/power-pages/skills/`: `GATE-must-have-marker`, `GATE-id-must-be-unique`, `GATE-must-be-in-catalog`, `GATE-intent-must-call-helper`, `GATE-cancel-leaves-known-vocab`, `GATE-prose-block-required` (the marker must be followed by a 🚦 prose block within 10 lines, outside any code fence), and `CATALOG-row-must-have-marker` (every `kind: gate` catalog row must have a corresponding marker in some SKILL.md — the reverse of `GATE-must-be-in-catalog`). --- @@ -78,6 +80,8 @@ Each gate fits one of six categories. Each gets a one-word prefix in the marker **Mechanism:** Skill presents a rendered artifact and a 2–4 option `AskUserQuestion`. Cancel exits without any Dataverse / filesystem write. +> **Deploy-dispatch prompts ("Deploy now?") are `plan`, not `consent`.** Many non-ALM skills (`create-site:8.deploy`, `add-server-logic:11.3.deploy`, `add-cloud-flow:8.4.deploy`, etc.) end with a "Deploy now / Later" prompt that invokes `/deploy-site`. These are `plan` gates — the user is choosing *whether to dispatch a child skill*, not approving the destructive action itself. The destructive Dataverse write lives inside `/deploy-site`'s own `consent` gate at `deploy-site:3.confirm-env`, which echoes the target env and requires explicit confirmation. The pattern is dispatch-then-consent: `plan` here, `consent` there. Compare `deploy-site:6.2.unblock-js` (`consent`, modifies tenant-wide `blockedattachments`) — that one IS destructive at its call site and is tagged accordingly. If a future "Deploy now?" prompt skips the dispatch and writes directly to Dataverse, retag it `consent`. + ### 3.3 `progress` — Mid-flow re-confirmation gate **Defining attribute:** A condition emerged mid-run that wasn't visible at planning time; the user re-confirms before the skill continues with the delta. @@ -149,7 +153,7 @@ Every gate gets a structural marker in SKILL.md. The marker has two parts: a **m > 🚦 **Gate (plan · skill-name:phase-id):** One-line summary of what the user is approving. > > **Trigger:** When this gate fires. -> **Blast radius if skipped:** What goes wrong if a tool bypasses the prompt. +> **Why we ask:** What goes wrong if a tool bypasses the prompt. > **Cancel leaves:** Explicit state description — either `nothing` (clean exit) or a specific state. ``` @@ -192,7 +196,7 @@ Custom values are allowed when none of the above fits — lint accepts any kebab > 🚦 **Gate (final · deploy-pipeline:6.0):** Final consent before DeployPackageAsync. > > **Trigger:** Validation passed (Phase 5); no completeness drift outstanding; no env-var override prompts outstanding. About to fire `DeployPackageAsync` or the `pac pipeline deploy` fallback. -> **Blast radius if skipped:** Wrong-stage deploy. Non-transactional — partial failure leaves whatever already imported on the target. +> **Why we ask:** Wrong-stage deploy. Non-transactional — partial failure leaves whatever already imported on the target. > **Cancel leaves:** Validated stage run on host (no `docs/alm/last-deploy.json` written). User can retry by re-invoking `deploy-pipeline`. [arbitrarily long rationale prose explaining why this gate exists, what alternatives were considered, etc.] @@ -264,7 +268,7 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke | `plan-alm:2.q1b-override` | gate | consent | 2 (Q1b) | User picked "keep single" — *"Confirm override + free-text reason"* | nothing | | `plan-alm:2.q2-strategy` | gate | plan | 2 (Q2) | *"PP Pipelines / Manual export-import / Already have pipeline / Help me decide"* | nothing | | `plan-alm:2.q3-stages` | gate | plan | 2 (Q3 PP) | *"How many deployment stages?"* | nothing | -| `plan-alm:2.q4-stage-env` | gate | plan | 2 (Q4 PP per stage) | *"Target env URL for stage {N}?"* | nothing | +| `plan-alm:2.q4-host` | gate | plan | 2 (Q4 PP) | Host environment selection — branched table consuming `HOST_RESOLUTION` status (use-detected / pick from list / NoHost host-type menu / Sandbox confirm / CannotRedirect block / manual paste). Per-stage env URLs are inferred from the Q3 stage-layout answer + `pac env list`, not collected via a separate prompt. | nothing | | `plan-alm:2.q5-approval` | gate | plan | 2 (Q5 PP) | *"Approvals: required each stage / staging auto + prod required / no gates"* | nothing | | `plan-alm:2.q3-manual` | gate | plan | 2 (Q3 Manual) | *"How many target envs?"* | nothing | | `plan-alm:2.q4-manual-target` | gate | plan | 2 (Q4 Manual per stage) | *"URL for target env {N}?"* | nothing | @@ -337,9 +341,8 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke | `deploy-pipeline:7.6.4.strip-secret-values` | gate | consent | 7.6.4 | Reactive Secret-reference validation failure — *"Strip invalid Secret values from `deployment-settings.json` and retry? Yes / No"* | `invalid-secret-in-file` | | `deploy-pipeline:7.7.activate` | gate | plan | 7.7 | Site deployed, not yet activated — *"Activate now / later"* | nothing | | `deploy-pipeline:7.cloud-flow-register` | gate | plan | 7 (cloud-flow path) | Cloud flows in solution — *"Registered in target? Yes / Later"* (informational continue) | nothing | -| `deploy-pipeline:6.1.pac-fallback-consent` | gate | final | 6.1 | `VALIDATE_PACKAGE_UNAVAILABLE=true` path uses `pac pipeline deploy` instead of `DeployPackageAsync` — same shape as `6.0` | `validated-stage-run` | -(Three additional `AskUserQuestion` calls in this skill are sub-prompts inside the gates above — env-var value entry per variable inside `5.env-vars`, validation `Approved? Yes / No` follow-ups inside `4.pending-approval` and `6.pending-approval`. They share the parent gate's marker.) +(Three additional `AskUserQuestion` calls in this skill are sub-prompts inside the gates above — env-var value entry per variable inside `5.env-vars`, validation `Approved? Yes / No` follow-ups inside `4.pending-approval` and `6.pending-approval`. They share the parent gate's marker. The `6.0.final-consent` marker covers both the `DeployPackageAsync` and `pac pipeline deploy` paths — no separate ID for the CLI fallback.) --- @@ -382,8 +385,7 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke |---|---|---|---|---|---| | `configure-env-variables:0.no-plan` | gate | intent | 0 | `check-alm-plan.js` `exists:false` — *"Run plan-alm? / Continue / Cancel"* | nothing | | `configure-env-variables:0.stale-plan` | gate | intent | 0 | `check-alm-plan.js` `stale:true` — *"Refresh / Continue / Cancel"* | nothing | -| `configure-env-variables:2.selection` | gate | plan | 2 | Settings classified — *"Which to promote? Per-stage values per setting"* | nothing | -| `configure-env-variables:6.confirm-matrix` | gate | plan | 6 | `deployment-settings.json` assembled — *"Confirm matrix before write"* | nothing | +| `configure-env-variables:2.selection` | gate | plan | 2 | Settings classified — *"Which to promote? Per-stage values per setting"*. Per-stage values matrix is built inside this same multi-question prompt. | nothing | | `configure-env-variables:6.1.invalid-secret-values` | gate | consent | 6.1 | Pre-write validation found Secret refs in invalid formats — hard-stop, *"Fix or abort"* | nothing | --- @@ -407,15 +409,13 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke --- -### 6.9 `force-link-environment` (5 calls) +### 6.9 `force-link-environment` (3 calls) | ID | Kind | Category | Phase | Trigger / question | Cancel leaves | |---|---|---|---|---|---| -| `force-link-environment:2.host-url` | gate | plan | 2 | Host URL not resolved from markers — *"Pick host"* | nothing | -| `force-link-environment:2.dev-env` | gate | plan | 2 | Dev env BAP GUID not resolved — *"Pick / paste"* | nothing | +| `force-link-environment:2.host-url` | gate | plan | 2 | Host URL not resolved from `--host` arg / `last-host-check.json` / `last-pipeline.json` — *"Pick host (with paste-URL fallback)"* | nothing | +| `force-link-environment:2.dev-env` | gate | plan | 2 | Dev env BAP GUID not resolved from `--dev-env` arg or `pac env who` confirmation — *"Pick / paste"* | nothing | | `force-link-environment:4.destructive` | gate | consent | 4 | Mandatory gate before `ManageEnvironmentStamp` — *"DESTRUCTIVE: confirm cross-host stamp move"* | nothing | -| `force-link-environment:2.host-fallback` | not-a-gate | — | 2 | Free-text host URL — data-gathering | — | -| `force-link-environment:2.dev-fallback` | not-a-gate | — | 2 | Free-text dev env GUID — data-gathering | — | --- @@ -468,27 +468,224 @@ When **removing** a gate, also remove its catalog row in the same PR. --- -## 8. Non-ALM skills — explicitly deferred +### 6.13 `create-site` (5 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `create-site:1.purpose` | gate | plan | 1 | Site purpose unclear — multi-question prompt (site name, framework, purpose, audience, location) | nothing | +| `create-site:3.requirements` | gate | plan | 3 | *"Which features? / Aesthetic / Mood"* — three sub-prompts sharing this gate; shape the rendered Phase 4 plan | nothing | +| `create-site:4.7.plan-approval` | gate | plan | 4.7 | HTML plan rendered — *"Approve and start building / I'd like to make changes"* | nothing | +| `create-site:7.review` | gate | plan | 7 | Live site ready — *"Would you like any changes?"* | nothing | +| `create-site:8.deploy` | gate | plan | 8 | *"Deploy now (Recommended) / Skip for now"* — invokes `/deploy-site` on Yes | nothing | + +--- -Per the v1 review, the catalog was incomplete because it claimed full coverage but only covered ~30% of `AskUserQuestion` calls. v2 fixes this by **scoping to ALM only**. The 13 non-ALM skills below contain ~70 additional `AskUserQuestion` calls that need to be catalogued in a follow-up: +### 6.14 `deploy-site` (8 calls) -| Skill | `AskUserQuestion` count | Status | -|---|---|---| -| `create-site` | 11 | Deferred | -| `deploy-site` | 9 | Deferred | -| `add-server-logic` | 13 | Deferred | -| `add-cloud-flow` | 7 | Deferred | -| `setup-auth` | 5 | Deferred | -| `integrate-webapi` | 6 | Deferred | -| `setup-datamodel` | 3 | Deferred | -| `add-sample-data` | 3 | Deferred | -| `add-seo` | 3 | Deferred | -| `create-webroles` | 3 | Deferred | -| `audit-permissions` | 2 | Deferred | -| `integrate-backend` | (see SKILL.md) | Deferred | -| `report-issue` | 1 | Deferred (cross-plugin, may not need a gate) | +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `deploy-site:2.auth-url` | not-a-gate | — | 2 | Free-text env URL when PAC CLI not authenticated — data-gathering | — | +| `deploy-site:3.confirm-env` | gate | consent | 3 | Echoes current env — *"Deploy to this environment? Yes / No, choose different"*. Covers the follow-up "pick different env" sub-prompt at the same step (single section, paired by marker proximity). Wrong-env deploy is destructive — confirmation is mandatory. | nothing | +| `deploy-site:4.1.multi-project` | gate | plan | 4.1 | Multiple `powerpages.config.json` candidates found — *"Which project to deploy?"* | nothing | +| `deploy-site:4.2.audit-permissions` | gate | plan | 4.2 | Re-deployment detected (`.powerpages-site` exists) — *"Run permissions audit first / Skip"* | nothing | +| `deploy-site:5.5.1.activate` | gate | plan | 5.5.1 | Site not yet activated — *"Activate now / Skip"* | nothing | +| `deploy-site:5.6.restart-cache` | gate | plan | 5.6 | Site activated — *"Restart site to flush cache? (brief downtime) / Skip"* | nothing | +| `deploy-site:6.2.unblock-js` | gate | consent | 6.2 | Upload failed because `.js` is blocked — *"Remove .js block from `blockedattachments`? / No"*. Modifies tenant-wide env setting — destructive shared state. | `attachment-block-modified` | + +--- + +### 6.15 `add-server-logic` (12 calls / 8 gates + 4 sub-prompts) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `add-server-logic:1.5.deploy-first` | gate | plan | 1.5 | `.powerpages-site` missing — *"Deploy now (Required) / Cancel"* — entry condition for the skill | nothing | +| `add-server-logic:2.1.2.use-custom-actions` | gate | plan | 2.1.2 | Custom actions discovered — *"Wrap an existing custom action? Yes / No, build from scratch"* — changes implementation shape | nothing | +| `add-server-logic:2.1.2.per-item-action` | not-a-gate | — | 2.1.2 | Per-item follow-up — *"For `` endpoint, which custom action?"* — data-gathering sub-prompt under the previous gate's Yes path | — | +| `add-server-logic:2.3.1.keyvault` | gate | plan | 2.3.1 | Secrets identified — *"Use Azure Key Vault (Recommended) / Store directly as env var"* — affects the Phase 4 plan and Phase 7 implementation | nothing | +| `add-server-logic:2.4.clarify` | not-a-gate | — | 2.4 | Multi-question clarification when intent is ambiguous — data-gathering | — | +| `add-server-logic:4.4.plan-approval` | gate | plan | 4.4 | HTML plan rendered — *"Approve and implement / Request changes / Cancel"* | nothing | +| `add-server-logic:7.2a.select-vault` | not-a-gate | — | 7.2a | Pick which Key Vault to use — data-gathering sub-prompt under the Phase 2.3.1 Yes path | — | +| `add-server-logic:7.2a.no-vaults` | gate | plan | 7.2a | No Key Vaults found — *"Create new (Recommended) / Fall back to plain env var"* — branches the secret-storage flow | nothing | +| `add-server-logic:7.2a.vault-params` | not-a-gate | — | 7.2a | Vault name / RG / location free-text — data-gathering for the create call | — | +| `add-server-logic:9.1.frontend-scope` | gate | plan | 9.1 | *"Fully integrate into UI (Recommended) / I'll handle frontend myself"* — decides Phase 9 work scope | nothing | +| `add-server-logic:11.3.deploy` | gate | plan | 11.3 | *"Deploy now (Recommended) / Later"* — invokes `/deploy-site` on Yes | nothing | +| `add-server-logic:11.3.test` | gate | plan | 11.3 | After successful deploy — *"Run `/test-site` now / Skip"* | nothing | + +--- + +### 6.16 `add-cloud-flow` (6 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `add-cloud-flow:1.3.deploy-first` | gate | plan | 1.3 | `.powerpages-site` missing — *"Deploy now (Required) / Cancel"* | nothing | +| `add-cloud-flow:3.1.select-flows` | gate | plan | 3.1 | Multi-select over discovered + already-registered flows — *"Which flows to add or integrate?"* | nothing | +| `add-cloud-flow:3.3.scenario-clarify` | not-a-gate | — | 3.3 | Per-flow scenario clarification when flow name/description is ambiguous — data-gathering for Phase 4 role assignment | — | +| `add-cloud-flow:5.3.plan-approval` | gate | plan | 5.3 | HTML plan rendered — *"Approve and implement / Request changes / Cancel"* | nothing | +| `add-cloud-flow:8.4.deploy` | gate | plan | 8.4 | *"Deploy now (Recommended) / Later"* | nothing | +| `add-cloud-flow:8.4.test` | gate | plan | 8.4 | After successful deploy — *"Run `/test-site` to validate flow integration / Skip"* | nothing | + +--- + +### 6.17 `setup-auth` (5 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `setup-auth:1.3.deploy-first` | gate | plan | 1.3 | `.powerpages-site` missing — *"Deploy now (Required) / Later"* | nothing | +| `setup-auth:1.4.create-webroles` | gate | plan | 1.4 | No web roles found — *"Create web roles first (Recommended) / Skip"* | nothing | +| `setup-auth:2.1.requirements` | gate | plan | 2.1 | *"Which auth features? Login+Logout+RBAC / Login+Logout only / RBAC only"* — covers the follow-up "which roles get access" sub-prompt in the same step | nothing | +| `setup-auth:2.2.plan-approval` | gate | plan | 2.2 | *"Approve and proceed / I'd like to make changes"* | nothing | +| `setup-auth:8.4.deploy` | gate | plan | 8.4 | *"Deploy now (Recommended) / Later"* — auth doesn't work until deployed | nothing | + +--- + +### 6.18 `integrate-webapi` (5 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `integrate-webapi:3.2.confirm-tables` | gate | plan | 3.2 | *"Which tables to integrate? All / Let me select / Add more"* | nothing | +| `integrate-webapi:6.1.deploy-first` | gate | plan | 6.1 | `.powerpages-site` missing — *"Deploy now (Recommended) / Skip permissions setup"* | nothing | +| `integrate-webapi:6.2.permissions-source` | gate | plan | 6.2 | *"Upload existing diagram / Let architects figure it out"* | nothing | +| `integrate-webapi:6.2.permissions-approval` | gate | plan | 6.2 (Path A) | Parsed permissions plan rendered — *"Approve and create files / Request changes / Cancel"* | nothing | +| `integrate-webapi:7.3.deploy` | gate | plan | 7.3 | *"Deploy now (Recommended) / Later"* — Web API calls won't work until deployed | nothing | + +--- + +### 6.19 `setup-datamodel` (2 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `setup-datamodel:2.source` | gate | plan | 2 | *"Upload existing ER diagram / Let the Data Model Architect figure it out"* | nothing | +| `setup-datamodel:4.2.approval` | gate | plan | 4.2 | Data model proposal rendered — *"Approve and create tables (Recommended) / Request changes / Cancel"* | nothing | + +--- + +### 6.20 `add-sample-data` (2 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `add-sample-data:3.1.tables` | gate | plan | 3.1 | Multi-select over discovered tables — *"Which tables to populate?"* | nothing | +| `add-sample-data:3.2.count` | gate | plan | 3.2 | *"How many records per table? 5 / 10 / 25 / Custom"* — covers the sub-prompt for the custom count | nothing | + +--- + +### 6.21 `add-seo` (3 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `add-seo:2.config-call1` | not-a-gate | — | 2 | Production URL + exclusion choice — data-gathering for the upcoming Phase 3 plan | — | +| `add-seo:2.config-call2` | not-a-gate | — | 2 | Meta description + OG-tag preference — data-gathering for the upcoming Phase 3 plan | — | +| `add-seo:3.plan-approval` | gate | plan | 3 | SEO plan rendered inline — *"Approve and proceed (Recommended) / Make changes"* | nothing | + +--- + +### 6.22 `create-webroles` (3 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `create-webroles:1.deploy-first` | gate | plan | 1 | `.powerpages-site` missing — *"Deploy now (Recommended) / Later"* | nothing | +| `create-webroles:3.role-selection` | gate | plan | 3 | *"Which web roles to create?"* — multi-select over suggested + custom roles | nothing | +| `create-webroles:6.deploy` | gate | plan | 6 | *"Deploy now (Recommended) / Later"* — roles don't take effect until deployed | nothing | + +--- + +### 6.23 `audit-permissions` (1 call) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `audit-permissions:6.fix-offer` | gate | plan | 6 | Audit complete — *"Would you like me to fix any of these issues? Yes / No"* — declining leaves the audit report untouched; accepting routes to the table-permissions-architect agent | nothing | + +--- + +### 6.24 `integrate-backend` (2 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `integrate-backend:2.2.clarify` | not-a-gate | — | 2.2 | Ambiguous-intent clarification (sync vs async, external APIs, secrets, one-off vs workflow) — data-gathering for the Phase 3 plan | — | +| `integrate-backend:3.4.plan-approval` | gate | plan | 3.4 | HTML plan rendered — *"Approve and proceed / Change approach / Cancel"* — branches to the right child skill (`integrate-webapi` / `add-server-logic` / `add-cloud-flow`) | nothing | + +--- + +### 6.24a Security skills — runtime-loop pattern (now anchored) + +The four security skills introduced in PR #151 (`manage-firewall`, `manage-headers`, `scan-site`, `security-review`) use `AskUserQuestion` differently from the other skills: most calls happen inside a "recommend then ask" runtime loop that was originally described in prose without a literal `AskUserQuestion`:` call site. -For non-ALM skills, the lint rules in §5 are **warn-only** until this section is extended. ALM lint rules are **hard-fail** from day one (per §9 decision). +v3 closed this coverage hole by surfacing the recommend-then-ask block as a real call site in the prose: + +- `manage-firewall:3.action-choice` (gate, plan) — `### Default approach`; routes to which destructive action runs. +- `manage-firewall:3.execute-consent` (gate, consent) — `### Plan-validate-execute`; final consent before each WAF mutation. +- `scan-site:3.action-choice` (gate, plan) — `### Default approach`; idle/running × has-report/no-report decision. +- `manage-headers:3.per-finding` (gate, plan) — `### Default approach`; per-finding accept / customize / skip loop. +- `security-review:2.1.goal` + `5.3.next-action` (gates, plan) — goal capture + post-report next-action. + +The `### Option rules` sections in `manage-firewall` and `scan-site` retain `` markers — they document HOW to construct prompts but aren't call sites themselves. + +--- + +### 6.25 `manage-firewall` (3 gate IDs) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `manage-firewall:3.action-choice` | gate | plan | 3 (`### Default approach`) | Recommend an action based on firewall state; user accepts or picks differently. Loops back here after each Phase 4 apply if the user wants additional changes. | nothing | +| `manage-firewall:3.execute-consent` | gate | consent | 3 (`### Plan-validate-execute`) | Final consent before any destructive WAF mutation (enable/disable, add/update/delete rule). Fires PER CHANGE. | nothing | +| `manage-firewall:3.option-rules-meta` | not-a-gate | — | 3 (`### Option rules`) | Meta-documentation describing how to structure `AskUserQuestion` options in this skill — not a literal call site. | — | + +--- + +### 6.26 `manage-headers` (1 call) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `manage-headers:3.per-finding` | gate | plan | 3 (`### Default approach`) | Per-finding loop — *"For each finding, present via `AskUserQuestion`: accept the recommendation, customize, or skip"*. Fires PER FINDING; skipped findings leave the header at its current value. | nothing | + +--- + +### 6.27 `scan-site` (2 gate IDs) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `scan-site:3.action-choice` | gate | plan | 3 (`### Default approach`) | Recommend an action based on scan state (running/idle/has-report). Starting a new scan triggers a multi-minute backend run; using an existing report is free. | nothing | +| `scan-site:3.option-rules-meta` | not-a-gate | — | 3 (`### Option rules`) | Meta-documentation describing how to structure `AskUserQuestion` options. Not a call site. | — | + +--- + +### 6.28 `security-review` (2 calls) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `security-review:2.1.goal` | gate | plan | 2.1 | *"What to review? Access & config / Release readiness / Deployed site"* — branches into 3 different sub-skill sets. | nothing | +| `security-review:5.3.next-action` | gate | plan | 5.3 | Post-report prompt — *"Walk me through the fixes / Re-run the review / Done for now"*. Drives whether remediation skills get invoked next. | nothing | + +--- + +### 6.29 `add-ai-webapi` (4 calls) + +New skill introduced by PR #144. Orchestrates AI summarization API integration across three layers (Web API settings, table permissions, and `Summarization/*` site settings). It delegates heavily to `/integrate-webapi` and `/create-webroles` sub-skills; the gates below cover the orchestrator-level decisions. + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `add-ai-webapi:iter.deploy-commit` | gate | consent | Iteration mode | End-of-iteration batched prompt — *"Deploy and commit now? / Just commit / Just deploy / Neither"*. Avoids per-tweak upload + commit noise on re-entry runs. | nothing | +| `add-ai-webapi:4.2.skip-webrole` | gate | consent | 4.2 | *"Continue without a web role (AI endpoints will 403) / Stop here"* — fires only when the user skipped web-role creation and the run is on a known-broken path. | nothing | +| `add-ai-webapi:5.5.commit` | gate | consent | 5.5 | *"Commit Phase 5 Layer 3 integration changes now? / Skip"* — explicit commit after summarization-service + UI wiring complete. | nothing | +| `add-ai-webapi:6.4.commit` | gate | consent | 6.4 | *"Commit new Summarization/* site settings? / Skip"* — explicit commit after `ai-webapi-settings-architect` creates the YAMLs. | nothing | + +--- + +### Cross-plugin shared skills — out of catalog scope + +`report-issue` — The power-pages SKILL.md wrapper at `plugins/power-pages/skills/report-issue/SKILL.md` is a thin re-export that contains no prompts. The actual workflow with `AskUserQuestion` calls lives in `shared/skills/report-issue/report-issue-workflow.md`, which is consumed by every plugin (not just power-pages). The shared workflow lies outside the per-plugin lint scope (`plugins/power-pages/skills/`), so its prompts are not catalogued here. If the shared workflow is ever ported into per-plugin SKILL.md files, add a `report-issue:*` section to this catalog. + +--- + +## 8. Plugin-wide enforcement (was: non-ALM deferral) + +> **v3 update.** This section previously listed 13 deferred non-ALM skills. Those skills are now catalogued in §6.13–§6.24 above (plus the security skills introduced by PR #151 in §6.25–§6.28) and the lint runs hard-fail across the whole plugin. + +The lint rules in §5 fire at `error` severity for **every** SKILL.md under `plugins/power-pages/skills/`. There is no skill-class carve-out. When you add a new skill: + +1. Catalog every `AskUserQuestion` call in §6 — pick category from §3, pick `cancel-leaves` from §4.3. +2. Add `` markers in the SKILL.md per §4. +3. Mark every data-gathering prompt with ``. +4. Run `node scripts/lint-skills-alm.js`. CI will block the PR otherwise. --- @@ -501,30 +698,25 @@ These need explicit confirmation from the reviewer before SKILL.md edits land. R | 1 | Canonical term | **"Approval Gate"** | CI/CD heritage; already the most common word in our SKILL.md files; concrete. Drop "review gate" if used informally. | | 2 | Marker syntax | **HTML comment `` + human `> 🚦 Gate (...)` block** | Comment is the lint anchor; block is for humans. Robust to interleaved prose. | | 3 | Catalog location | **`plugins/power-pages/references/approval-gates.md`** (this file) + a one-line pointer in `PLUGIN_DEVELOPMENT_GUIDE.md` | Sits with other shared references; cross-skill scope is obvious from the path. | -| 4 | Lint rollout strictness | **ALM: hard-fail. Non-ALM: warn-only until §8 catalog extends.** | ALM is fully catalogued; non-ALM is the follow-up. Hard-fail on ALM forces drift to be caught at PR time. | +| 4 | Lint rollout strictness | **[v2 — superseded by §10.] ALM: hard-fail. Non-ALM: warn-only until §8 catalog extends.** v3 made enforcement hard-fail across every skill once the catalog was extended; the warn-only branch is gone. | ALM is fully catalogued; non-ALM is the follow-up. Hard-fail on ALM forces drift to be caught at PR time. | | 5 | Emoji vs plain text | **Keep `🚦` in the human block; lint anchors on the HTML comment regardless** | Emoji is for humans; tooling doesn't depend on it. | | 6 | Wildcard gate IDs (e.g. `diagnose-deployment:6.*`) | **Disallowed. Enumerate per pattern.** | Per-pattern markers enforce that each catalog-listed deployment-error pattern has matching prompt logic. | --- -## 10. Landing plan - -The reviewer's recommendation — **land §1–§5 + §7–§9 as documentation now; do the SKILL.md sweep + lint rule as a follow-up PR** — is the right shape. Concretely: +## 10. Landing history -**This PR (proposed):** -- Land this `approval-gates.md` v2 file. -- Add a one-line pointer in `PLUGIN_DEVELOPMENT_GUIDE.md` (under the Three-Point Approval Pattern section). -- No SKILL.md edits. -- No new lint rule yet. +**v2 PR (landed)** — `approval-gates.md` v2 doc; `` markers added to the 12 ALM SKILL.md files; 5 GATE lint rules added to `scripts/lint-skills-alm.js` at hard-fail for ALM, warn-only for non-ALM. -**Follow-up PR (after §9 decisions confirmed):** -- For each ALM SKILL.md, add the `` HTML comment + human `> 🚦 Gate (...)` block above every gate listed in §6. -- Mark every "not-a-gate" row with ``. -- Add the 5 lint rules to `scripts/lint-skills-alm.js` with hard-fail for ALM, warn-only for non-ALM. -- Update `references/deployment-error-catalog.md` to cross-reference the per-pattern gate IDs from §6.12. +**v3 PR (this branch — `users/nityagi/ApplyApprovalGatesPattern`)** — extends the catalog and enforcement to the 12 non-ALM skills plus the 4 security skills picked up in the rebase: -**Follow-up #2 (non-ALM extension):** -- Sweep the 13 non-ALM skills, populate §8 with full catalog rows, switch their lint mode from warn to hard-fail. +- §6.13–§6.24 added — full catalog rows for `create-site`, `deploy-site`, `add-server-logic`, `add-cloud-flow`, `setup-auth`, `integrate-webapi`, `setup-datamodel`, `add-sample-data`, `add-seo`, `create-webroles`, `audit-permissions`, `integrate-backend` (45 gates + 9 not-a-gates). +- §6.24a–§6.28 added — security skills introduced by PR #151 (`manage-firewall`, `manage-headers`, `scan-site`, `security-review`). The new skills use a runtime-loop prompt pattern; §6.24a documents the marker convention for that pattern. 3 gates + 2 not-a-gates. +- Markers added to all non-ALM SKILL.md files (HTML comment + 🚦 block per gate; `not-a-gate` comment per data-gathering prompt or meta-mention). +- `scripts/lint-skills-alm.js` warn-only branch removed — all skills now hard-fail. +- `AGENTS.md` Key Patterns updated — Approval Gate convention applies plugin-wide; new skills must extend §6 in the same PR they introduce a prompt. +- `report-issue` excluded from the catalog (cross-plugin shared workflow lives outside the per-plugin lint scope). **Cross-plugin TODO:** the `AskUserQuestion` calls in `shared/skills/report-issue/report-issue-workflow.md` are not caught by any per-plugin lint today. A future cross-plugin sweep should either (a) define a `shared:*` namespace in the catalog and add markers in the shared workflow, or (b) duplicate the workflow into each plugin so the per-plugin lint covers it. +- v3 lint changes: removed warn-only branch; tightened `m <= promptLine` to strict `m <`; relaxed `CATALOG_GATE_ID_PATTERN` to case-insensitive on the skill-name segment; added two new rules — `CATALOG-row-must-have-marker` (reverse check — catalog rows of `kind: gate` must have a matching SKILL.md marker; prevents the orphan-row class of bug v3 closed by hand) and `GATE-prose-block-required` (every marker must be followed within 10 lines by a 🚦 sentinel; minimum-viable check against prose-block deletion). Field rename: `Blast radius if skipped:` → `Why we ask:` across all ~60 prose blocks plus §4.1 template. --- @@ -536,3 +728,6 @@ These are honest unresolved questions — not necessary to answer before v2 land - **Should `pause` gates be allowed to auto-resume?** Currently the lint rule would flag any tooling that auto-responds. But if PP Pipelines exposes a polling endpoint that detects approval state, a deterministic auto-resume becomes possible. Worth a future rule extension. - **Telemetry on gate cancellation.** A gate that's cancelled 80% of the time is asking the wrong question. Out of scope for v2; worth instrumenting once §5 lint lands. - **Multi-prompt gates.** Some entries in §6 cover multiple `AskUserQuestion` calls under one marker (e.g., `setup-solution:5.5*` is one logical gate but renders three multiSelect prompts). The lint rule says one marker can cover multiple calls if the catalog row documents it. Worth a more precise rule once we see drift. +- **Phase-number drift is silent.** Catalog rows reference SKILL.md phase numbers as plain strings (`7.6.4`, `3 (Q3 PP)`, `2.1.2`). If a skill is refactored to renumber phases (e.g. `7.6.4` → `7.7.1`), the catalog row's "Phase" column desyncs with no signal. **Convention:** any SKILL.md phase renumber MUST grep this catalog for the old phase number and update the row(s). Worth a future lint rule that asserts each catalog phase-reference is findable as a heading in the owning SKILL.md. +- **Runtime-loop coverage — historical.** Earlier v3 drafts treated the manage-firewall + scan-site destructive prompts as a known coverage gap because they sat inside a "recommend then ask" loop with no statically-locatable `AskUserQuestion`:` call site, and only the `### Option rules` sections carried `not-a-gate` markers. v3 closed the gap (see §6.24a) by restructuring the prose to surface real call sites and adding `manage-firewall:3.action-choice`, `manage-firewall:3.execute-consent`, and `scan-site:3.action-choice` as full gate markers. The `not-a-gate` markers on the `### Option rules` sections are retained for the meta-documentation sections only. +- **Lint enforces only the 🚦 sentinel, not the 3 structured labels.** `GATE-prose-block-required` checks for 🚦 within 10 lines of a marker. It does NOT verify the recommended `> **Trigger:**` / `> **Why we ask:**` / `> **Cancel leaves:**` labels — 80+ legacy v2 markers use a one-line prose style. Drift on the labels remains possible. Tightening the rule to require the 3 labels would force a structural rewrite of every legacy marker — explicit deferral. diff --git a/plugins/power-pages/scripts/lib/validate-deployment-settings.js b/plugins/power-pages/scripts/lib/validate-deployment-settings.js index 17c5edfd2..966cb54d4 100644 --- a/plugins/power-pages/scripts/lib/validate-deployment-settings.js +++ b/plugins/power-pages/scripts/lib/validate-deployment-settings.js @@ -326,45 +326,20 @@ function parseArgs(argv) { return out; } -// Read entries from the settings file, preserving stage attribution so -// findings can report which stage a bad value lives in (matters in -// multi-stage files where the same schema may be valid for one stage -// and broken for another). -function readEntriesPreservingStage(filePath, stageLabel) { - // verify-env-var-values.js exports readSettingsFile, which returns - // `[{ schemaName, value }]` after filtering by stageLabel — but loses - // the stage attribution. Re-read here so we can tag each finding with - // its stage when it came from a Stages[]-shaped file without a filter. - const fs = require('fs'); - let parsed; - try { - parsed = JSON.parse(fs.readFileSync(filePath, 'utf8')); - } catch (err) { - throw new Error(`could not read --settingsFile ${filePath}: ${err.message}`); - } - const out = []; - if (Array.isArray(parsed.Stages)) { - for (const stage of parsed.Stages) { - if (stageLabel && (stage.Name || '').toLowerCase() !== stageLabel.toLowerCase()) continue; - if (!Array.isArray(stage.EnvironmentVariables)) continue; - for (const ev of stage.EnvironmentVariables) { - out.push({ schemaName: ev.SchemaName, value: ev.Value, stageLabel: stage.Name || null }); - } - } - return out; - } - if (Array.isArray(parsed.EnvironmentVariables)) { - for (const ev of parsed.EnvironmentVariables) { - out.push({ schemaName: ev.SchemaName, value: ev.Value, stageLabel: null }); - } - return out; - } - return []; -} - +// Entry reader: delegates to verify-env-var-values.js#readSettingsFile, +// which handles all three deployment-settings.json shapes and returns +// `{ schemaName, value, stageLabel }` on each entry. +// +// `preserveAllStages: true` is critical here — without it, the default +// readSettingsFile path dedupes by schemaName (keeping only the first +// stage's value for each env var). For VALIDATION we must inspect every +// stage's value independently: the same schema can be valid in Staging +// and invalid in Production, and the validator must catch both. async function validateSettings({ settingsFile, envUrl, stageLabel, token }) { if (!settingsFile) throw new Error('--settingsFile is required'); - const entries = readEntriesPreservingStage(settingsFile, stageLabel); + const entries = readSettingsFile(settingsFile, stageLabel, { + preserveAllStages: true, + }); // Collect unique schema names for the type lookup pass. const uniqueSchemas = Array.from(new Set(entries.map((e) => e.schemaName).filter(Boolean))); @@ -452,7 +427,6 @@ module.exports = { classifyEntry, classifyValueFormat, lookupTypes, - readEntriesPreservingStage, KV_URI_PATTERN, KV_RESOURCE_ID_PATTERN, KV_PLACEHOLDER_PATTERNS, diff --git a/plugins/power-pages/scripts/lib/verify-env-var-values.js b/plugins/power-pages/scripts/lib/verify-env-var-values.js index 1f73b65a6..d597ff684 100644 --- a/plugins/power-pages/scripts/lib/verify-env-var-values.js +++ b/plugins/power-pages/scripts/lib/verify-env-var-values.js @@ -87,11 +87,31 @@ function parseArgs(argv) { } // Read EnvironmentVariables[] from a Microsoft-standard deployment-settings.json. -// Supports two shapes that appear in the wild: -// - Top-level `EnvironmentVariables: [{ SchemaName, Value }, ...]` (single-stage file) -// - Per-stage `Stages: [{ Name, EnvironmentVariables: [...] }, ...]` (multi-stage file) -// Returns `[{ schemaName, value }]` filtered to stageLabel when provided. -function readSettingsFile(filePath, stageLabel) { +// Supports three shapes that appear in the wild: +// 1. Top-level `EnvironmentVariables: [{ SchemaName, Value }, ...]` (single-stage file) +// 2. Per-stage array: `Stages: [{ Name, EnvironmentVariables: [...] }, ...]` +// (capital-S, used by some hand-authored files) +// 3. Per-stage keyed object: `stages: { "": { EnvironmentVariables: [...] } }` +// (lowercase-s, the actual schema emitted by configure-env-variables and +// consumed by Power Platform Pipelines — see schemas.microsoft.com/ +// power-platform/deployment-settings/2024. Discovered as a real-world gap +// against the Citizens portal site, 2026-05-26.) +// +// Options: +// stageLabel — narrows the result to a single stage's entries (matched +// case-insensitively against `Name` for shape 2 or the +// keyed-object's key for shape 3). +// preserveAllStages — when true, returns EVERY entry verbatim without +// dedupe — critical for validators that must inspect +// each stage's value independently. Default false +// (matches the old single-source-of-truth semantics for +// legacy callers like verify-env-var-values that report +// a single value per schema). +// +// Returns `[{ schemaName, value, stageLabel }]`. +// stageLabel is the stage name for shape 2/3 entries; null for shape 1. +function readSettingsFile(filePath, stageLabel, options = {}) { + const preserveAllStages = options.preserveAllStages === true; let raw; try { raw = fs.readFileSync(filePath, 'utf8'); @@ -105,35 +125,85 @@ function readSettingsFile(filePath, stageLabel) { throw new Error(`--settingsFile ${filePath} is not valid JSON: ${err.message}`); } - // Per-stage shape + const lowerLabel = stageLabel ? stageLabel.toLowerCase() : null; + + // Shape 2: per-stage array (`Stages: []`) if (Array.isArray(parsed.Stages)) { - if (!stageLabel) { - // No filter — flatten all stages + if (!lowerLabel) { const all = []; for (const stage of parsed.Stages) { if (Array.isArray(stage.EnvironmentVariables)) { for (const ev of stage.EnvironmentVariables) { - all.push({ schemaName: ev.SchemaName, value: ev.Value }); + all.push({ + schemaName: ev.SchemaName, + value: ev.Value, + stageLabel: stage.Name || null, + }); } } } - return dedupeBySchemaName(all); + return preserveAllStages ? all : dedupeBySchemaName(all); } const stage = parsed.Stages.find( - (s) => (s.Name || '').toLowerCase() === stageLabel.toLowerCase() + (s) => (s.Name || '').toLowerCase() === lowerLabel + ); + if (!stage || !Array.isArray(stage.EnvironmentVariables)) return []; + return stage.EnvironmentVariables.map((ev) => ({ + schemaName: ev.SchemaName, + value: ev.Value, + stageLabel: stage.Name || null, + })); + } + + // Shape 3: per-stage keyed object (e.g. `stages: { "": {...} }`). + // This is the schema emitted by configure-env-variables and the one + // Power Platform Pipelines actually accepts. Accept any common casing of + // the top-level key (`stages`, `Stages`, `STAGES`) so hand-authored files + // with mixed casing still resolve. (The shape 2 array check above only + // matches when `parsed.Stages` is an Array; if it's a plain object, this + // branch picks it up.) + const stagesObj = + parsed.stages || parsed.Stages || parsed.STAGES || null; + if ( + stagesObj && + typeof stagesObj === 'object' && + !Array.isArray(stagesObj) + ) { + if (!lowerLabel) { + const all = []; + for (const [name, stage] of Object.entries(stagesObj)) { + if (stage && Array.isArray(stage.EnvironmentVariables)) { + for (const ev of stage.EnvironmentVariables) { + all.push({ + schemaName: ev.SchemaName, + value: ev.Value, + stageLabel: name, + }); + } + } + } + return preserveAllStages ? all : dedupeBySchemaName(all); + } + // Case-insensitive key match + const matchKey = Object.keys(stagesObj).find( + (k) => k.toLowerCase() === lowerLabel ); + if (!matchKey) return []; + const stage = stagesObj[matchKey]; if (!stage || !Array.isArray(stage.EnvironmentVariables)) return []; return stage.EnvironmentVariables.map((ev) => ({ schemaName: ev.SchemaName, value: ev.Value, + stageLabel: matchKey, })); } - // Top-level shape + // Shape 1: top-level `EnvironmentVariables: []` (single-stage file) if (Array.isArray(parsed.EnvironmentVariables)) { return parsed.EnvironmentVariables.map((ev) => ({ schemaName: ev.SchemaName, value: ev.Value, + stageLabel: null, })); } return []; diff --git a/plugins/power-pages/scripts/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 073c000bf..fca79873b 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -29,9 +29,10 @@ // (matched by `` `AskUserQuestion` `` near a `:`). // Require: the same section contains at least one preceding // `` or `` marker. -// Scope: ALM skills (see ALM_SKILLS) → severity 'error'. -// Non-ALM skills → severity 'warning' (warn-only until the -// catalog in references/approval-gates.md is extended). +// Scope: Every SKILL.md under plugins/power-pages/skills/ → severity 'error'. +// (v2 had a warn-only branch for non-ALM skills; v3 removed it once +// the catalog in references/approval-gates.md was extended to cover +// the full skill set — see §10 landing history.) // Waivable: yes, inline `` // or `.almlintignore` entry. // @@ -45,7 +46,8 @@ // Trigger: any gate-id used in a SKILL.md marker. // Require: the same gate-id appears (backticked) somewhere in // references/approval-gates.md (the catalog). -// Scope: ALM skills → 'error'. Non-ALM → 'warning'. +// Scope: Every SKILL.md → 'error'. (Same v3 enforcement note as +// GATE-must-have-marker above.) // Waivable: yes (inline + allowlist). // // GATE-intent-must-call-helper @@ -64,10 +66,34 @@ // Waivable: yes — custom slugs allowed; rule flags only typos / // non-kebab values. // +// CATALOG-row-must-have-marker +// Trigger: every `| `` | gate | ...` row in §6 of the catalog +// (references/approval-gates.md). Only rows tagged `gate` — +// `not-a-gate` rows are skipped because not-a-gate markers in +// SKILL.md don't carry an ID to match against. +// Require: at least one `` marker in some +// SKILL.md under plugins/power-pages/skills/. +// Waivable: `.almlintignore` allowlist only — this rule operates on the +// catalog as a whole (not per SKILL.md), so inline +// `` comments are not honored. +// To suppress an orphan-row finding, add an allowlist entry like: +// references/approval-gates.md CATALOG-row-must-have-marker +// +// GATE-prose-block-required +// Trigger: any `` marker. +// Require: within the next 10 lines, a line carrying the 🚦 sentinel +// appears. Catches the "future PR deletes the prose block" +// drift case without forcing the v3 3-label structure on 80+ +// legacy v2 single-line markers. The richer structured fields +// (Trigger / Why we ask / Cancel leaves) are recommended in +// §4.1 of references/approval-gates.md but not enforced. +// Waivable: yes — inline ignore comment. +// // Usage: // node scripts/lint-skills-alm.js [--plugin-root ] -// Exit 0 when no findings (or only warnings); exit 1 when at least one -// finding has severity 'error'. stderr lists errors; stdout lists warnings. +// Exit 0 only when there are zero findings. Exit 1 on any finding — +// v3 is hard-fail uniform, so every finding has severity 'error' and +// every output goes to stderr. // // The script is pure-Node, has no dependencies, and returns findings // programmatically so the tests can assert behavior without spawning processes. @@ -185,24 +211,8 @@ const KNOWN_RULES = new Set([ 'GATE-must-be-in-catalog', 'GATE-intent-must-call-helper', 'GATE-cancel-leaves-known-vocab', -]); - -// ALM skills get hard-fail severity; everything else gets warn-only until -// the approval-gates catalog is extended to non-ALM skills (see §8 of -// references/approval-gates.md). Folder name = skill name. -const ALM_SKILLS = new Set([ - 'plan-alm', - 'setup-solution', - 'setup-pipeline', - 'deploy-pipeline', - 'export-solution', - 'import-solution', - 'configure-env-variables', - 'ensure-pipelines-host', - 'force-link-environment', - 'activate-site', - 'test-site', - 'diagnose-deployment', + 'CATALOG-row-must-have-marker', + 'GATE-prose-block-required', ]); // `category=intent` markers must be backed by a real helper invocation. @@ -428,23 +438,24 @@ function findPromptLines(content) { return out; } -function skillNameFromFile(file) { - // Expect .../skills//SKILL.md - const parts = file.split(/[\\/]/); - const idx = parts.lastIndexOf('skills'); - if (idx < 0 || idx + 1 >= parts.length) return null; - return parts[idx + 1]; -} - -function severityForSkill(skillName) { - return ALM_SKILLS.has(skillName) ? 'error' : 'warning'; -} +// v3: hard-fail uniformly across every SKILL.md under plugins/power-pages/skills/. +// Kept as a named constant so every finding-push site references the same +// source of truth; flipping severity for a future rule class would touch a +// single line here rather than rewriting every literal. +const SKILL_SEVERITY = 'error'; // Parse the catalog file (references/approval-gates.md) and extract all // backticked gate-id strings. Returns a Set, or null if the catalog isn't // present (downgrades GATE-must-be-in-catalog to no-op so the lint isn't // hard-broken when the catalog is removed/renamed). -const CATALOG_GATE_ID_PATTERN = /`([a-z][a-z0-9-]*:[A-Za-z0-9._-]+)`/g; +// +// Case-insensitive on the skill-name segment to align with GATE_MARKER_PATTERN +// — SKILL.md gate markers allow any case in the skill-name segment, so the +// catalog scan must too, otherwise a CamelCase skill would fail +// GATE-must-be-in-catalog even with a correct catalog row. Underscores are +// intentionally NOT allowed — current naming convention is kebab-case for +// skill names; allow only [A-Za-z][A-Za-z0-9-]* on that segment. +const CATALOG_GATE_ID_PATTERN = /`([A-Za-z][A-Za-z0-9-]*:[A-Za-z0-9._-]+)`/g; function loadCatalogGateIds(pluginRoot) { const catalogFile = path.join(pluginRoot, 'references', 'approval-gates.md'); @@ -459,6 +470,31 @@ function loadCatalogGateIds(pluginRoot) { return ids; } +// Catalog row pattern: matches `| `` | gate | ...` rows in §6 tables. +// Only rows tagged `gate` (not `not-a-gate`) need a matching marker — not-a-gate +// markers in SKILL.md don't carry an ID (they're free-text reasons), so the +// reverse check is meaningless for them. +// +// Leading whitespace tolerance: GFM accepts up to 3 spaces of indentation +// before the leading `|`, so we allow `^\s{0,3}\|`. Without this, a future +// markdown reformat that nests §6 tables under a parent list silently +// disables orphan detection for the indented rows. +const CATALOG_GATE_ROW_PATTERN = + /^\s{0,3}\|\s*`([A-Za-z][A-Za-z0-9-]*:[A-Za-z0-9._-]+)`\s*\|\s*gate\s*\|/gm; + +function loadCatalogGateRows(pluginRoot) { + const catalogFile = path.join(pluginRoot, 'references', 'approval-gates.md'); + if (!fs.existsSync(catalogFile)) return null; + const content = fs.readFileSync(catalogFile, 'utf8'); + const ids = new Set(); + CATALOG_GATE_ROW_PATTERN.lastIndex = 0; + let m; + while ((m = CATALOG_GATE_ROW_PATTERN.exec(content)) !== null) { + ids.add(m[1]); + } + return ids; +} + // Section-level pair: for each prompt, is there a preceding gate or // not-a-gate marker in the same section? function checkSectionPairing(content) { @@ -476,7 +512,11 @@ function checkSectionPairing(content) { (l) => sectionStart - 1 + l ); for (const promptLine of prompts) { - const hasPreceding = fileMarkerLines.some((m) => m <= promptLine); + // Marker must be on a line STRICTLY BEFORE the prompt line. + // `<=` would let a marker sit on the same line as the prompt (or be + // smuggled inside the prompt's line via inline HTML), which trivially + // satisfies the rule without matching its intent. + const hasPreceding = fileMarkerLines.some((m) => m < promptLine); if (!hasPreceding) unmatched.push({ heading: section.heading, lineNum: promptLine }); } } @@ -511,15 +551,13 @@ function collectFindings({ pluginRoot }) { for (const file of skillFiles) { const content = fs.readFileSync(file, 'utf8'); const ignores = extractIgnores(content); - const skillName = skillNameFromFile(file); - const skillSeverity = severityForSkill(skillName); if (!ignores.has('SKILL-must-read-manifest')) { const touches = touchesDataverseWrites(content); if (touches && !hasManifestRead(content)) { findings.push({ rule: 'SKILL-must-read-manifest', - severity: 'error', + severity: SKILL_SEVERITY, file, message: 'Skill creates Dataverse records but does not reference `.solution-manifest.json`. ' + @@ -536,7 +574,7 @@ function collectFindings({ pluginRoot }) { if (!knownPpcTypes.has(typeValue)) { findings.push({ rule: 'DISCOVER-coverage', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Skill references powerpagecomponenttype=${typeValue} but that value is not in ` + @@ -558,7 +596,7 @@ function collectFindings({ pluginRoot }) { for (const u of unmatched) { findings.push({ rule: 'GATE-must-have-marker', - severity: skillSeverity, + severity: SKILL_SEVERITY, file, message: `Phase section "${u.heading}" contains an \`AskUserQuestion\` prompt (line ${u.lineNum}) ` + @@ -576,7 +614,7 @@ function collectFindings({ pluginRoot }) { if (!callsHelper) { findings.push({ rule: 'GATE-intent-must-call-helper', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Skill declares ${intentMarkers.length} \`category=intent\` gate(s) ` + @@ -589,6 +627,77 @@ function collectFindings({ pluginRoot }) { } } + // GATE-prose-block-required — every `` must be followed + // within 10 lines (inclusive of the marker's own line, to support a + // single-line marker-plus-🚦 style if a future contributor chooses it) + // by a line carrying the 🚦 sentinel OUTSIDE any fenced code block. + // This is the minimum viable check against "future PR deletes the + // human-readable block" drift: it catches deletion + ID-line tampering + // without forcing a structural rewrite of legacy v2 single-line prose. + // The richer structured fields (`> **Trigger:**`, `> **Why we ask:**`, + // `> **Cancel leaves:**`) are recommended in §4.1 of references/approval- + // gates.md for new markers but not lint-enforced — keeping the rule + // tight enough that 80+ existing v2 markers don't need to be rewritten + // in the same PR. + // + // Code-fence awareness: a literal 🚦 inside a ```bash``` example or + // similar should NOT satisfy the rule, otherwise a contributor who + // deletes the real Gate prose block but leaves an example 🚦 within + // the window silently passes. We track fence state across the FULL + // file (not just the window) so the in/out determination is correct + // when the window opens mid-fence. + if (!ignores.has('GATE-prose-block-required')) { + const lines = content.split(/\r?\n/); + // Pre-compute: for each line, is it inside a fenced code block? + // A fence-toggle line is one that starts with ``` (optionally followed + // by an info string). Bare ``` or ```lang both toggle, but only when + // appearing at column 0 (after trimming up to 3 spaces of indent per + // CommonMark fenced-code rules — approximated with /^\s{0,3}```/). + // + // The fence-delimiter LINE ITSELF (both opening and closing) is marked + // OUTSIDE — it's Markdown syntax, not code content. Only the content + // lines strictly between two delimiters are INSIDE. This makes both + // delimiters classified consistently with each other (a previous + // version flipped `inside` BEFORE recording, which left the opening + // fence INSIDE and the closing fence OUTSIDE — asymmetric). + const inFence = new Array(lines.length).fill(false); + let inside = false; + const FENCE_PATTERN = /^\s{0,3}```/; + for (let i = 0; i < lines.length; i++) { + if (FENCE_PATTERN.test(lines[i])) { + inFence[i] = false; // delimiter line itself: not inside + inside = !inside; + } else { + inFence[i] = inside; + } + } + for (const gm of gateMarkers) { + const startIdx = gm.lineNum - 1; // 0-based + const endExclusive = Math.min(startIdx + 10, lines.length); + let hasSentinel = false; + for (let i = startIdx; i < endExclusive; i++) { + if (lines[i].includes('🚦') && !inFence[i]) { + hasSentinel = true; + break; + } + } + if (!hasSentinel) { + findings.push({ + rule: 'GATE-prose-block-required', + severity: SKILL_SEVERITY, + file, + message: + `Gate \`${gm.gateId}\` (line ${gm.lineNum}) is missing the 🚦 ` + + `prose block within 10 lines (a line carrying the 🚦 sentinel ` + + `outside any fenced code block). Every gate marker must be ` + + `followed by a \`> 🚦 **Gate (...)**\` line so humans see ` + + `the same context the lint sees.`, + hint: 'See references/approval-gates.md §4.1 for the marker + prose template.', + }); + } + } + } + // GATE-cancel-leaves-known-vocab — value must be in vocab OR kebab-case. if (!ignores.has('GATE-cancel-leaves-known-vocab')) { for (const gm of gateMarkers) { @@ -597,7 +706,7 @@ function collectFindings({ pluginRoot }) { if (KEBAB_CASE_PATTERN.test(v)) continue; findings.push({ rule: 'GATE-cancel-leaves-known-vocab', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Gate \`${gm.gateId}\` has \`cancel-leaves=${v}\` which is neither a known vocabulary value ` + @@ -614,7 +723,7 @@ function collectFindings({ pluginRoot }) { if (!catalogGateIds.has(gm.gateId)) { findings.push({ rule: 'GATE-must-be-in-catalog', - severity: skillSeverity, + severity: SKILL_SEVERITY, file, message: `Gate \`${gm.gateId}\` is declared in SKILL.md but is not in the catalog ` + @@ -640,7 +749,7 @@ function collectFindings({ pluginRoot }) { .join(', '); findings.push({ rule: 'GATE-id-must-be-unique', - severity: 'error', + severity: SKILL_SEVERITY, file: occurrences[0].file, message: `Gate id \`${gateId}\` is declared in ${occurrences.length} places: ${locs}. ` + @@ -649,6 +758,30 @@ function collectFindings({ pluginRoot }) { }); } + // CATALOG-row-must-have-marker — every `kind: gate` catalog row must have at + // least one matching marker in a SKILL.md. This is the + // reverse of GATE-must-be-in-catalog and prevents the orphan-row class of + // bug that v3 closed by hand: catalog rows that describe gates with no + // corresponding SKILL.md anchor accumulate silently otherwise. + const catalogGateRows = loadCatalogGateRows(pluginRoot); + if (catalogGateRows) { + const markerIdSet = new Set(allGateMarkers.map((m) => m.gateId)); + const catalogFile = path.join(pluginRoot, 'references', 'approval-gates.md'); + for (const gateId of catalogGateRows) { + if (markerIdSet.has(gateId)) continue; + findings.push({ + rule: 'CATALOG-row-must-have-marker', + severity: SKILL_SEVERITY, + file: catalogFile, + message: + `Catalog row \`${gateId}\` (kind: gate) has no matching ` + + `\`\` marker in any SKILL.md. Either add ` + + `the marker to the owning skill, or remove/retag the catalog row.`, + hint: 'See references/approval-gates.md §7 (How to add a new gate) and §10 (catalog completeness).', + }); + } + } + // Rule 2 — SCRIPT-must-use-resolver. for (const file of scriptFiles) { const content = fs.readFileSync(file, 'utf8'); @@ -659,7 +792,7 @@ function collectFindings({ pluginRoot }) { findings.push({ rule: 'SCRIPT-must-use-resolver', - severity: 'error', + severity: SKILL_SEVERITY, file, message: 'Script creates Dataverse records (AddSolutionComponent / publisher / solution / env var definition) ' + @@ -697,32 +830,33 @@ function main(argv) { } const findings = collectFindings({ pluginRoot }); - const errors = findings.filter((f) => f.severity === 'error'); - const warnings = findings.filter((f) => f.severity === 'warning'); if (findings.length === 0) { process.stdout.write('alm-lint: 0 findings\n'); return 0; } - // Warnings go to stdout (informational); errors go to stderr. - for (const f of warnings) process.stdout.write(formatFinding(f, pluginRoot)); - for (const f of errors) process.stderr.write(formatFinding(f, pluginRoot)); - - if (errors.length === 0) { - process.stdout.write( - `\nalm-lint: ${warnings.length} warning(s) in ${pluginRoot} (no errors)\n` - ); - return 0; - } - process.stderr.write( - `\nalm-lint: ${errors.length} error(s), ${warnings.length} warning(s) in ${pluginRoot}\n` - ); + // v3: every finding is `severity: 'error'`. The warn-only branch existed + // pre-v3 for non-ALM skills; once the catalog covered every skill the + // branch became unreachable. Any future re-introduction of a 'warning' + // severity should restore the stdout-vs-stderr split here. + // + // Single concatenation + one write call avoids the Node-on-Windows + // truncation case where many small synchronous writes to a piped stderr + // (CI redirecting to a log file) can drop trailing data when process.exit + // fires before the OS has drained the pipe. + const out = + findings.map((f) => formatFinding(f, pluginRoot)).join('') + + `\nalm-lint: ${findings.length} error(s) in ${pluginRoot}\n`; + process.stderr.write(out); return 1; } if (require.main === module) { - process.exit(main(process.argv)); + // Use exitCode instead of process.exit so the event loop drains stderr + // before the process terminates — important for CI runs that pipe stderr + // to a log file. + process.exitCode = main(process.argv); } module.exports = { @@ -732,7 +866,6 @@ module.exports = { parseAllowlist, allowlistPathMatches, KNOWN_RULES, - ALM_SKILLS, INTENT_HELPERS, CANCEL_LEAVES_VOCAB, // Approval Gate parsing helpers (exported for tests): diff --git a/plugins/power-pages/scripts/tests/lint-skills-alm.test.js b/plugins/power-pages/scripts/tests/lint-skills-alm.test.js index 152e2f9a1..1de627bd4 100644 --- a/plugins/power-pages/scripts/tests/lint-skills-alm.test.js +++ b/plugins/power-pages/scripts/tests/lint-skills-alm.test.js @@ -12,7 +12,6 @@ const { parseAllowlist, allowlistPathMatches, KNOWN_RULES, - ALM_SKILLS, CANCEL_LEAVES_VOCAB, extractGateMarkers, extractNotAGateMarkers, @@ -565,7 +564,9 @@ Ask via \`AskUserQuestion\`: assert.equal(match.severity, 'error', 'ALM skill → error severity'); }); -test('GATE-must-have-marker: fires as warning for non-ALM skill', async (t) => { +test('GATE-must-have-marker: fires as error for non-ALM skill (v3 plugin-wide enforcement)', async (t) => { + // v3 removed the ALM-vs-non-ALM severity carve-out: every skill is hard-fail. + // See references/approval-gates.md §10 landing history. const root = mkPluginRoot(t); writeSkill( root, @@ -578,7 +579,7 @@ Ask via \`AskUserQuestion\`: const findings = collectFindings({ pluginRoot: root }); const match = findings.find((f) => f.rule === 'GATE-must-have-marker'); assert.ok(match); - assert.equal(match.severity, 'warning', 'non-ALM skill → warning severity'); + assert.equal(match.severity, 'error', 'v3: all skills → error severity'); }); test('GATE-must-have-marker: passes when section has a gate marker before the prompt', async (t) => { @@ -613,6 +614,27 @@ Ask via \`AskUserQuestion\`: assert.equal(findings.filter((f) => f.rule === 'GATE-must-have-marker').length, 0); }); +test('GATE-must-have-marker: fails when marker is on the SAME line as the prompt (strict precede)', async (t) => { + // v3 tightened m <= promptLine to m < promptLine. A single-line shape like + // ` Use \`AskUserQuestion\` for X:` matches both regexes + // at the same line number; under the old <= rule this passed trivially. + // The new < rule requires the marker to be on a line STRICTLY BEFORE the + // prompt. + const root = mkPluginRoot(t); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + Use \`AskUserQuestion\` for X: +` + ); + writeCatalog(root, ['plan-alm:1.foo']); + const findings = collectFindings({ pluginRoot: root }); + const match = findings.find((f) => f.rule === 'GATE-must-have-marker'); + assert.ok(match, 'expected GATE-must-have-marker to fire — marker on prompt line should not satisfy pairing'); +}); + test('GATE-must-have-marker: pairing is per-section (marker in earlier section does NOT cover later section)', async (t) => { const root = mkPluginRoot(t); writeSkill( @@ -758,21 +780,223 @@ Ask via \`AskUserQuestion\`: assert.equal(findings.filter((f) => f.rule === 'GATE-cancel-leaves-known-vocab').length, 0); }); -test('ALM_SKILLS export includes the 12 documented ALM skills', () => { - const required = [ - 'plan-alm', 'setup-solution', 'setup-pipeline', 'deploy-pipeline', - 'export-solution', 'import-solution', 'configure-env-variables', - 'ensure-pipelines-host', 'force-link-environment', 'activate-site', - 'test-site', 'diagnose-deployment', - ]; - for (const skill of required) { - assert.ok(ALM_SKILLS.has(skill), `ALM_SKILLS missing: ${skill}`); - } -}); - test('CANCEL_LEAVES_VOCAB export has the documented values', () => { const required = ['nothing', 'validated-stage-run', 'partial-manifest', 'attachment-block-modified']; for (const v of required) { assert.ok(CANCEL_LEAVES_VOCAB.has(v), `CANCEL_LEAVES_VOCAB missing: ${v}`); } }); + +// Helper for the new CATALOG-row-must-have-marker tests — writes a §6-style +// markdown table that matches CATALOG_GATE_ROW_PATTERN. Each entry: { id, kind } +// where kind is 'gate' or 'not-a-gate'. +function writeCatalogTable(root, rows) { + fs.mkdirSync(path.join(root, 'references'), { recursive: true }); + const header = + '# Approval Gates Catalog (test fixture)\n\n' + + '## 6. Catalog\n\n' + + '### 6.1 fixture\n\n' + + '| ID | Kind | Category | Phase | Trigger | Cancel leaves |\n' + + '|---|---|---|---|---|---|\n'; + const body = rows + .map((r) => `| \`${r.id}\` | ${r.kind} | plan | 1 | test | nothing |`) + .join('\n'); + fs.writeFileSync(path.join(root, 'references', 'approval-gates.md'), header + body + '\n'); +} + +test('CATALOG-row-must-have-marker: fires when a catalog gate row has no SKILL.md marker', async (t) => { + const root = mkPluginRoot(t); + // Catalog has two gate rows, but only one SKILL.md marker exists. + writeCatalogTable(root, [ + { id: 'plan-alm:1.foo', kind: 'gate' }, + { id: 'plan-alm:1.orphan', kind: 'gate' }, + ]); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + +> 🚦 **Gate (plan · plan-alm:1.foo):** Description. +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + const orphans = findings.filter((f) => f.rule === 'CATALOG-row-must-have-marker'); + assert.equal(orphans.length, 1, `expected 1 orphan finding, got ${JSON.stringify(orphans)}`); + assert.match(orphans[0].message, /plan-alm:1\.orphan/); +}); + +test('CATALOG-row-must-have-marker: passes when every gate row has a marker', async (t) => { + const root = mkPluginRoot(t); + writeCatalogTable(root, [ + { id: 'plan-alm:1.foo', kind: 'gate' }, + { id: 'plan-alm:1.bar', kind: 'gate' }, + ]); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + +> 🚦 **Gate (plan · plan-alm:1.foo):** Description. +Ask via \`AskUserQuestion\`: + +## Phase 2 + +> 🚦 **Gate (plan · plan-alm:1.bar):** Description. +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + assert.equal(findings.filter((f) => f.rule === 'CATALOG-row-must-have-marker').length, 0); +}); + +test('CATALOG-row-must-have-marker: skips not-a-gate rows (no marker required)', async (t) => { + // Only `kind: gate` rows are checked. A `not-a-gate` row in the catalog has + // no corresponding ID-bearing marker (not-a-gate comments are free-text), so + // the reverse check must skip them. + const root = mkPluginRoot(t); + writeCatalogTable(root, [ + { id: 'plan-alm:1.foo', kind: 'not-a-gate' }, + { id: 'plan-alm:1.bar', kind: 'not-a-gate' }, + ]); + writeSkill(root, 'plan-alm', '# plan-alm\n\nNo gates here.\n'); + const findings = collectFindings({ pluginRoot: root }); + assert.equal(findings.filter((f) => f.rule === 'CATALOG-row-must-have-marker').length, 0); +}); + +test('CATALOG-row-must-have-marker: tolerates leading whitespace on table rows (GFM)', async (t) => { + // GFM allows up to 3 leading spaces before the leading `|`. The reverse- + // check regex must match those rows; otherwise nested catalog rows go + // invisible to orphan detection. + const root = mkPluginRoot(t); + fs.mkdirSync(path.join(root, 'references'), { recursive: true }); + const content = + '# Approval Gates Catalog (test fixture)\n\n' + + ' | `plan-alm:1.indented` | gate | plan | 1 | test | nothing |\n'; + fs.writeFileSync(path.join(root, 'references', 'approval-gates.md'), content); + writeSkill(root, 'plan-alm', '# plan-alm\n\nNo marker for the indented row.\n'); + const findings = collectFindings({ pluginRoot: root }); + const orphans = findings.filter((f) => f.rule === 'CATALOG-row-must-have-marker'); + assert.equal(orphans.length, 1, 'indented row should still be checked'); + assert.match(orphans[0].message, /plan-alm:1\.indented/); +}); + +test('GATE-prose-block-required: fires when marker has no 🚦 within 10 lines', async (t) => { + const root = mkPluginRoot(t); + writeCatalog(root, ['plan-alm:1.bare']); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + + +No prose block here, just text. +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + const match = findings.find((f) => f.rule === 'GATE-prose-block-required'); + assert.ok(match, `expected GATE-prose-block-required to fire; got ${JSON.stringify(findings)}`); + assert.match(match.message, /plan-alm:1\.bare/); +}); + +test('GATE-prose-block-required: passes when 🚦 sentinel follows within window', async (t) => { + const root = mkPluginRoot(t); + writeCatalog(root, ['plan-alm:1.good']); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + +> 🚦 **Gate (plan · plan-alm:1.good):** Description. +> +> **Trigger:** something. +> **Why we ask:** to avoid X. +> **Cancel leaves:** nothing. + +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + assert.equal(findings.filter((f) => f.rule === 'GATE-prose-block-required').length, 0); +}); + +test('GATE-prose-block-required: ignores 🚦 inside a fenced code block', async (t) => { + // A literal 🚦 inside a ```bash``` example should NOT satisfy the rule — + // otherwise a contributor who deletes the real Gate prose but leaves an + // example 🚦 within the window passes silently. The rule's purpose is to + // catch prose-block drift, not match the symbol anywhere. + const root = mkPluginRoot(t); + writeCatalog(root, ['plan-alm:1.fenced']); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + + +\`\`\`bash +echo "🚦 starting deploy" +\`\`\` + +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + const match = findings.find((f) => f.rule === 'GATE-prose-block-required'); + assert.ok(match, '🚦 inside code fence should not satisfy the rule'); +}); + +test('GATE-prose-block-required: treats both fence delimiter lines as OUTSIDE the code block (regression — Copilot review)', async (t) => { + // Pre-fix, the toggle ran BEFORE recording inFence[i], which made the + // opening ``` line INSIDE and the closing ``` line OUTSIDE — asymmetric. + // Post-fix, both delimiter lines are classified OUTSIDE; only the + // content lines strictly between them are INSIDE. A 🚦 placed on the + // OPENING fence line itself (legal Markdown info string like ```🚦) + // should therefore satisfy the rule, not be falsely rejected as + // "inside the code block". + const root = mkPluginRoot(t); + writeCatalog(root, ['plan-alm:1.fence-line']); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + +\`\`\`🚦 fence-info contains the sentinel +not really inside per the rule +\`\`\` + +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + assert.equal( + findings.filter((f) => f.rule === 'GATE-prose-block-required').length, + 0, + '🚦 on the opening fence delimiter line should satisfy the rule (delimiter lines are OUTSIDE the code block)' + ); +}); + +test('GATE-prose-block-required: tolerates 🚦 on the same line as the marker (single-line style)', async (t) => { + // The window is inclusive of the marker's own line, so a compact one-line + // marker + 🚦 should pass (legal Markdown). + const root = mkPluginRoot(t); + writeCatalog(root, ['plan-alm:1.oneliner']); + writeSkill( + root, + 'plan-alm', + `# plan-alm +## Phase 1 + > 🚦 **Gate (plan · plan-alm:1.oneliner):** Compact. + +Ask via \`AskUserQuestion\`: +` + ); + const findings = collectFindings({ pluginRoot: root }); + assert.equal(findings.filter((f) => f.rule === 'GATE-prose-block-required').length, 0); +}); diff --git a/plugins/power-pages/scripts/tests/validate-deployment-settings.test.js b/plugins/power-pages/scripts/tests/validate-deployment-settings.test.js index db967e7c5..872e9c8c8 100644 --- a/plugins/power-pages/scripts/tests/validate-deployment-settings.test.js +++ b/plugins/power-pages/scripts/tests/validate-deployment-settings.test.js @@ -11,10 +11,13 @@ const { validateSettings, classifyEntry, classifyValueFormat, - readEntriesPreservingStage, KV_URI_PATTERN, KV_RESOURCE_ID_PATTERN, } = require('../lib/validate-deployment-settings'); +// The duplicated readSettingsFile was removed in favor of +// verify-env-var-values#readSettingsFile, which now returns stageLabel on +// each entry. Tests below exercise the unified parser via its new home. +const { readSettingsFile } = require('../lib/verify-env-var-values'); function withTempDir(t) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'validate-settings-')); @@ -229,7 +232,7 @@ test('classifyEntry: unknown type with regular value falls back to unknown-type' // File parsing — preserving stage attribution // ──────────────────────────────────────────────────────────────────────────── -test('readEntriesPreservingStage handles top-level shape (no stages)', (t) => { +test('readSettingsFile handles top-level shape (no stages)', (t) => { const dir = withTempDir(t); const file = writeSettings(dir, { EnvironmentVariables: [ @@ -237,12 +240,12 @@ test('readEntriesPreservingStage handles top-level shape (no stages)', (t) => { { SchemaName: 'b', Value: 'vb' }, ], }); - const entries = readEntriesPreservingStage(file); + const entries = readSettingsFile(file); assert.equal(entries.length, 2); assert.equal(entries[0].stageLabel, null); }); -test('readEntriesPreservingStage preserves stage names in Stages[] shape', (t) => { +test('readSettingsFile preserves stage names in Stages[] shape', (t) => { const dir = withTempDir(t); const file = writeSettings(dir, { Stages: [ @@ -250,13 +253,13 @@ test('readEntriesPreservingStage preserves stage names in Stages[] shape', (t) = { Name: 'Production', EnvironmentVariables: [{ SchemaName: 'b', Value: 'pb' }] }, ], }); - const entries = readEntriesPreservingStage(file); + const entries = readSettingsFile(file); assert.equal(entries.length, 2); assert.equal(entries[0].stageLabel, 'Staging'); assert.equal(entries[1].stageLabel, 'Production'); }); -test('readEntriesPreservingStage filters by stageLabel (case-insensitive)', (t) => { +test('readSettingsFile filters by stageLabel (case-insensitive)', (t) => { const dir = withTempDir(t); const file = writeSettings(dir, { Stages: [ @@ -264,7 +267,7 @@ test('readEntriesPreservingStage filters by stageLabel (case-insensitive)', (t) { Name: 'Production', EnvironmentVariables: [{ SchemaName: 'b', Value: 'pb' }] }, ], }); - const entries = readEntriesPreservingStage(file, 'production'); + const entries = readSettingsFile(file, 'production'); assert.equal(entries.length, 1); assert.equal(entries[0].schemaName, 'b'); }); diff --git a/plugins/power-pages/scripts/tests/verify-env-var-values.test.js b/plugins/power-pages/scripts/tests/verify-env-var-values.test.js index 8d6acbf88..a4330473a 100644 --- a/plugins/power-pages/scripts/tests/verify-env-var-values.test.js +++ b/plugins/power-pages/scripts/tests/verify-env-var-values.test.js @@ -97,8 +97,8 @@ test('readSettingsFile reads top-level EnvironmentVariables shape', async (t) => ); const entries = readSettingsFile(file); assert.deepEqual(entries, [ - { schemaName: 'foo_a', value: 'a-value' }, - { schemaName: 'foo_b', value: 'b-value' }, + { schemaName: 'foo_a', value: 'a-value', stageLabel: null }, + { schemaName: 'foo_b', value: 'b-value', stageLabel: null }, ]); }); @@ -125,7 +125,9 @@ test('readSettingsFile filters Stages[] by stageLabel (case-insensitive)', async }) ); const stagingEntries = readSettingsFile(file, 'staging'); - assert.deepEqual(stagingEntries, [{ schemaName: 'foo_a', value: 'staging-a' }]); + assert.deepEqual(stagingEntries, [ + { schemaName: 'foo_a', value: 'staging-a', stageLabel: 'Staging' }, + ]); const prodEntries = readSettingsFile(file, 'Production'); assert.equal(prodEntries.length, 2); assert.equal(prodEntries[1].value, 'prod-b'); @@ -151,6 +153,148 @@ test('readSettingsFile with no stageLabel flattens Stages[]', async (t) => { assert.equal(entries[1].schemaName, 'foo_b'); }); +test('readSettingsFile accepts mixed-case `Stages` key when it is an object (regression — Copilot review)', async (t) => { + // Shape 2's array check handles `Stages: []`; shape 3's object check + // must handle `stages` / `Stages` / `STAGES` when the value is a plain + // object. Earlier code only checked `parsed.stages || parsed.STAGES`, + // so a hand-authored file with `Stages: { ... }` (mixed case + object) + // returned 0 entries. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-env-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + const file = path.join(dir, 'deployment-settings.json'); + fs.writeFileSync( + file, + JSON.stringify({ + Stages: { + Staging: { EnvironmentVariables: [{ SchemaName: 'foo_a', Value: 'sv' }] }, + Production: { EnvironmentVariables: [{ SchemaName: 'foo_b', Value: 'pv' }] }, + }, + }) + ); + const all = readSettingsFile(file); + // Without preserveAllStages, distinct schemas → no dedupe collision. + assert.equal(all.length, 2); + assert.equal(all[0].stageLabel, 'Staging'); + assert.equal(all[1].stageLabel, 'Production'); +}); + +test('readSettingsFile dedupes by schemaName by default; preserveAllStages keeps per-stage entries (regression — Copilot review)', async (t) => { + // Real-world case: same env var ships under multiple stages with + // different per-stage values. The default (dedupe) is correct for + // "tell me the configured value of X" callers, but VALIDATION must + // see every stage's value to catch a Staging-valid / Production-invalid + // mismatch. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-env-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + const file = path.join(dir, 'deployment-settings.json'); + fs.writeFileSync( + file, + JSON.stringify({ + stages: { + Staging: { + EnvironmentVariables: [ + { SchemaName: 'c311_api_secret', Value: 'https://kv.vault.azure.net/secrets/staging-secret' }, + ], + }, + Production: { + EnvironmentVariables: [ + { SchemaName: 'c311_api_secret', Value: '@KeyVault(vaultName=prod-kv;secretName=secret)' }, + ], + }, + }, + }) + ); + // Default: dedupe — caller sees one value for c311_api_secret. + const deduped = readSettingsFile(file); + assert.equal(deduped.length, 1); + // preserveAllStages: every stage's entry preserved. + const all = readSettingsFile(file, null, { preserveAllStages: true }); + assert.equal(all.length, 2); + const stages = all.map((e) => e.stageLabel).sort(); + assert.deepEqual(stages, ['Production', 'Staging']); +}); + +test('validateSettings catches a Production-only invalid value even when Staging is valid (regression — Copilot review)', async (t) => { + // The end-to-end shape of the bug Copilot flagged: validateSettings + // delegates to readSettingsFile, which deduped by schemaName when the + // same schema appeared in multiple stages. With dedupe, the Staging + // (valid) value won and the Production (invalid) value was silently + // skipped. validateSettings now passes preserveAllStages: true so + // both entries are inspected. + const { validateSettings } = require('../lib/validate-deployment-settings'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-env-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + const file = path.join(dir, 'deployment-settings.json'); + fs.writeFileSync( + file, + JSON.stringify({ + stages: { + Staging: { + EnvironmentVariables: [ + // Valid canonical Key Vault URI + { SchemaName: 'c311_api_secret', Value: 'https://lakeshore-kv.vault.azure.net/secrets/api-secret' }, + ], + }, + Production: { + EnvironmentVariables: [ + // Same schema name; the broken @KeyVault(...) placeholder format + { SchemaName: 'c311_api_secret', Value: '@KeyVault(vaultName=prod-kv;secretName=api-secret)' }, + ], + }, + }, + }) + ); + const result = await validateSettings({ settingsFile: file }); + assert.equal(result.summary.total, 2, 'validator must inspect both stages, not just the first'); + const prodFinding = result.findings.find( + (f) => f.stageLabel === 'Production' && f.schemaName === 'c311_api_secret' + ); + assert.ok(prodFinding, 'expected Production-stage finding to be present'); + assert.equal(prodFinding.valueFormat, 'kv-placeholder'); + assert.equal(prodFinding.status, 'invalid'); +}); + +test('readSettingsFile reads keyed-object Stages shape (Microsoft schema 2024)', async (t) => { + // The Microsoft-standard `deployment-settings/2024` schema and the file + // configure-env-variables emits use a KEYED OBJECT for stages, not an + // array. Real-world discovery against C:/Projects/Citizens portal — pre-fix, + // readSettingsFile returned 0 entries for this shape, which made + // validate-deployment-settings.js silently pass even for known-broken + // values like `@KeyVault(...)`. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-env-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + const file = path.join(dir, 'deployment-settings.json'); + fs.writeFileSync( + file, + JSON.stringify({ + $schema: 'https://schemas.microsoft.com/power-platform/deployment-settings/2024', + stages: { + 'Deploy to Staging': { + EnvironmentVariables: [ + { SchemaName: 'c311_label', Value: 'Staging' }, + { + SchemaName: 'c311_api_secret', + Value: '@KeyVault(vaultName=lakeshore-staging-kv;secretName=api-secret)', + }, + ], + ConnectionReferences: [], + }, + }, + }) + ); + const all = readSettingsFile(file); + assert.equal(all.length, 2); + assert.equal(all[0].schemaName, 'c311_label'); + assert.equal(all[0].stageLabel, 'Deploy to Staging'); + assert.equal(all[1].schemaName, 'c311_api_secret'); + // stageLabel filter (case-insensitive on the stage-name key) + const filtered = readSettingsFile(file, 'deploy to staging'); + assert.equal(filtered.length, 2); + assert.equal(filtered[0].stageLabel, 'Deploy to Staging'); + // Unknown stage label → empty + assert.equal(readSettingsFile(file, 'nope').length, 0); +}); + test('readSettingsFile throws on missing file', () => { assert.throws(() => readSettingsFile('/tmp/nonexistent-deployment-settings.json')); }); diff --git a/plugins/power-pages/skills/add-ai-webapi/SKILL.md b/plugins/power-pages/skills/add-ai-webapi/SKILL.md index 858319ff3..97689d2a9 100644 --- a/plugins/power-pages/skills/add-ai-webapi/SKILL.md +++ b/plugins/power-pages/skills/add-ai-webapi/SKILL.md @@ -140,7 +140,17 @@ When the user asks for follow-up UI changes to an already-integrated AI surface: end of the session, when the user has finished tweaking. - **Do NOT automatically `git commit`.** Let the user batch related tweaks into a single commit. - **Batch the deployment and commit into a single end-of-session prompt** once the user signals - they're done (or when you've completed the last requested change). Use `AskUserQuestion`: + they're done (or when you've completed the last requested change). + + + +> 🚦 **Gate (consent · add-ai-webapi:iter.deploy-commit):** End-of-iteration batched deploy + commit prompt — avoids a noisy per-tweak upload/commit cadence. +> +> **Trigger:** User signals they're done with UI tweaks for the session. +> **Why we ask:** Auto-deploying or committing after every small edit produces one `git commit` + one `pac pages upload-code-site` per tweak; batching keeps history readable and avoids redundant deploys. +> **Cancel leaves:** Nothing — source files already edited; no deploy or commit fired. + + Use `AskUserQuestion`: | Question | Header | Options | |----------|--------|---------| @@ -440,7 +450,17 @@ Wait for it to complete. Then re-check `.powerpages-site/web-roles/` before proc On **Skip**: this puts the run on a known-broken path — the AI endpoints will return 403 at runtime until the user manually creates a web role + table permissions. Don't fall through -silently. Confirm the trade-off with a second `AskUserQuestion`: +silently. + + + +> 🚦 **Gate (consent · add-ai-webapi:4.2.skip-webrole):** Explicit acknowledgement before continuing without a web role — skipping leaves Layer 1/2 broken at runtime. +> +> **Trigger:** User chose "Skip" on the web-role creation offer. +> **Why we ask:** Proceeding silently means the AI API endpoints return 403 at runtime; surfacing the trade-off lets the user make an informed stop-vs-continue choice. +> **Cancel leaves:** Nothing — no code written yet. + +Confirm the trade-off with a second `AskUserQuestion`: | Question | Header | Options | |----------|--------|---------| @@ -547,7 +567,17 @@ files and verifying each resolved call uses the real entity set and id. ### 5.5 Offer to commit Don't commit automatically — on iterative runs an unprompted `git commit` here creates a noisy -series of commits for what is effectively one set of changes. Use `AskUserQuestion`: +series of commits for what is effectively one set of changes. + + + +> 🚦 **Gate (consent · add-ai-webapi:5.5.commit):** Explicit commit decision after Phase 5 summarization-service + UI wiring is complete. +> +> **Trigger:** All Phase 5 targets have been wired (service, framework wrapper, UI call sites). +> **Why we ask:** Auto-committing on every integration run creates noisy one-commit-per-tweak history; letting the user batch is safer. +> **Cancel leaves:** Nothing — source files written; no `git commit` fired. + +Use `AskUserQuestion`: | Question | Header | Options | |----------|--------|---------| @@ -617,6 +647,14 @@ than attempting to create Layer 1/2 files here. ### 6.4 Offer to commit + + +> 🚦 **Gate (consent · add-ai-webapi:6.4.commit):** Explicit commit decision after `Summarization/*` site settings are created by the architect. +> +> **Trigger:** `ai-webapi-settings-architect` has written all `Summarization/Data/Enable` + `Summarization/prompt/` YAMLs. +> **Why we ask:** Auto-committing could bundle dirty pre-existing YAMLs into the commit; explicit consent scopes the commit to just the architect's output. +> **Cancel leaves:** Nothing — YAML files written to disk; no `git commit` fired. + Use `AskUserQuestion`: | Question | Header | Options | diff --git a/plugins/power-pages/skills/add-cloud-flow/SKILL.md b/plugins/power-pages/skills/add-cloud-flow/SKILL.md index 845884288..273d78865 100644 --- a/plugins/power-pages/skills/add-cloud-flow/SKILL.md +++ b/plugins/power-pages/skills/add-cloud-flow/SKILL.md @@ -76,6 +76,14 @@ Look for the `.powerpages-site` folder in the project root. **If not found**: + + +> 🚦 **Gate (plan · add-cloud-flow:1.3.deploy-first):** `.powerpages-site` missing — cloud flow YAML lives inside it. Deploy first or stop. +> +> **Trigger:** Phase 1.3 found no `.powerpages-site` directory. +> **Why we ask:** Cloud flow YAML written to a non-existent path will never deploy. +> **Cancel leaves:** Nothing — no YAML files written. + Use `AskUserQuestion`: | Question | Options | @@ -190,6 +198,14 @@ Already registered (available for additional frontend integration): 3. Support Ticket Handler — Already connected, can be wired into more pages ``` + + +> 🚦 **Gate (plan · add-cloud-flow:3.1.select-flows):** Multi-select over discovered + already-registered flows. Drives the rest of Phases 4–7. +> +> **Trigger:** Phase 2 list-cloud-flows returned at least one flow. +> **Why we ask:** Wrong flows get registered (new `.cloudflowconsumer.yml` files written) or wrong existing flows get re-wired into the frontend. +> **Cancel leaves:** Nothing — no YAML or client code written yet. + Use `AskUserQuestion`: | Question | Options | @@ -218,6 +234,8 @@ For each selected flow, identify its scenario from the name and description: | **Admin action** | Bulk processing, content approval, data export | Admins / specific roles only | | **Background / system** | Scheduled sync, enrichment | Not triggered by portal users directly | + + If a flow's scenario is unclear, use `AskUserQuestion` per flow: | Question | Context | @@ -325,7 +343,6 @@ Assemble the plan JSON (kept in memory — not written to disk). Include all sel ``` For **`integration-only`** flows, `webRoles` should reflect the existing roles from the `.cloudflowconsumer.yml` (read-only — not being changed). The `rationale` should describe where the flow will be additionally integrated (e.g., "Wiring existing Contact Form flow into the support page"). -``` ### 5.2 Render HTML Plan @@ -343,6 +360,14 @@ Open the rendered file in the default browser (`open` on macOS, `start` on Windo Give a brief CLI summary: number of flows, scenarios, role count, any anonymous-role warnings. + + +> 🚦 **Gate (plan · add-cloud-flow:5.3.plan-approval):** Final sign-off on the rendered HTML plan before any web role / `.cloudflowconsumer.yml` / client code is written. +> +> **Trigger:** Phase 5.2 rendered the HTML plan. +> **Why we ask:** Wrong web role assignments committed (especially Anonymous Users on auth-protected flows); orphaned YAML files in `.powerpages-site/cloud-flow-consumer/`. +> **Cancel leaves:** Nothing — no YAML or frontend changes yet. + Use `AskUserQuestion`: | Question | Options | @@ -583,12 +608,28 @@ Use `--skillName "AddCloudFlow"`. ### 8.4 Ask to Deploy + + +> 🚦 **Gate (plan · add-cloud-flow:8.4.deploy):** Post-implementation deploy prompt — flows don't trigger until deployed. +> +> **Trigger:** All flow YAML + client integration in place. +> **Why we ask:** Auto-deploy picks wrong env. +> **Cancel leaves:** Nothing — artifacts stay on disk; no deploy fired. + Use `AskUserQuestion`: | Question | Options | |----------|---------| | Everything is ready. Deploy the site to make the flows live? | Yes, deploy now (Recommended), No, I'll deploy later | + + +> 🚦 **Gate (plan · add-cloud-flow:8.4.test):** Post-deploy validation prompt — invokes `/test-site` to confirm the flow trigger endpoint returns 202/200. +> +> **Trigger:** Deploy from the previous gate succeeded. +> **Why we ask:** Skipping is harmless (manual test still possible); auto-invoking `/test-site` adds runtime and Playwright traffic. +> **Cancel leaves:** Nothing — deploy has already completed. + **If "Yes"**: Invoke `/deploy-site`. After it succeeds, use `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/add-sample-data/SKILL.md b/plugins/power-pages/skills/add-sample-data/SKILL.md index 2e52ffac8..67ed908b7 100644 --- a/plugins/power-pages/skills/add-sample-data/SKILL.md +++ b/plugins/power-pages/skills/add-sample-data/SKILL.md @@ -80,10 +80,26 @@ Show the user the list of discovered tables with their columns so they can choos ### 3.1 Select Tables + + +> 🚦 **Gate (plan · add-sample-data:3.1.tables):** Multi-select over discovered tables — decides which tables get populated with sample data. +> +> **Trigger:** Phase 2 discovery completed. +> **Why we ask:** Auto-selecting all tables can pollute production-shape tables (e.g. `contact`) with throwaway records. +> **Cancel leaves:** Nothing — no record inserts yet. + Use `AskUserQuestion` to ask which tables they want to populate (use `multiSelect: true`). List all discovered tables as options. ### 3.2 Select Record Count + + +> 🚦 **Gate (plan · add-sample-data:3.2.count):** Pick records-per-table (5 / 10 / 25 / Custom). Custom branch is a follow-up free-text prompt under the same gate. +> +> **Trigger:** Tables selected in 3.1. +> **Why we ask:** Inserting hundreds of records can blow past Dataverse storage / API throttling budgets. +> **Cancel leaves:** Nothing — no record inserts yet. + Use `AskUserQuestion` to ask how many sample records per table: | Option | Description | diff --git a/plugins/power-pages/skills/add-seo/SKILL.md b/plugins/power-pages/skills/add-seo/SKILL.md index b9ff322e5..6282df42f 100644 --- a/plugins/power-pages/skills/add-seo/SKILL.md +++ b/plugins/power-pages/skills/add-seo/SKILL.md @@ -73,6 +73,8 @@ Build a list of all routes (e.g., `/`, `/about`, `/contact`, `/blog`). ### Actions + + Use `AskUserQuestion` to collect SEO preferences: #### Call 1 @@ -82,6 +84,8 @@ Use `AskUserQuestion` to collect SEO preferences: | What is the production URL for your site? (e.g., ) | Site URL | *(free text — use single generic option so user types via "Other")* | | Which pages should be excluded from search engine indexing? | Exclusions | None — index all pages (Recommended), Admin/auth pages only, Let me specify | + + #### Call 2 | Question | Header | Options | @@ -111,6 +115,14 @@ Present the SEO additions that will be made as a clear, inline summary: 3. **Meta tags to add to index.html** — title, description, viewport, charset, Open Graph, Twitter Card 4. **Favicon** — link tag and placeholder SVG + + +> 🚦 **Gate (plan · add-seo:3.plan-approval):** Final sign-off on the SEO additions before any `robots.txt` / `sitemap.xml` / `index.html` write. +> +> **Trigger:** Phase 3 has presented the full plan inline (robots.txt, sitemap.xml, meta tags, favicon). +> **Why we ask:** SEO assets land on disk with the wrong production URL, wrong exclusions, or unwanted OG tags — fixable but noisy in git history. +> **Cancel leaves:** Nothing — no file writes yet. + After presenting the plan, use `AskUserQuestion` to get approval: | Question | Header | Options | diff --git a/plugins/power-pages/skills/add-server-logic/SKILL.md b/plugins/power-pages/skills/add-server-logic/SKILL.md index c08b77f5c..5aa8ec00f 100644 --- a/plugins/power-pages/skills/add-server-logic/SKILL.md +++ b/plugins/power-pages/skills/add-server-logic/SKILL.md @@ -103,6 +103,14 @@ Look for the `.powerpages-site` folder: > "The `.powerpages-site` folder was not found. Server logic files are stored inside this folder, so the site must be deployed at least once before creating server logic. Would you like to deploy now?" + + +> 🚦 **Gate (plan · add-server-logic:1.5.deploy-first):** `.powerpages-site` missing — server logic files live inside it. Deploy first or stop. +> +> **Trigger:** Phase 1.5 found no `.powerpages-site` directory. +> **Why we ask:** Server logic `.js`/`.yml` files written to a non-existent path won't deploy. +> **Cancel leaves:** Nothing — no server logic files written yet. + Use `AskUserQuestion`: | Question | Options | @@ -182,6 +190,14 @@ Each entry includes: `name`, `displayName`, `description`, `type` (`action` or ` If custom actions are found (`total > 0`), present a summary to the user grouped by binding type (unbound vs. entity-bound) and ask whether any should be used: + + +> 🚦 **Gate (plan · add-server-logic:2.1.2.use-custom-actions):** Custom actions discovered — decide whether to wrap existing Dataverse Custom APIs/Process Actions or build server logic from scratch. Choice changes the Phase 5 implementation shape. +> +> **Trigger:** `list-custom-actions.js` returned at least one entry. +> **Why we ask:** Auto-wrapping could attach the wrong action; auto-skipping duplicates logic that already exists in Dataverse. +> **Cancel leaves:** Nothing — no server logic files written yet. + Use `AskUserQuestion`: | Question | Options | @@ -196,6 +212,8 @@ If the user says **No**, skip to Phase 2.2. If the user says **Yes**, for each server logic item being created, ask which custom action (if any) it should wrap: + + Use `AskUserQuestion` for each server logic item: | Question | Context | @@ -248,6 +266,14 @@ These values will be used in Phase 7 to create the environment variables and sit If secrets were identified in Phase 2.3, ask the user now whether they want to use Azure Key Vault. This decision must happen before Phase 4 so the implementation plan can show the chosen secret management approach. + + +> 🚦 **Gate (plan · add-server-logic:2.3.1.keyvault):** Pick secret-storage mechanism (Key Vault vs plain env var). Choice changes the Phase 4 rendered plan and the Phase 7 implementation pipeline. +> +> **Trigger:** Phase 2.3 identified at least one secret value. +> **Why we ask:** Plain env var creation can expose secrets in solution exports; auto-picking Key Vault forces additional Azure setup. +> **Cancel leaves:** Nothing — no env var definitions written yet. + Use `AskUserQuestion`: | Question | Options | @@ -258,6 +284,8 @@ Record the user's choice — it will be shown in the HTML plan (Phase 4) and exe ### 2.4 Confirm with User + + If the requirements are ambiguous, use `AskUserQuestion` to clarify: | Question | Context | @@ -367,6 +395,14 @@ Do not restate the per-server-logic breakdown, rationale, role assignments, or f ### 4.4 Confirm with User + + +> 🚦 **Gate (plan · add-server-logic:4.4.plan-approval):** Final sign-off on the rendered HTML plan before Phase 5 writes any `.serverlogic.yml` / `.js` files or Phase 7 creates env vars. +> +> **Trigger:** Phase 4.2 rendered the HTML plan; Phase 4.3 surfaced the CLI summary. +> **Why we ask:** Server logic files committed under wrong names / wrong roles; env var definitions created against the wrong secret-storage mode. +> **Cancel leaves:** Nothing — no server logic files written yet. + Use `AskUserQuestion`: | Question | Options | @@ -846,6 +882,8 @@ The script outputs a JSON array of Key Vaults (`name`, `resourceGroup`, `locatio If Key Vaults were found, present the list and ask which one to use: + + Use `AskUserQuestion`: | Question | Context | @@ -854,6 +892,14 @@ Use `AskUserQuestion`: If **no Key Vaults are found**, ask the user how to proceed: + + +> 🚦 **Gate (plan · add-server-logic:7.2a.no-vaults):** No Key Vaults found in the user's subscription — create one or fall back to plain env vars. Branches the secret-storage flow. +> +> **Trigger:** Phase 2.3.1 chose Key Vault but `list-azure-keyvaults.js` returned an empty list. +> **Why we ask:** Auto-creating a Key Vault provisions Azure resources without explicit consent; auto-falling-back stores secrets as plain env vars after the user explicitly opted in to Key Vault. +> **Cancel leaves:** Nothing — no Azure or Dataverse writes yet. + Use `AskUserQuestion`: | Question | Options | @@ -862,6 +908,8 @@ Use `AskUserQuestion`: **If "Create a new Key Vault"**: Ask for a vault name, resource group, and location, then create it: + + Use `AskUserQuestion`: | Question | Context | @@ -1040,6 +1088,14 @@ Server logic creates the backend — but without frontend code to call it, the e ### 9.1 Ask User About Integration Scope + + +> 🚦 **Gate (plan · add-server-logic:9.1.frontend-scope):** Decide whether the skill also wires the server logic into the frontend UI or stops at the backend. +> +> **Trigger:** Phase 8 completed (server logic deployed-ready). +> **Why we ask:** Auto-integrating mutates UI files the user wanted to handle themselves; auto-skipping leaves the endpoints unreachable from the app. +> **Cancel leaves:** Nothing — server logic backend is already on disk; this prompt only decides frontend follow-through. + Use `AskUserQuestion`: | Question | Options | @@ -1170,12 +1226,28 @@ Present a summary of everything that was done: ### 11.3 Ask to Deploy + + +> 🚦 **Gate (plan · add-server-logic:11.3.deploy):** Post-implementation deploy prompt — server logic endpoints aren't reachable until deployed. +> +> **Trigger:** All server logic artifacts written and committed. +> **Why we ask:** Auto-deploy picks wrong env. +> **Cancel leaves:** Nothing — artifacts stay on disk; no deploy fired. + Use `AskUserQuestion`: | Question | Options | |----------|---------| | The server logic work is ready. To make it live, the site needs to be deployed. Would you like to deploy now? | Yes, deploy now (Recommended), No, I'll deploy later | + + +> 🚦 **Gate (plan · add-server-logic:11.3.test):** Post-deploy validation prompt — invokes `/test-site` to exercise the new endpoints live. +> +> **Trigger:** Deploy from the previous gate succeeded. +> **Why we ask:** Skipping is harmless (manual test still possible); auto-invoking `/test-site` adds runtime. +> **Cancel leaves:** Nothing — deploy has already completed. + **If "Yes, deploy now"**: Invoke the `/deploy-site` skill to deploy the site. After deployment succeeds, use `AskUserQuestion`: diff --git a/plugins/power-pages/skills/audit-permissions/SKILL.md b/plugins/power-pages/skills/audit-permissions/SKILL.md index 505ad79c4..435042c3b 100644 --- a/plugins/power-pages/skills/audit-permissions/SKILL.md +++ b/plugins/power-pages/skills/audit-permissions/SKILL.md @@ -489,6 +489,14 @@ Follow the skill tracking instructions in the reference to record this skill's u ### 6.2 Present Summary + + +> 🚦 **Gate (plan · audit-permissions:6.fix-offer):** Offer to apply auto-fixes for critical/warning findings. The audit report has already been written; declining here just leaves the HTML report in place — no Dataverse / filesystem mutation. +> +> **Trigger:** Phase 6 has tallied findings and the HTML report is saved. +> **Why we ask:** Tooling could silently invoke the table-permissions-architect agent — accept-by-default would write or mutate permission YAML against the user's intent. +> **Cancel leaves:** Nothing — the audit report stays at its saved path. No web-role / table-permission files written. + Present a summary to the user: 1. **Critical findings count** — these need immediate attention diff --git a/plugins/power-pages/skills/create-site/SKILL.md b/plugins/power-pages/skills/create-site/SKILL.md index 3c4cae095..d112db2b5 100644 --- a/plugins/power-pages/skills/create-site/SKILL.md +++ b/plugins/power-pages/skills/create-site/SKILL.md @@ -35,6 +35,8 @@ Guide the user through creating a complete, production-quality Power Pages code ## Live Preview Status Protocol + + While the scaffold loading screen is visible (from Phase 2.6 until the Home page itself is replaced in Phase 5), the loader polls `GET /scaffold-status.json` every 1.5 seconds. The `message` you write into `/public/scaffold-status.json` appears as the label under the progress bar, and `awaitingInput` controls the "waiting for your input" banner. The decorative spinner above the progress bar continues its built-in phrase cycle; keep the progress-bar label current so the loader still reflects what is actually happening. **Why this matters**: When the browser with the loader takes over the user's screen, a prompt in the terminal can sit unanswered for a long time because the user doesn't realize anything is waiting. The banner makes it obvious. @@ -70,6 +72,14 @@ Write the file with the `Write` tool (atomic overwrite). You do not need to read **Actions**: + + +> 🚦 **Gate (plan · create-site:1.purpose):** Multi-question prompt collecting site name, framework, purpose, audience, and target directory. Determines what gets scaffolded. Fires only on the "site purpose unclear" branch (step 3 below). +> +> **Trigger:** Phase 1 when site purpose was not provided in `$ARGUMENTS`. +> **Why we ask:** Wrong framework picked → wrong template copied into the wrong directory; cleanup is annoying. +> **Cancel leaves:** Nothing — no scaffolding has started yet. + 1. Create todo list with all 8 phases (see [Progress Tracking](#progress-tracking) table) 2. If site purpose is clear from arguments: - Summarize understanding @@ -217,6 +227,14 @@ Immediately after the dev server starts, verify the scaffold is working: **Goal**: Determine what pages, components, and design elements the site needs — while the user previews the running scaffold + + +> 🚦 **Gate (plan · create-site:3.requirements):** Three sub-prompts (features multi-select, aesthetic, mood) — shape the Phase 4 plan and the Phase 5 implementation. Fires at step 2 of the action list below. +> +> **Trigger:** Phase 3 entry; scaffold loader is up. +> **Why we ask:** Wrong feature set / aesthetic gets baked into the rendered plan — the Phase 4.7 gate would still catch most errors, but it's wasteful to defer the catch. +> **Cancel leaves:** Nothing — scaffold loader files are throwaway artifacts replaced wholesale in Phase 5. + **Actions**: 1. **Raise the "awaiting input" banner** so the user notices the terminal prompt even while the browser loader is full-screen. `Write` `/public/scaffold-status.json`: @@ -379,6 +397,14 @@ Immediately after the user answers, `Write` the same file again with `"awaitingI ### 4.7 Ask for Approval + + +> 🚦 **Gate (plan · create-site:4.7.plan-approval):** Final sign-off on the rendered HTML plan before Phase 5 starts replacing the scaffold with real pages, components, and design tokens. +> +> **Trigger:** Phase 4.3 rendered `docs/create-site-plan.html`; Phase 4.4 opened it in the browser. +> **Why we ask:** Phase 5 rewrites the entire scaffold (theme.css, Layout, Home page, components, routes) — undoing that touches every commit in the implementation phase. +> **Cancel leaves:** Nothing destructive — the scaffold itself can be deleted with the project directory; no Dataverse / deploy fired. + Use `AskUserQuestion`: | Question | Header | Options | @@ -595,6 +621,14 @@ Present a summary table to the user: **Goal**: Ensure the site meets user expectations and all pages work correctly + + +> 🚦 **Gate (plan · create-site:7.review):** Live-site review — last chance to request changes before the deploy prompt. Cancel branch lets the user keep iterating. Fires at step 4 of the action list below. +> +> **Trigger:** Phase 7 has verified all pages render via Playwright. +> **Why we ask:** User loses the chance to spot UI issues before deploy; broken pages get pushed. +> **Cancel leaves:** Nothing — site files stay as-is on disk. + **Actions**: 1. Browse through each page via Playwright (`browser_navigate` + `browser_snapshot`) to verify all pages load correctly — do NOT take screenshots @@ -624,6 +658,14 @@ Present a summary table to the user: > **This phase is MANDATORY. Do NOT end the session without asking about deployment.** + + +> 🚦 **Gate (plan · create-site:8.deploy):** Deploy prompt — invokes `/deploy-site` on Yes. Skipping leaves the site files on disk for the user to deploy later. Fires at step 2 of the action list below. +> +> **Trigger:** Phase 8 entry; Phase 7 review approved. +> **Why we ask:** Auto-deploy picks whatever env PAC CLI happens to be pointing at — wrong-env first deploy is messy to undo. +> **Cancel leaves:** Nothing — site files stay on disk; no deploy fired. + **Actions**: 1. Record skill usage: diff --git a/plugins/power-pages/skills/create-webroles/SKILL.md b/plugins/power-pages/skills/create-webroles/SKILL.md index d3bd59522..2f19224fb 100644 --- a/plugins/power-pages/skills/create-webroles/SKILL.md +++ b/plugins/power-pages/skills/create-webroles/SKILL.md @@ -65,6 +65,14 @@ regression guard — no human invocation changes behavior. 1. Locate the project root (`**/powerpages.config.json`) and check for `.powerpages-site/web-roles/`. + + +> 🚦 **Gate (plan · create-webroles:1.deploy-first):** `.powerpages-site` missing — skill cannot proceed without that folder. Prompt to deploy first or stop. +> +> **Trigger:** Phase 1 found no `.powerpages-site` directory. +> **Why we ask:** Web role YAML files written to a non-existent path will never get picked up by deploy; user thinks roles were created but they weren't. +> **Cancel leaves:** Nothing — no YAML files written. + 2. **If `.powerpages-site` does NOT exist:** - **In caller-suppress mode** (Phase 0 flag): stop with a contract-violation message — the calling skill should have gated on this folder existing before invoking us. @@ -115,6 +123,14 @@ regression guard — no human invocation changes behavior. **Actions**: + + +> 🚦 **Gate (plan · create-webroles:3.role-selection):** Multi-select over suggested + custom web roles. Drives the Phase 4 YAML file writes. +> +> **Trigger:** Phase 2 inventoried existing roles; Phase 3 suggests new ones. +> **Why we ask:** Wrong roles get created locally — fixable but adds churn to the `.powerpages-site/web-roles/` folder. +> **Cancel leaves:** Nothing — no YAML files written yet. + 1. Based on the site's purpose and the existing roles, suggest appropriate web roles. Use `AskUserQuestion` to confirm with the user. Common web roles for Power Pages sites include: @@ -225,6 +241,14 @@ name: > | Content Editors | `a1b2c3d4-...` | false | false | > | *(etc.)* | + + +> 🚦 **Gate (plan · create-webroles:6.deploy):** Final post-create prompt — deploy now to make the new roles take effect, or defer. +> +> **Trigger:** Phase 5 validation succeeded. +> **Why we ask:** Auto-invoking `/deploy-site` would push the site to whatever env PAC CLI happens to point at — wrong-env push is messy to undo. +> **Cancel leaves:** Nothing — the YAML files stay on disk; no deploy fired. + 3. **In caller-suppress mode** (Phase 0 flag): skip the deploy ask and the closing reminder entirely. Return the created-roles summary to the caller and stop. The caller owns the single end-of-orchestration deploy decision; nesting deploy reminders inside delegations diff --git a/plugins/power-pages/skills/deploy-site/SKILL.md b/plugins/power-pages/skills/deploy-site/SKILL.md index 6d446d5e0..25b5dd80f 100644 --- a/plugins/power-pages/skills/deploy-site/SKILL.md +++ b/plugins/power-pages/skills/deploy-site/SKILL.md @@ -81,6 +81,9 @@ Guide the user through deploying an existing Power Pages code site to a Power Pa 3. **If not authenticated**: 1. Inform the user they are not authenticated with PAC CLI. + + + 2. Use `AskUserQuestion` to ask for the environment URL: | Question | Header | Options | @@ -113,6 +116,14 @@ Guide the user through deploying an existing Power Pages code site to a Power Pa **Actions**: + + +> 🚦 **Gate (consent · deploy-site:3.confirm-env):** Echo the current environment and require explicit confirmation before any upload. Covers the follow-up "pick a different env" sub-prompt in the same section — wrong-env deploys are the #1 destructive shared-state failure for this skill, so this gate must fire even when PAC CLI shows a recognizable env. +> +> **Trigger:** Phase 3 entry; environment resolved from PAC CLI. +> **Why we ask:** Site uploaded to wrong tenant / wrong env — committed to a Dataverse instance the user did not intend; cleanup requires manual deletion or another deploy from a different env. +> **Cancel leaves:** Nothing — no upload fired. + 1. Present the current environment information to the user and ask them to confirm. Use `AskUserQuestion` with the following structure: @@ -159,6 +170,14 @@ Determine the project root directory. The project root is the directory containi **/powerpages.config.json ``` + + +> 🚦 **Gate (plan · deploy-site:4.1.multi-project):** More than one `powerpages.config.json` candidate found — pick the right project to deploy. +> +> **Trigger:** Phase 4.1 glob returned multiple matches. +> **Why we ask:** Wrong project uploaded — pollutes the target env with files from a different site. +> **Cancel leaves:** Nothing — no upload fired. + If found in the current working directory or a subdirectory, use that directory as `PROJECT_ROOT`. If multiple are found, ask the user which one to deploy using `AskUserQuestion`. If not found, ask the user to provide the path to the project root. @@ -167,6 +186,14 @@ If not found, ask the user to provide the path to the project root. If `.powerpages-site` already exists (i.e., this is not the first deployment), table permissions and site settings may have drifted from the code since the last deployment. Offer to audit before deploying. + + +> 🚦 **Gate (plan · deploy-site:4.2.audit-permissions):** Re-deployment detected — offer to run `/audit-permissions` to catch drift between code and table permissions before push. +> +> **Trigger:** `.powerpages-site` exists (not first deployment). +> **Why we ask:** Stale permission YAML deploys, causing 403s for users until next audit run. +> **Cancel leaves:** Nothing — audit is read-only; declining just proceeds to build + upload. + Use `AskUserQuestion`: | Question | Header | Options | @@ -254,6 +281,14 @@ Evaluate the JSON result: #### 5.5.1 Ask About Activation (only if site is NOT already activated) + + +> 🚦 **Gate (plan · deploy-site:5.5.1.activate):** Site deployed but not yet activated — offer to invoke `/activate-site` to provision the subdomain. +> +> **Trigger:** Phase 5.5 detected `activated:false` on a fresh deploy. +> **Why we ask:** Auto-activating writes the wrong subdomain (permanent for the site); auto-skipping leaves the site without a live URL. +> **Cancel leaves:** Nothing — no activation API call fired. + Ask the user if they want to activate the site using `AskUserQuestion`: | Question | Header | Options | @@ -269,6 +304,14 @@ After confirming the site is activated (either it was already activated in step **Prerequisites**: The site must be activated and the project root must be known (from Phase 4.1). + + +> 🚦 **Gate (plan · deploy-site:5.6.restart-cache):** Restart the deployed site to flush its runtime cache. Brief production downtime (a few seconds) — explicit user consent required even though the action is recoverable. +> +> **Trigger:** Site confirmed activated (either pre-existing or just activated in 5.5.1). +> **Why we ask:** Production users see stale content for several minutes until cache TTL expires. +> **Cancel leaves:** Nothing — the deploy itself already succeeded; cache will refresh on its own. + Use `AskUserQuestion` to confirm before proceeding: | Question | Header | Options | @@ -307,6 +350,14 @@ Tell the user: ### 6.2 Ask for Permission + + +> 🚦 **Gate (consent · deploy-site:6.2.unblock-js):** Reactive `blockedattachments` modification — destructive shared-state change (tenant-wide env setting). Same shape as `deploy-pipeline:7.6.2.blocked-attachments`. +> +> **Trigger:** Upload failed with the blocked-`.js` error. +> **Why we ask:** Auto-unblocking modifies a tenant security setting without explicit consent — visible across the entire environment, not just this site. +> **Cancel leaves:** `attachment-block-modified` is only possible if the user approved and then a downstream step partial-completed. Pure Cancel here leaves nothing — the original blockedattachments value is untouched. + Use `AskUserQuestion`: | Question | Header | Options | diff --git a/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md b/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md index 6852386bc..03866e39c 100644 --- a/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md +++ b/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md @@ -422,6 +422,22 @@ A PE already exists in the tenant (one is provisioned automatically the first ti #### 3.C — Status `NoHost` (host-type decision tree) + + +> 🚦 **Gate (plan · ensure-pipelines-host:3.C.host-type):** Top-level host-type prompt — pick Platform Host / Custom Host / PPAC manual / Manual export-import strategy / Cancel. Drives the rest of Phase 3 and Phase 4 routing. +> +> **Trigger:** Status `NoHost` AND no upstream `hostResolution.willProvision*` flag carries the answer. +> **Why we ask:** Auto-picking provisions an env (PE or Custom Host) without consent; PE is tenant-singleton and admin-non-deletable. +> **Cancel leaves:** Nothing — no provisioning fired yet. + + + +> 🚦 **Gate (plan · ensure-pipelines-host:3.C.env-pick):** Sub-prompt under 3.C top-level option "Custom Host" — present the eligible-env list (capped at 5) with role labels (`dev env`, `source env`, `staging env`, `production env`) and the "Other (paste URL)" fallback. Fires only on the Custom-Host branch of the host-type menu. +> +> **Trigger:** User picked "Custom Host" in 3.C top-level. +> **Why we ask:** Auto-picking the wrong env routes pipelines through a host the user didn't intend. +> **Cancel leaves:** Nothing — no app install fired. + The prompt asks the user to pick the **host type** first (Platform Host, Custom Host, PPAC manual, or cancel). Picking Custom Host opens a sub-prompt for the install method (existing env vs. create-new). The Platform-Host path is the lowest-friction default and is presented first. **Skip rule — caller already collected the answer.** When this skill is invoked from `setup-pipeline` and `docs/alm/last-pipeline.json` carries a `hostResolution` block populated by plan-alm Phase 2 Q4, inspect those flags before showing the prompt: @@ -570,11 +586,19 @@ Surface the specific failure to the user. Out of automated remediation scope. Re #### 4.0 — Fast-path: Platform Host via `getOrCreate` + + +> 🚦 **Gate (consent · ensure-pipelines-host:4.0.pre-call):** Echo the tenant identity in the request body before firing the BAP `getOrCreate` Platform Host call. NON-SKIPPABLE even when upstream `hostResolution.willProvisionPlatform === true`. PE is tenant-singleton and admin-non-deletable — this gate is the principal wrong-tenant mitigation. +> +> **Trigger:** Phase 4.0 entry (routed in from 3.C or upstream). +> **Why we ask:** PE provisioned in wrong tenant; cannot be deleted by tenant admin. +> **Cancel leaves:** Nothing — no provisioning fired yet. + The lowest-friction host-provisioning path. Calls the idempotent BAP `getOrCreate` endpoint with a `D365_1stPartyAdminApps` + `Platform` body. Same call `make.powerapps.com → Pipelines` page makes when a user clicks "Get started" — we just invoke it directly. Spec from `useGetOrCreatePlatformEnvironment.v4.ts`. New helper `provision-platform-host.js`. **No sub-prompts.** BAP picks tenant home geo + default display name; no admin role required. -**Pre-call confirmation (NON-SKIPPABLE single consent gate):** +**Pre-call confirmation via `AskUserQuestion`** — NON-SKIPPABLE single consent gate: > "About to provision a Platform Host for tenant **{TENANT_DISPLAY_NAME}** (`{tenantId}`). > @@ -605,15 +629,31 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/lib/provision-platform-host.js" \ #### 4.A — Fast-path: Custom Host via `D365_ProjectHost` template + + +> 🚦 **Gate (consent · ensure-pipelines-host:4.A.pre-call):** Echo the BAP env-create request body (region + display name + template) before firing the Custom Host create call. NON-SKIPPABLE even when upstream `hostResolution.willProvisionCustom === true`. Last chance to catch a wrong-tenant/wrong-region provisioning. Bug fixed 2026-05-05 was caused by skipping this gate. +> +> **Trigger:** Phase 4.A entry; sub-prompts (display name, region, admin attestation) collected. +> **Why we ask:** Custom Host provisioned in wrong tenant or wrong region; consumes Azure capacity quota; potentially attributes the env to the wrong organization. +> **Cancel leaves:** Nothing — no provisioning fired yet. + + + +> 🚦 **Gate (consent · ensure-pipelines-host:4.sandbox-confirm):** When the picked install-target env has `environmentSku=Sandbox`, surface the sandbox-limits warning and require explicit re-confirmation before installing. Fires once per Sandbox-SKU env selected. +> +> **Trigger:** Phase 4.B / Phase 3.C sub-option a — picked env is Sandbox SKU. +> **Why we ask:** Installing Pipelines onto a Sandbox env without consent — sandbox envs are deletable on inactivity and have reduced capacity; pipelines may break unexpectedly. +> **Cancel leaves:** Nothing — no app install fired. + Standard env-create API with the `D365_ProjectHost` template (eng.ms-documented; same template PPAC `New custom host` uses internally). New helper `provision-custom-host.js`. -**Sub-prompts (collected before the API call):** +**Sub-prompts (collected before the API call) via `AskUserQuestion`:** 1. Display name (default suggestion: `"{tenant displayName} Pipelines Host"`) 2. Region (default: tenant home geo from BAP `tenant` endpoint; offer override) 3. Confirm caller is admin — single AskUserQuestion *"Are you a Global / Power Platform / Dynamics admin in this tenant? Yes / No / Not sure"*. If No or Not sure, recommend Path 4.B/4.C and fall back. -**Pre-call confirmation (NON-SKIPPABLE second consent gate):** +**Pre-call confirmation via `AskUserQuestion`** — NON-SKIPPABLE second consent gate: > "About to call `POST https://api.bap.microsoft.com/providers/Microsoft.BusinessAppPlatform/environments?api-version=2021-04-01` for tenant `{tenantId}` with body: > ```json @@ -751,11 +791,19 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/lib/install-pipelines-app.js" \ #### 4.C — Guided manual: PPAC `New custom host` + + +> 🚦 **Gate (progress · ensure-pipelines-host:4.C.ppac-done):** External-system wait gate — user must complete the PPAC "New custom host" flow manually, then return to confirm. Skill polls BAP afterward for the new env. +> +> **Trigger:** Phase 4.C entry; PPAC URL printed. +> **Why we ask:** Skill proceeds without confirming the new env exists; downstream `setup-pipeline` then fails with no host. Worse, if PPAC creation succeeded partway and the user cancelled the gate, the host binding is partially established (a `D365_ProjectHost`-templated env exists in tenant). +> **Cancel leaves:** `host-binding` — a manually-created env may be left in tenant; user must clean up via PPAC. + 1. Print: `https://admin.powerplatform.microsoft.com/deployments` and instructions: *"Click 'New custom host' → fill name (suggested: '{tenant} Pipelines Host') → choose Production environment in tenant home region → Create. Provisioning takes 5–10 min."* > Per eng.ms doc: *"the panel will default to the Production environment type. Adding Dataverse is also required... template and sample apps options are hidden here, as we use a specific organization template for this scenario."* (The template is `D365_ProjectHost` — same one Path 4.A automates.) -2. Two-option AskUserQuestion: *"Done — provisioning kicked off"* / *"Cancel"*. +2. Two-option `AskUserQuestion`: *"Done — provisioning kicked off"* / *"Cancel"*. 3. After confirmation, poll BAP `list-tenant-envs.js` every 15s looking for a new env with the Pipelines marker. On detection, capture URLs, `actionTaken = "user-created-custom-ppac"`. Proceed to Phase 5. #### Common: Timeout handling diff --git a/plugins/power-pages/skills/export-solution/SKILL.md b/plugins/power-pages/skills/export-solution/SKILL.md index 61ac541bb..1e53baea9 100644 --- a/plugins/power-pages/skills/export-solution/SKILL.md +++ b/plugins/power-pages/skills/export-solution/SKILL.md @@ -129,10 +129,20 @@ Cap this step at ~30 seconds. If MCP search / fetch errors out, log a one-line n ### Phase 2 — Identify Solution + + +> 🚦 **Gate (plan · export-solution:2.identify):** No `.solution-manifest.json` in project root — user must pick or paste a solution unique name before export proceeds. Fires only on the "not found" branch (step 3 below). +> +> **Trigger:** Phase 2 step 1 didn't find a manifest. +> **Why we ask:** Auto-picking the wrong solution exports a managed zip that ships the wrong table/site/flow set to staging. +> **Cancel leaves:** Nothing — no ExportSolutionAsync call yet. + 1. Look for `.solution-manifest.json` in project root (use `findProjectRoot` or `glob('**/.solution-manifest.json')`) 2. If found: read `solution.uniqueName`, `solution.solutionId`, `environmentUrl` - Verify environment URLs match (warn if different — may be cross-environment export) -3. If not found: ask user for solution unique name via `AskUserQuestion` +3. If not found, use `AskUserQuestion` to pick the solution: + - Query Dataverse for available unmanaged solutions and present them as options + - Free-text fallback ("Other") for pasting the unique name directly 4. Confirm solution exists in environment: ``` GET {envUrl}/api/data/v9.2/solutions?$filter=uniquename eq '{solutionName}'&$select=solutionid,uniquename,friendlyname,version,ismanaged @@ -225,8 +235,17 @@ Invoke `AskUserQuestion` immediately — do NOT describe this choice as chat tex Use the answer to set `"Managed": true` or `"Managed": false` in the `ExportSolutionAsync` request body. -Also ask (separate `AskUserQuestion`): + + +> 🚦 **Gate (plan · export-solution:3.overwrite):** Output directory and overwrite-vs-new-name decision for the produced zip. If an existing zip is detected at the target path, the prompt offers Overwrite / pick new name / cancel. +> +> **Trigger:** Phase 3 after Managed/Unmanaged is picked. +> **Why we ask:** Auto-overwriting replaces a previous export that may have been hand-tested already. +> **Cancel leaves:** Nothing — no zip written. + +Also ask via `AskUserQuestion`: - Output directory (default: current project root) +- If a zip already exists at the resolved output path: *"Overwrite / Pick new name / Cancel"* ### Phase 4 — Trigger Async Export diff --git a/plugins/power-pages/skills/force-link-environment/SKILL.md b/plugins/power-pages/skills/force-link-environment/SKILL.md index 6d4069e28..0083f44df 100644 --- a/plugins/power-pages/skills/force-link-environment/SKILL.md +++ b/plugins/power-pages/skills/force-link-environment/SKILL.md @@ -88,8 +88,21 @@ Confirm the *"Using Force Link…"* section's current warnings before proceeding ## Phase 2 — Identify host + dev env - - + + +> 🚦 **Gate (plan · force-link-environment:2.host-url):** Pick the target host environment URL when arg / marker resolution paths all came up empty. Fires only on the "no `--host` arg, no `last-host-check.json`, no `last-pipeline.json`" branch (step 4 below). +> +> **Trigger:** Phase 2 resolution order steps 1–3 all returned no value. +> **Why we ask:** Auto-picking the wrong host runs `ManageEnvironmentStamp` against the wrong tenant and moves the stamp irreversibly without consent. +> **Cancel leaves:** Nothing — no API call yet. + + + +> 🚦 **Gate (plan · force-link-environment:2.dev-env):** Pick (or paste) the source dev env's BAP env GUID when `--dev-env` arg is absent and `pac env who` didn't confirm. Fires only on the "no arg + no confirmation" branch (step 3 below). +> +> **Trigger:** Phase 2 BAP-GUID resolution steps 1–2 all returned no value. +> **Why we ask:** Auto-picking the wrong dev env relinks a different env to the new host — makers of the wrong env lose pipeline access. +> **Cancel leaves:** Nothing — no API call yet. Resolution order for `hostEnvUrl`: 1. `--host ` argument, if supplied. diff --git a/plugins/power-pages/skills/integrate-backend/SKILL.md b/plugins/power-pages/skills/integrate-backend/SKILL.md index ea26144b5..e14dfb7f5 100644 --- a/plugins/power-pages/skills/integrate-backend/SKILL.md +++ b/plugins/power-pages/skills/integrate-backend/SKILL.md @@ -104,6 +104,8 @@ From the user's request and the existing site state, determine: ### 2.2 Clarify if Ambiguous + + If the request could map to multiple approaches and the right choice isn't clear, use `AskUserQuestion` to clarify: | Question | When to ask | @@ -280,7 +282,6 @@ Prepare a JSON object with these keys: { "approach": "serverlogic", "name": "Validate Transition", "detail": "Checks Draft→Submitted" }, { "approach": "webapi", "name": "Update Status", "detail": "PATCH status to Submitted" } ``` -``` Write the plan to `/docs/backend-plan.html` (create `docs/` if needed). Use the render script: @@ -310,6 +311,14 @@ In the CLI, give only a brief summary: ### 3.4 Confirm with User + + +> 🚦 **Gate (plan · integrate-backend:3.4.plan-approval):** Approve the integration plan before invoking the appropriate child skill (`integrate-webapi` / `add-server-logic` / `add-cloud-flow`). The plan HTML stays on disk regardless of choice — Cancel just stops the dispatch. +> +> **Trigger:** Phase 3.3 has rendered the HTML plan and surfaced a brief CLI summary. +> **Why we ask:** Wrong child skill gets dispatched — `add-server-logic` for a Web API task wastes minutes; `add-cloud-flow` for a Web API task creates orphaned flow YAML. +> **Cancel leaves:** Nothing — no child skill invoked, HTML plan stays at its saved path. + Use `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/integrate-webapi/SKILL.md b/plugins/power-pages/skills/integrate-webapi/SKILL.md index 5ab35eb8c..a4b79d8b1 100644 --- a/plugins/power-pages/skills/integrate-webapi/SKILL.md +++ b/plugins/power-pages/skills/integrate-webapi/SKILL.md @@ -184,6 +184,14 @@ Show the user: ### 3.2 Confirm Tables + + +> 🚦 **Gate (plan · integrate-webapi:3.2.confirm-tables):** Final say on which tables get Web API integration code (client, types, services, hooks). +> +> **Trigger:** Explore agent surfaced candidate tables in Phase 3.1. +> **Why we ask:** Auto-selecting all tables can generate orphaned TypeScript files for tables the user never intended to expose via Web API. +> **Cancel leaves:** Nothing — no service/type/hook files written yet. + **When AI-only read mode is active (Phase 1.6 flag set):** skip this step entirely. Use the `tables` list parsed from the sentinel verbatim — the caller has already confirmed the selection with the user. Do not issue an `AskUserQuestion`. Otherwise, use `AskUserQuestion` to confirm: @@ -325,6 +333,14 @@ Do NOT issue the deploy prompt below — the caller owns the single deploy decis Otherwise, if `.powerpages-site` doesn't exist: + + +> 🚦 **Gate (plan · integrate-webapi:6.1.deploy-first):** `.powerpages-site` missing — needed by both architect agents. Deploy first, or skip permissions setup and finish without them. +> +> **Trigger:** Phase 6.1 found no `.powerpages-site` folder. +> **Why we ask:** Auto-skipping leaves the integration broken (no permissions to back the Web API calls); auto-deploying picks the wrong env. +> **Cancel leaves:** Nothing — services/types/hooks from Phase 4 stay on disk regardless. + Use `AskUserQuestion`: | Question | Options | @@ -337,6 +353,14 @@ Use `AskUserQuestion`: ### 6.2 Choose Permissions Source + + +> 🚦 **Gate (plan · integrate-webapi:6.2.permissions-source):** Decide between uploading an existing permissions diagram (Path A) and letting the architect agents derive it (Path B). Choice routes the rest of Phase 6. +> +> **Trigger:** Entering Phase 6.2 after deployment prerequisite is satisfied. +> **Why we ask:** Path A produces table permissions matching a stale or wrong diagram; Path B can take minutes to query Dataverse. +> **Cancel leaves:** Nothing — no permission YAML written yet. + **When AI-only read mode is active (Phase 1.6 flag set):** skip the permissions-source question entirely and default to **Path B (let the architects figure it out)** — proceed directly to section 6.3. Per the Phase 1.6 contract, no `AskUserQuestion` is issued here; the caller @@ -357,6 +381,14 @@ Route to the appropriate path: #### Path A: Upload Existing Permissions Diagram + + +> 🚦 **Gate (plan · integrate-webapi:6.2.permissions-approval):** Final sign-off on the parsed permissions plan (from the uploaded diagram) before any web-role / table-permission / site-setting YAML write. Fires at step 6 of the Path A sequence below. +> +> **Trigger:** Path A — diagram parsed and Mermaid flowchart rendered. +> **Why we ask:** Wrong scope / wrong CRUD flags get committed to `.powerpages-site/table-permissions/` — fixable but noisy in git history. +> **Cancel leaves:** Nothing — no YAML files written yet. + If the user chooses to upload an existing diagram: 1. Ask the user to provide their permissions diagram. Supported formats: @@ -560,6 +592,14 @@ Present a summary of everything that was done: ### 7.3 Ask to Deploy + + +> 🚦 **Gate (plan · integrate-webapi:7.3.deploy):** Post-integration deploy prompt — Web API calls won't work until permissions and site settings are deployed. +> +> **Trigger:** All integration code + permissions YAML committed. +> **Why we ask:** Auto-deploy picks whatever env PAC CLI happens to point at. +> **Cancel leaves:** Nothing — integration artifacts stay on disk; no deploy fired. + **Skip when AI-only read mode is active** (Phase 1.6 flag set) — the caller owns the single end-of-orchestration deploy decision. Return the Phase 7.2 summary and stop. diff --git a/plugins/power-pages/skills/manage-firewall/SKILL.md b/plugins/power-pages/skills/manage-firewall/SKILL.md index a55abde9e..543080188 100644 --- a/plugins/power-pages/skills/manage-firewall/SKILL.md +++ b/plugins/power-pages/skills/manage-firewall/SKILL.md @@ -121,7 +121,15 @@ Each `AskUserQuestion` call is a **separate** call. Wait for the user's answer b ### Default approach -Analyze the site's current state (firewall status, existing custom rules, managed rules, region eligibility) and **recommend the single most relevant action**. Present the recommendation with a plain-language explanation of why. The user can accept, choose a different action, or ask to just view the current state. + + +> 🚦 **Gate (plan · manage-firewall:3.action-choice):** Recommend an action based on the site's current state, then ask the user to accept or choose differently. Fires once per Phase 3 entry — loops back here if the user wants to make additional changes after Phase 4 applies the first one. +> +> **Trigger:** Phase 3 entry (interactive mode only — skipped in review mode). +> **Why we ask:** Wrong-action firewall changes are visible to every site visitor; auto-recommend without consent can disable an active rule the maker added deliberately. +> **Cancel leaves:** Nothing — Phase 4 hasn't fired yet. + +Analyze the site's current state (firewall status, existing custom rules, managed rules, region eligibility) and **recommend the single most relevant action**. Present the recommendation via `AskUserQuestion`: - Firewall off → recommend enabling it. - Firewall on, no custom rules → recommend adding a rule if there is a clear gap (e.g., no rate limiting). Otherwise, summarize the state and ask if the user wants to add a rule. @@ -133,6 +141,8 @@ MUST NOT proactively offer actions that reduce security (disabling the firewall, ### Option rules + + When presenting options via `AskUserQuestion`: - Keep `label` to 1–5 words. Include `description` on every option. - Include `preview` **only** when the option represents a concrete change (create, update, or delete a rule) — use it to show the configuration that will be applied so the user can review before approving. Do not add `preview` to navigation or informational choices. @@ -151,6 +161,14 @@ List current custom rules showing: what each rule does (plain language), what tr ### Plan-validate-execute + + +> 🚦 **Gate (consent · manage-firewall:3.execute-consent):** Final consent before any destructive WAF mutation (enable/disable, add/update/delete rule). Echoes the proposed JSON payload + the surfaced validation issues. Fires PER CHANGE — each enable, disable, rule add, rule update, and rule delete is its own consent. +> +> **Trigger:** Phase 3 action chosen, plan + validation surfaced. +> **Why we ask:** Firewall changes are env-level and visible to every site visitor; auto-applying can lock out legitimate traffic or weaken protection. +> **Cancel leaves:** Nothing — the API call hasn't fired yet; the plan + validation are throwaway. + For all rule changes: 1. **Plan** — build the JSON payload containing only the rules being added or updated. @@ -159,7 +177,7 @@ For all rule changes: - Overlapping match conditions (same `matchVariable`/`operator`, overlapping `matchValue`) — explain which rule wins via first-match-wins - Contradictions between Allow and Block rules — flag and explain priority implications - Redundancy — suggest updating the existing rule instead of adding a duplicate -3. **Execute** — apply only after user approval. +3. **Execute** — apply only after user approval via `AskUserQuestion`: For deletions, show the rule names and what each currently does before proceeding. diff --git a/plugins/power-pages/skills/manage-headers/SKILL.md b/plugins/power-pages/skills/manage-headers/SKILL.md index 0895e37c8..d326f22b0 100644 --- a/plugins/power-pages/skills/manage-headers/SKILL.md +++ b/plugins/power-pages/skills/manage-headers/SKILL.md @@ -97,6 +97,14 @@ MUST use plain language only. Never lead with words like CSP, CORS, HSTS, or MIM Read `references/headers-reference.md` for recommended values and guidance. **Present the most important gaps first** — headers that are missing or misconfigured relative to the recommended values. + + +> 🚦 **Gate (plan · manage-headers:3.per-finding):** Per-finding loop — for each header gap, prompt accept / customize / skip. Fires PER FINDING in the loop; skipped findings leave the header at its current value, accepted/customized findings get an Edit / create-script call in Phase 4. +> +> **Trigger:** Phase 3 entry has tallied header gaps against `references/headers-reference.md`. +> **Why we ask:** Auto-accepting can apply CSP/CORS values that break the site (legitimate scripts blocked, third-party widgets refused); auto-skipping leaves the site missing important headers. +> **Cancel leaves:** Nothing — Phase 4's Edit / create-script call only fires on accepted findings. + For each finding, present via `AskUserQuestion`: - A plain-language explanation of why the change matters - The recommended value diff --git a/plugins/power-pages/skills/plan-alm/SKILL.md b/plugins/power-pages/skills/plan-alm/SKILL.md index d75ee8431..88e5eccc1 100644 --- a/plugins/power-pages/skills/plan-alm/SKILL.md +++ b/plugins/power-pages/skills/plan-alm/SKILL.md @@ -445,6 +445,14 @@ Store stages as `PP_STAGES` (array of `{ label, envUrl, envName, type }`). Dev i For each stage, populate `envName` from `ENV_LIST` (gathered in Phase 1 Step 5 via `pac env list --output json`). Match by URL origin (lowercase, trailing slash stripped, path/query ignored) and copy the entry's `DisplayName` (or `displayName`) into `envName`. When no match is found — usually because the user pasted a custom URL via "Other" — leave `envName` unset; the renderer falls back to showing the URL alone in the stage card. The renderer puts `envName` between the stage label and the URL (e.g. *Staging / **Supplier Portal Staging** / https://orgd6a9894f.crm5.dynamics.com/*) so reviewers recognize the env at a glance and the URL stays available as a one-click jump-to-env. Set `type: "source"` for the dev/source stage and `type: "target"` for every downstream stage so the renderer applies the active-stage styling correctly. + + +> 🚦 **Gate (plan · plan-alm:2.q4-host):** Host environment selection — branches on `HOST_RESOLUTION.status` and surfaces the right menu (use-detected / pick from list / NoHost host-type / Sandbox confirm / CannotRedirect block / manual paste). Drives `HOST_ENV_URL` and `WILL_PROVISION_*` flags for the rest of plan-alm and ensure-pipelines-host. Uses `AskUserQuestion` per branch. +> +> **Trigger:** Phase 2 Q4 entry; `HOST_RESOLUTION` populated in Phase 1 step 12. +> **Why we ask:** Auto-picking a host can provision a new Custom Host (`WILL_PROVISION_CUSTOM`) consuming an Azure capacity quota the user didn't intend; or pick the wrong env, sending pipelines through a foreign host. The downstream ensure-pipelines-host skill TRUSTS this answer and skips its own 3.C menu. +> **Cancel leaves:** Nothing — no provisioning fired yet. + **Q4 (host environment — branches on `HOST_RESOLUTION.status` from Phase 1 step 12):** This question consumes `HOST_RESOLUTION` populated by the new detect-only wrapper run in Phase 1 step 12. Each branch sets `HOST_ENV_URL` (which feeds the rest of plan-alm) and may also set the auxiliary flags `CHOSEN_ENV_URL`, `WILL_PROVISION_PLATFORM`, `WILL_PROVISION_CUSTOM`, `WILL_USE_PPAC`, `WILL_ENSURE_HOST`, and `USER_CHOSE_DEFER_TO_SETUP_PIPELINE`. Defaults: `HOST_ENV_URL = HOST_RESOLUTION.finalHostEnvUrl`, all flags `false` / null. diff --git a/plugins/power-pages/skills/scan-site/SKILL.md b/plugins/power-pages/skills/scan-site/SKILL.md index fd1f2ac6a..5d4024280 100644 --- a/plugins/power-pages/skills/scan-site/SKILL.md +++ b/plugins/power-pages/skills/scan-site/SKILL.md @@ -109,7 +109,15 @@ MUST use plain language only. Never use words like CSP, CORS, OWASP, hardening, ### Default approach -Analyze the site's current state and **recommend the single most relevant action**. Present the recommendation with a plain-language explanation of why. The user can accept or choose differently. + + +> 🚦 **Gate (plan · scan-site:3.action-choice):** Recommend an action based on the site's scan state (running, idle, has report, no report), then ask the user to accept or choose differently. Starting a new scan triggers a multi-minute backend run; using an existing report is free. +> +> **Trigger:** Phase 3 entry (interactive mode only — review mode bypasses to step 4). +> **Why we ask:** Auto-starting a new scan wastes minutes if a recent report already answers the question; auto-using a stale report misses recent findings. +> **Cancel leaves:** Nothing — no scan triggered, no report consumed. + +Analyze the site's current state and **recommend the single most relevant action** via `AskUserQuestion`: - Scan running, no completed report → recommend waiting for the running scan to finish. - Scan running, report exists → recommend showing the latest results while the new scan continues. @@ -120,6 +128,8 @@ If the site's state does not warrant a specific recommendation, do not force one ### Option rules + + When presenting options via `AskUserQuestion`: - Keep `label` to 1–5 words. Include `description` on every option. - For options that trigger a new scan, surface the relevant caveats inside that option's `description` so the user has them at decision time. Do not ask a separate confirmation question after the user picks the option. diff --git a/plugins/power-pages/skills/security-review/SKILL.md b/plugins/power-pages/skills/security-review/SKILL.md index 517e9a94c..92842103a 100644 --- a/plugins/power-pages/skills/security-review/SKILL.md +++ b/plugins/power-pages/skills/security-review/SKILL.md @@ -94,6 +94,14 @@ The final HTML always lives at `/docs/security-review- + +> 🚦 **Gate (plan · security-review:2.1.goal):** Capture the review goal — choice branches into one of three sub-skill sets (`access-config` / `release` / `monitor`). +> +> **Trigger:** Phase 2.1 entry, unless `$ARGUMENTS` already answers it. +> **Why we ask:** Auto-picking `release` runs ALL sub-skills (slow; possibly hits scan/firewall endpoints unnecessarily); auto-picking the wrong goal mis-scopes the review. +> **Cancel leaves:** Nothing — no sub-skills invoked yet. + Ask the user with a single `AskUserQuestion` call. If the user's initial request already answers it, skip and continue. **Question — What to review?** @@ -256,6 +264,14 @@ Open `` in the user's default browser. ### 5.3 In-chat summary + + +> 🚦 **Gate (plan · security-review:5.3.next-action):** Post-report next-action prompt — *"Walk me through the fixes / Re-run the review / Done for now"*. Drives whether remediation skills get invoked. +> +> **Trigger:** Phase 5.1 wrote the HTML report. +> **Why we ask:** Auto-invoking remediation skills (`/manage-headers`, `/manage-firewall`, `/audit-permissions`) without the user reading the report; auto-re-running the review wastes time on a still-fresh result. +> **Cancel leaves:** Nothing — the HTML report at `docs/security-review-.html` is the final artifact regardless. + Show a short plain-language summary in the chat: counts of critical / warning / info findings, where the report lives. Then offer the next action with `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/setup-auth/SKILL.md b/plugins/power-pages/skills/setup-auth/SKILL.md index 1d5d3b0ee..95baa5db3 100644 --- a/plugins/power-pages/skills/setup-auth/SKILL.md +++ b/plugins/power-pages/skills/setup-auth/SKILL.md @@ -84,6 +84,14 @@ Look for the `.powerpages-site` folder: > "The `.powerpages-site` folder was not found. The site needs to be deployed at least once before authentication can be configured." + + +> 🚦 **Gate (plan · setup-auth:1.3.deploy-first):** `.powerpages-site` missing — auth setup writes site settings inside that folder. Deploy first or stop. +> +> **Trigger:** Phase 1.3 detected no `.powerpages-site` folder. +> **Why we ask:** Auto-deploy picks the wrong env; skipping leaves auth wiring broken. +> **Cancel leaves:** Nothing — no auth files written yet. + Use `AskUserQuestion`: | Question | Options | @@ -104,7 +112,15 @@ Look for web role YAML files in `.powerpages-site/web-roles/`: Read each file and compile a list of existing web roles (name, id, flags). -**If no web roles exist**: Warn the user that web roles are needed for authorization. Ask if they want to create them first: + + +> 🚦 **Gate (plan · setup-auth:1.4.create-webroles):** No web roles found — role-based authorization needs at least one role. Create roles first or skip and add later. +> +> **Trigger:** Phase 1.4 found no YAML files in `.powerpages-site/web-roles/`. +> **Why we ask:** Auto-invoking `/create-webroles` runs another full skill; auto-skipping leaves RBAC checks against an empty role set. +> **Cancel leaves:** Nothing — no auth files written yet. + +**If no web roles exist**: Warn the user that web roles are needed for authorization. Ask via `AskUserQuestion` whether to create them first: | Question | Options | |----------|---------| @@ -256,6 +272,14 @@ Before asking the user which providers they want, analyze the site context from #### 2.1 Gather Requirements + + +> 🚦 **Gate (plan · setup-auth:2.1.requirements):** Pick which auth features to build (login+logout / RBAC / both). Covers the conditional follow-up "which roles get access" sub-prompt in the same step. +> +> **Trigger:** Phase 2.1 entry. +> **Why we ask:** Wrong feature set gets generated — e.g. building RBAC files when the user only wanted login. +> **Cancel leaves:** Nothing — no auth files written yet. + **Re-run handling — when Phase 1.5 detected existing providers:** The behavior depends on the `MERGE_MODE` chosen in Phase 1.5: @@ -972,6 +996,14 @@ Present the implementation plan inline: - Which routes/components will be protected and with which roles - The site setting that needs to be configured (`Authentication/Registration/ProfileRedirectEnabled = false`) + + +> 🚦 **Gate (plan · setup-auth:2.2.plan-approval):** Final sign-off on the auth implementation plan before any file is written. +> +> **Trigger:** Phase 2.2 presented the full plan inline. +> **Why we ask:** Wrong components generated; site settings written; ProfileRedirectEnabled flipped — fixable but adds churn. +> **Cancel leaves:** Nothing — no auth files written yet. + Use `AskUserQuestion` to get approval: | Question | Options | @@ -2840,6 +2872,14 @@ The renderer refuses to overwrite an existing file. If a previous report already #### 8.4 Ask to Deploy + + +> 🚦 **Gate (plan · setup-auth:8.4.deploy):** Final deploy prompt — auth doesn't work until deployed (site settings ship with the deploy). +> +> **Trigger:** All auth files created and verified. +> **Why we ask:** Auto-deploy picks the wrong env. +> **Cancel leaves:** Nothing — auth artifacts stay on disk; no deploy fired. + Use `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/setup-datamodel/SKILL.md b/plugins/power-pages/skills/setup-datamodel/SKILL.md index 4397ecf15..0495d3627 100644 --- a/plugins/power-pages/skills/setup-datamodel/SKILL.md +++ b/plugins/power-pages/skills/setup-datamodel/SKILL.md @@ -44,6 +44,14 @@ Guide the user through creating Dataverse tables, columns, and relationships for **Actions**: + + +> 🚦 **Gate (plan · setup-datamodel:2.source):** Decide whether the user uploads an existing ER diagram or the data-model-architect agent infers the model. Choice routes the rest of the skill into Path A vs Path B. +> +> **Trigger:** Entering Phase 2. +> **Why we ask:** Auto-picking either path can run a multi-minute architect agent against the wrong intent (Path B) or skip Dataverse-existence checks (Path A). +> **Cancel leaves:** Nothing — no Dataverse calls made yet. + 1. Ask the user how they want to define the data model using the `AskUserQuestion` tool: **Question**: "How would you like to define the data model for your site?" @@ -137,6 +145,14 @@ Present the data model proposal directly to the user as a formatted message, inc ### 4.2 Get User Approval + + +> 🚦 **Gate (plan · setup-datamodel:4.2.approval):** Final sign-off on the data model proposal before any Dataverse write. Cancel here stops the skill with zero side effects. +> +> **Trigger:** Phase 4.1 rendered the proposal (tables, columns, relationships, ER diagram). +> **Why we ask:** Tables and columns get created in Dataverse against the user's actual schema intent — column types and relationship cardinalities are awkward to undo. +> **Cancel leaves:** Nothing — no `EntityDefinitions` POST yet, no `.datamodel-manifest.json` write. + Use `AskUserQuestion` to get approval: | Question | Header | Options | diff --git a/plugins/power-pages/skills/setup-pipeline/SKILL.md b/plugins/power-pages/skills/setup-pipeline/SKILL.md index f2b2d0291..6adb631ca 100644 --- a/plugins/power-pages/skills/setup-pipeline/SKILL.md +++ b/plugins/power-pages/skills/setup-pipeline/SKILL.md @@ -282,6 +282,15 @@ Capture output as JSON; check `.found`. If `false`: warn the user — the soluti GET {hostEnvUrl}/api/data/v9.1/deploymentpipelines?$filter=name eq '{PIPELINE_NAME}'&$select=deploymentpipelineid&$top=1 Authorization: Bearer {HOST_TOKEN} ``` + + + +> 🚦 **Gate (plan · setup-pipeline:4.3.name-conflict):** A pipeline with the same name already exists in the host env. Pick: reuse the existing pipeline ID, or create a new one with a different name. Auto-reusing risks attaching to a pipeline owned by someone else; auto-overwriting loses their stage history. +> +> **Trigger:** Phase 4.3 query returned a hit. +> **Why we ask:** Either a foreign pipeline gets its stages overwritten, or a duplicate pipeline gets created that pollutes the host env's pipeline list. +> **Cancel leaves:** Nothing — no Dataverse write yet. + If found: ask via `AskUserQuestion` whether to use the existing pipeline ID or create a new one with a different name. **4.4 Check `blockedattachments` on source + all target envs:** @@ -477,6 +486,14 @@ Confirm `statecode = 0` (Active). If the query fails, report as "verification in } ``` + + +> 🚦 **Gate (plan · setup-pipeline:6b.v2-migration):** v2 `pipelines[]` manifest detected on re-run. Pick: migrate to v3 (delete the N-1 extra pipelines and collapse to one) or keep the legacy layout. +> +> **Trigger:** Re-running setup-pipeline on a project whose `docs/alm/last-pipeline.json` is `schemaVersion: 2`. +> **Why we ask:** Auto-migrating deletes Dataverse pipeline records — destructive against host env state, irreversible without re-running setup-pipeline. +> **Cancel leaves:** Nothing — no pipeline records deleted yet. + > **Migration note:** Earlier versions of this skill used `schemaVersion: 2` with a `pipelines[]` array (one Dataverse pipeline record per solution). Projects pinned to v2 continue to work with the old `deploy-pipeline` MULTI_PIPELINE_MODE path; the v3 format should be used for all new setups. When re-running `setup-pipeline` on a v2 project, ask via `AskUserQuestion` whether to migrate (delete the N-1 extra pipelines and collapse to a single one) or keep the legacy layout. **7.3 Write (or re-render) `docs/pipeline-setup.md`** (create `docs/` directory if needed). diff --git a/plugins/power-pages/skills/test-site/SKILL.md b/plugins/power-pages/skills/test-site/SKILL.md index e7cde20a2..478f17de5 100644 --- a/plugins/power-pages/skills/test-site/SKILL.md +++ b/plugins/power-pages/skills/test-site/SKILL.md @@ -400,7 +400,15 @@ For each failed API request, provide specific remediation: #### 5.5 Test Form Submissions (Optional) -If forms are detected on any page (via `browser_snapshot` showing form elements), ask the user before interacting: + + +> 🚦 **Gate (consent · test-site:5.5.form-submit):** About to submit a form on the live site — may create or modify Dataverse records. Destructive against shared state (the live Dataverse env); requires explicit opt-in. +> +> **Trigger:** Forms detected via `browser_snapshot` in Phase 5. +> **Why we ask:** Auto-submitting test data into production records pollutes real customer data. +> **Cancel leaves:** Nothing — read-only API checks continue from earlier 5.x phases. + +If forms are detected on any page (via `browser_snapshot` showing form elements), ask the user via `AskUserQuestion` before interacting: | Question | Header | Options | |----------|--------|---------|