From 3a55b95e971be70821f801327c6e7f83d766f7b6 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 17:05:33 +0530 Subject: [PATCH 01/10] Apply Approval Gate pattern to non-ALM power-pages skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the catalog in references/approval-gates.md (ยง6.13-ยง6.24) to cover the 12 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. 45 gates + 9 not-a-gates inserted as HTML comment markers + human ๐Ÿšฆ blocks above each AskUserQuestion call. Flip lint to hard-fail across the whole plugin -- the ALM-only warn-only branch in scripts/lint-skills-alm.js has been removed. severityForSkill() now returns 'error' for every skill. Update the corresponding test in scripts/tests/lint-skills-alm.test.js. AGENTS.md Key Patterns generalized: the Approval Gate convention applies plugin-wide. New skills must extend the catalog in the same PR that introduces an AskUserQuestion, or CI blocks the PR. Workflow files get header comments explaining why power-pages- alm-lint and power-pages-script-tests are deliberately separate (policy enforcement vs functional tests; single-OS fast-fail vs cross-OS matrix). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/power-pages-alm-lint.yml | 6 + .../workflows/power-pages-script-tests.yml | 5 + plugins/power-pages/AGENTS.md | 4 +- .../power-pages/references/approval-gates.md | 201 ++++++++++++++---- .../power-pages/scripts/lint-skills-alm.js | 26 ++- .../scripts/tests/lint-skills-alm.test.js | 6 +- .../skills/add-cloud-flow/SKILL.md | 42 ++++ .../skills/add-sample-data/SKILL.md | 16 ++ plugins/power-pages/skills/add-seo/SKILL.md | 12 ++ .../skills/add-server-logic/SKILL.md | 72 +++++++ .../skills/audit-permissions/SKILL.md | 9 + .../power-pages/skills/create-site/SKILL.md | 43 ++++ .../skills/create-webroles/SKILL.md | 24 +++ .../power-pages/skills/deploy-site/SKILL.md | 51 +++++ .../skills/integrate-backend/SKILL.md | 10 + .../skills/integrate-webapi/SKILL.md | 40 ++++ .../power-pages/skills/setup-auth/SKILL.md | 40 ++++ .../skills/setup-datamodel/SKILL.md | 16 ++ 18 files changed, 574 insertions(+), 49 deletions(-) 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 7a3baa427..92f27b533 100644 --- a/plugins/power-pages/AGENTS.md +++ b/plugins/power-pages/AGENTS.md @@ -241,7 +241,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 five 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`). ยง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. @@ -339,7 +339,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** โ€” If a skill needs command validation or checklist enforcement, update `hooks/hooks.json` and `scripts/lib/powerpages-hook-utils.js`. 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/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index c33d141f0..6ad3a9010 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 the five GATE 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`) at error severity on every SKILL.md under `plugins/power-pages/skills/`. --- @@ -468,27 +470,159 @@ 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) -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: +| 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 | -| 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) | +--- + +### 6.14 `deploy-site` (8 calls) + +| 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) -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). +| 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 | + +--- + +### 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 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. --- @@ -507,24 +641,17 @@ These need explicit confirmation from the reviewer before SKILL.md edits land. R --- -## 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: -**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). +- Markers added to all 12 non-ALM SKILL.md files (HTML comment + ๐Ÿšฆ block per gate; `not-a-gate` comment per data-gathering prompt). +- `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). --- diff --git a/plugins/power-pages/scripts/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 073c000bf..14937c674 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 @@ -187,9 +189,13 @@ const KNOWN_RULES = new Set([ '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. +// v2 had a separate ALM_SKILLS set used to flip lint severity between hard-fail +// (ALM) and warn-only (non-ALM). v3 removed the carve-out โ€” every skill under +// plugins/power-pages/skills/ is hard-fail because the catalog in +// references/approval-gates.md now covers all skills (see ยง10 landing history). +// The set is kept as the canonical "ALM skill family" list for reference and +// for downstream tooling that needs to enumerate ALM skills, but it no longer +// influences lint severity. const ALM_SKILLS = new Set([ 'plan-alm', 'setup-solution', @@ -437,7 +443,11 @@ function skillNameFromFile(file) { } function severityForSkill(skillName) { - return ALM_SKILLS.has(skillName) ? 'error' : 'warning'; + // v3: every skill under plugins/power-pages/skills/ is enforced at hard-fail. + // `skillName` retained as a parameter so future per-skill policy can hook in + // without rewiring the call sites. + void skillName; + return 'error'; } // Parse the catalog file (references/approval-gates.md) and extract all 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..682fb1117 100644 --- a/plugins/power-pages/scripts/tests/lint-skills-alm.test.js +++ b/plugins/power-pages/scripts/tests/lint-skills-alm.test.js @@ -565,7 +565,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 +580,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) => { diff --git a/plugins/power-pages/skills/add-cloud-flow/SKILL.md b/plugins/power-pages/skills/add-cloud-flow/SKILL.md index 845884288..05ba3addc 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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 | @@ -343,6 +361,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. +> **Blast radius if skipped:** 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 +609,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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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..6ec9e78a1 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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..1634e6997 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). +> **Blast radius if skipped:** 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..08cd0db26 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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). +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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..543b245b7 100644 --- a/plugins/power-pages/skills/audit-permissions/SKILL.md +++ b/plugins/power-pages/skills/audit-permissions/SKILL.md @@ -494,6 +494,15 @@ Present a summary to the user: 1. **Critical findings count** โ€” these need immediate attention 2. **Warning findings count** โ€” should be addressed 3. **Report location** โ€” where the HTML file was saved + + + +> ๐Ÿšฆ **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. +> **Blast radius if skipped:** 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. + 4. **Ask the user** using `AskUserQuestion`: "Would you like me to fix any of these issues? I can create or update table permissions to resolve the critical and warning findings." If the user wants fixes applied: diff --git a/plugins/power-pages/skills/create-site/SKILL.md b/plugins/power-pages/skills/create-site/SKILL.md index fe88aa8f3..e7b4127d5 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. @@ -74,6 +76,14 @@ Write the file with the `Write` tool (atomic overwrite). You do not need to read 2. If site purpose is clear from arguments: - Summarize understanding - Identify site type (portal, dashboard, landing page, blog, etc.) + + +> ๐Ÿšฆ **Gate (plan ยท create-site:1.purpose):** Multi-question prompt collecting site name, framework, purpose, audience, and target directory. Determines what gets scaffolded. +> +> **Trigger:** Phase 1 when site purpose was not provided in `$ARGUMENTS`. +> **Blast radius if skipped:** Wrong framework picked โ†’ wrong template copied into the wrong directory; cleanup is annoying. +> **Cancel leaves:** Nothing โ€” no scaffolding has started yet. + 3. If site purpose is unclear, use `AskUserQuestion`: | Question | Header | Options | @@ -227,6 +237,14 @@ Immediately after the dev server starts, verify the scaffold is working: Immediately after the user answers, `Write` the same file again with `"awaitingInput": false` so the banner disappears. + + +> ๐Ÿšฆ **Gate (plan ยท create-site:3.requirements):** Three sub-prompts (features multi-select, aesthetic, mood) โ€” shape the Phase 4 plan and the Phase 5 implementation. +> +> **Trigger:** Phase 3 entry; scaffold loader is up. +> **Blast radius if skipped:** 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. + 2. Use `AskUserQuestion` to collect feature and design requirements: | Question | Header | Options | @@ -342,6 +360,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. +> **Blast radius if skipped:** 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 | @@ -555,6 +581,15 @@ Present a summary table to the user: ``` 3. Share the dev server URL with the user and list all available routes + + + +> ๐Ÿšฆ **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. +> +> **Trigger:** Phase 7 has verified all pages render via Playwright. +> **Blast radius if skipped:** User loses the chance to spot UI issues before deploy; broken pages get pushed. +> **Cancel leaves:** Nothing โ€” site files stay as-is on disk. + 4. Ask the user to review using `AskUserQuestion`: > "The site is ready for review at ``. Please check it out in your browser. Would you like any changes?" 5. If the user requests changes, apply them and re-verify by browsing via `browser_snapshot` @@ -577,6 +612,14 @@ Present a summary table to the user: Follow the skill tracking instructions in the reference to record this skill's usage. Use `--skillName "CreateSite"`. Note: `.powerpages-site` may not exist for first-time sites โ€” the script exits silently. + + +> ๐Ÿšฆ **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. +> +> **Trigger:** Phase 8 entry; Phase 7 review approved. +> **Blast radius if skipped:** 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. + 2. Use `AskUserQuestion` with options: **Deploy now (Recommended)**, **Skip for now**: > "Would you like to deploy your site to Power Pages now?" 3. If the user chooses to deploy, invoke the `/deploy-site` skill. diff --git a/plugins/power-pages/skills/create-webroles/SKILL.md b/plugins/power-pages/skills/create-webroles/SKILL.md index 6975ce817..4a7356cd8 100644 --- a/plugins/power-pages/skills/create-webroles/SKILL.md +++ b/plugins/power-pages/skills/create-webroles/SKILL.md @@ -46,6 +46,14 @@ Create web roles for a Power Pages code site. Web roles define the permissions a 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. +> **Blast radius if skipped:** 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:** Ask the user to deploy first via `AskUserQuestion` (options: "Yes, deploy now (Recommended)", "No, I'll do it later"). If yes, invoke `/deploy-site` then resume from Phase 2. If no, stop. 3. **If `.powerpages-site` exists but `web-roles/` does NOT:** Create the `/.powerpages-site/web-roles/` directory. @@ -93,6 +101,14 @@ Create web roles for a Power Pages code site. Web roles define the permissions a **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. +> **Blast radius if skipped:** 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: @@ -203,6 +219,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. +> **Blast radius if skipped:** 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. Then ask the user if they want to deploy the site to apply the new roles: | Question | Options | diff --git a/plugins/power-pages/skills/deploy-site/SKILL.md b/plugins/power-pages/skills/deploy-site/SKILL.md index 6d446d5e0..c80270fe5 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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). +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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). +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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/integrate-backend/SKILL.md b/plugins/power-pages/skills/integrate-backend/SKILL.md index 3e8cbb46a..15e2ebf24 100644 --- a/plugins/power-pages/skills/integrate-backend/SKILL.md +++ b/plugins/power-pages/skills/integrate-backend/SKILL.md @@ -101,6 +101,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 | @@ -296,6 +298,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. +> **Blast radius if skipped:** 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 89f7071ea..a08ac4520 100644 --- a/plugins/power-pages/skills/integrate-webapi/SKILL.md +++ b/plugins/power-pages/skills/integrate-webapi/SKILL.md @@ -154,6 +154,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. +> **Blast radius if skipped:** 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. + Use `AskUserQuestion` to confirm: | Question | Options | @@ -279,6 +287,14 @@ Present a table summarizing the verification: Both agents require the `.powerpages-site` folder. If it 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. +> **Blast radius if skipped:** 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 | @@ -291,6 +307,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. +> **Blast radius if skipped:** 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. + Ask the user how they want to define the permissions using the `AskUserQuestion` tool: **Question**: "How would you like to define the Web API permissions and settings for your site?" @@ -322,6 +346,14 @@ If the user chooses to upload an existing diagram: 5. Generate a Mermaid flowchart from the parsed data (if the user provided an image or text) for visual confirmation. + + +> ๐Ÿšฆ **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. +> +> **Trigger:** Path A โ€” diagram parsed and Mermaid flowchart rendered. +> **Blast radius if skipped:** 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. + 6. Present the parsed permissions plan to the user for approval using `AskUserQuestion`: | Question | Options | @@ -478,6 +510,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. +> **Blast radius if skipped:** Auto-deploy picks whatever env PAC CLI happens to point at. +> **Cancel leaves:** Nothing โ€” integration artifacts stay on disk; no deploy fired. + Use `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/setup-auth/SKILL.md b/plugins/power-pages/skills/setup-auth/SKILL.md index f3be25e12..9fe0a4dc0 100644 --- a/plugins/power-pages/skills/setup-auth/SKILL.md +++ b/plugins/power-pages/skills/setup-auth/SKILL.md @@ -75,6 +75,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. +> **Blast radius if skipped:** Auto-deploy picks the wrong env; skipping leaves auth wiring broken. +> **Cancel leaves:** Nothing โ€” no auth files written yet. + Use `AskUserQuestion`: | Question | Options | @@ -95,6 +103,14 @@ 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). + + +> ๐Ÿšฆ **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/`. +> **Blast radius if skipped:** 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 if they want to create them first: | Question | Options | @@ -134,6 +150,14 @@ If auth files already exist, present them to the user and ask whether to overwri #### 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. +> **Blast radius if skipped:** Wrong feature set gets generated โ€” e.g. building RBAC files when the user only wanted login. +> **Cancel leaves:** Nothing โ€” no auth files written yet. + Use `AskUserQuestion` to determine the scope: | Question | Options | @@ -155,6 +179,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. +> **Blast radius if skipped:** 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 | @@ -514,6 +546,14 @@ Present a summary of everything created: #### 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. +> **Blast radius if skipped:** 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..dbf85833b 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. +> **Blast radius if skipped:** 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). +> **Blast radius if skipped:** 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 | From 3fa42cd0cd3849b51165b90bdda3a8c36bcee6ee Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 17:14:21 +0530 Subject: [PATCH 02/10] Catalog the 4 security skills picked up during rebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebase onto origin/main brought in PR #151 which added 4 new power-pages skills: manage-firewall, manage-headers, scan-site, security-review. With the warn-only lint branch removed in the previous commit, those skills now had unmarked AskUserQuestion prompts and broke CI. These skills use a different prompt shape โ€” most calls happen inside a runtime "recommend then ask" loop described in prose, not at statically-locatable call sites. The catalog now has a new ยง6.24a section explaining the convention for runtime-loop skills: meta-mention sections get not-a-gate markers; concrete call sites get full gate markers. Catalog rows added: - ยง6.25 manage-firewall โ€” 1 not-a-gate (option-rules-meta) - ยง6.26 manage-headers โ€” 1 gate (per-finding loop, plan) - ยง6.27 scan-site โ€” 1 not-a-gate (option-rules-meta) - ยง6.28 security-review โ€” 2 gates (2.1 goal, 5.3 next-action) Total: 3 gates + 2 not-a-gates. Lint passes (0 findings). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../power-pages/references/approval-gates.md | 51 +++++++++++++++++-- .../skills/manage-firewall/SKILL.md | 2 + .../skills/manage-headers/SKILL.md | 8 +++ plugins/power-pages/skills/scan-site/SKILL.md | 2 + .../skills/security-review/SKILL.md | 16 ++++++ 5 files changed, 76 insertions(+), 3 deletions(-) diff --git a/plugins/power-pages/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index 6ad3a9010..891ac4773 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -607,6 +607,50 @@ When **removing** a gate, also remove its catalog row in the same PR. --- +### 6.24a Security skills โ€” runtime-loop pattern + +The four security skills introduced in PR #151 (`manage-firewall`, `manage-headers`, `scan-site`, `security-review`) use `AskUserQuestion` differently from the other skills: they don't have phase-numbered, statically-locatable prompt sites. Most calls happen inside a "recommend then ask" runtime loop that's described in prose (e.g., *"recommend the single most relevant action"* in `manage-firewall` ยง3 and `scan-site` ยง3). The marker convention treats these as follows: + +- **Meta-mention sections** (`### Option rules` in `manage-firewall` and `scan-site`) are tagged `` โ€” they document HOW to construct prompts, they aren't prompt sites themselves. +- **Concrete prompt sites** that exist as literal "`AskUserQuestion`:" blocks (e.g. `manage-headers` ยง3 per-finding loop, `security-review` ยง2.1 and ยง5.3) get full `` markers. + +Future hardening (out of scope for v3): the runtime-loop calls in `manage-firewall` and `scan-site` could be made statically locatable by moving the "recommend an action" block into a numbered subsection like `### 3.1 Recommend` with a literal AskUserQuestion code example. The PR #151 authors can revisit if the loop pattern proves hard to audit. + +--- + +### 6.25 `manage-firewall` (1 lint-caught meta-mention + dynamic runtime loop) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `manage-firewall:3.option-rules-meta` | not-a-gate | โ€” | 3 (`### Option rules`) | Documentation describing how to structure `AskUserQuestion` options in this skill โ€” not a call site itself. The actual destructive firewall changes (enable/disable/add-rule/remove-rule) gate via the prose-described "apply only after user approval" rule in ยง3 Plan-validate-execute and ยง4 Apply the change. | โ€” | + +--- + +### 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` (1 lint-caught meta-mention + dynamic runtime loop) + +| ID | Kind | Category | Phase | Trigger / question | Cancel leaves | +|---|---|---|---|---|---| +| `scan-site:3.option-rules-meta` | not-a-gate | โ€” | 3 (`### Option rules`) | Documentation describing how to structure `AskUserQuestion` options. The actual prompt โ€” *"use existing report / run a fresh scan"* โ€” fires dynamically in ยง3 Default approach and triggers a long-running scan but no destructive site change. | โ€” | + +--- + +### 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 | + +--- + ### 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. @@ -615,7 +659,7 @@ When **removing** a gate, also remove its catalog row in the same PR. ## 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 and the lint runs hard-fail across the whole plugin. +> **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: @@ -645,10 +689,11 @@ These need explicit confirmation from the reviewer before SKILL.md edits land. R **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. -**v3 PR (this branch โ€” `users/nityagi/ApplyApprovalGatesPattern`)** โ€” extends the catalog and enforcement to the 12 non-ALM skills: +**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: - ยง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). -- Markers added to all 12 non-ALM SKILL.md files (HTML comment + ๐Ÿšฆ block per gate; `not-a-gate` comment per data-gathering prompt). +- ยง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). diff --git a/plugins/power-pages/skills/manage-firewall/SKILL.md b/plugins/power-pages/skills/manage-firewall/SKILL.md index a55abde9e..044fb2631 100644 --- a/plugins/power-pages/skills/manage-firewall/SKILL.md +++ b/plugins/power-pages/skills/manage-firewall/SKILL.md @@ -133,6 +133,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. diff --git a/plugins/power-pages/skills/manage-headers/SKILL.md b/plugins/power-pages/skills/manage-headers/SKILL.md index 0895e37c8..2a7becbd7 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`. +> **Blast radius if skipped:** 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/scan-site/SKILL.md b/plugins/power-pages/skills/scan-site/SKILL.md index fd1f2ac6a..28e2d1c58 100644 --- a/plugins/power-pages/skills/scan-site/SKILL.md +++ b/plugins/power-pages/skills/scan-site/SKILL.md @@ -120,6 +120,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..a0eb053f5 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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 | From 1011030adaf14b05d30a08ad38a22ea3c10b95f5 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 17:43:31 +0530 Subject: [PATCH 03/10] Apply review fixes: doc, list-rendering, lint cleanups, ALM orphan markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the 12 findings from the pre-PR code review. Fixes ordered by severity: MUST FIX: - PLUGIN_DEVELOPMENT_GUIDE.md: drop the stale "non-ALM warn-only" text on line 273 and mirror the v3 hard-fail language. AGENTS.md tells new authors to read this file first, so the stale doc would have misled them. MARKDOWN RENDERING: - audit-permissions, create-site (4 places), integrate-webapi: move gate marker blocks ABOVE the ordered list intro sentence so they don't sit between numbered items. CommonMark/GFM split a list at an unindented HTML comment + blockquote, restarting numbering. The rendered numbering on GitHub was 1,2,3,1,2 instead of 1..N. PRE-EXISTING FILE BUGS: - Stray ``` token at add-cloud-flow:346 and integrate-backend:271 (both from PR #167) โ€” removed. These broke fence parity for everything below them. LINT CLEANUPS: - severityForSkill() is now a constant (SKILL_SEVERITY); unused skillName parameter and void no-op removed; call sites simplified. - Dead warnings-only exit branch in main() removed โ€” no code path produces severity 'warning' post-v3. - Tightened checkSectionPairing: m < promptLine (strictly before), not m <= promptLine, so a marker on the SAME line as the prompt no longer trivially satisfies the rule. - CATALOG_GATE_ID_PATTERN now case-insensitive [A-Za-z] to align with GATE_MARKER_PATTERN โ€” future CamelCase / underscored skill names won't break GATE-must-be-in-catalog. ALM CATALOG ORPHANS: - 16 catalog gate rows previously lacked SKILL.md markers (introduced pre-v3). Resolved in two ways: - Added markers where the prompt call site is concrete: test-site:5.5.form-submit, setup-pipeline:4.3.name-conflict + 6b.v2-migration, export-solution:2.identify + 3.overwrite, force-link-environment:2.host-url + 2.dev-env, ensure-pipelines- host:3.C.host-type + 3.C.env-pick + 4.0.pre-call + 4.A.pre-call + 4.sandbox-confirm + 4.C.ppac-done, plan-alm:2.q4-host (renamed from q4-stage-env to match the actual host-selection prompt). - Removed orphan rows that described gates not implemented as separate AskUserQuestion calls: deploy-pipeline:6.1.pac-fallback- consent (covered by 6.0.final-consent prose), configure-env- variables:6.confirm-matrix (covered by 2.selection's per-stage matrix), force-link-environment:2.host-fallback + 2.dev-fallback (redundant โ€” the gate IDs above cover the fallback path). PROSE ANCHORS: - setup-auth Phase 1.4 prompt prose now includes the literal backticked AskUserQuestion + colon anchor required by PROMPT_LINE_PATTERN. Removing the gate marker would now actually trigger GATE-must-have-marker. VERIFIED: lint reports 0 findings; 984/984 plugin tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../power-pages/PLUGIN_DEVELOPMENT_GUIDE.md | 2 +- .../power-pages/references/approval-gates.md | 18 +++--- .../power-pages/scripts/lint-skills-alm.js | 50 ++++++++-------- .../skills/add-cloud-flow/SKILL.md | 1 - .../skills/audit-permissions/SKILL.md | 11 ++-- .../power-pages/skills/create-site/SKILL.md | 59 +++++++++---------- .../skills/ensure-pipelines-host/SKILL.md | 56 ++++++++++++++++-- .../skills/export-solution/SKILL.md | 23 +++++++- .../skills/force-link-environment/SKILL.md | 17 +++++- .../skills/integrate-backend/SKILL.md | 1 - .../skills/integrate-webapi/SKILL.md | 16 ++--- plugins/power-pages/skills/plan-alm/SKILL.md | 8 +++ .../power-pages/skills/setup-auth/SKILL.md | 2 +- .../skills/setup-pipeline/SKILL.md | 17 ++++++ plugins/power-pages/skills/test-site/SKILL.md | 10 +++- 15 files changed, 199 insertions(+), 92 deletions(-) diff --git a/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md b/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md index dddaccad7..eb9bab943 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 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`). **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 891ac4773..278078d62 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -266,7 +266,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 | @@ -339,7 +339,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` | + +The previously-listed `deploy-pipeline:6.1.pac-fallback-consent` row was merged into `6.0.final-consent` โ€” the 6.0 marker's prose explicitly covers both the `DeployPackageAsync` and `pac pipeline deploy` paths, so a separate ID would have been redundant and the new row produced no second prompt. (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.) @@ -384,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"* โ€” the per-stage values matrix is built inside this same multi-question prompt, so a separate "confirm matrix" gate would be redundant. | 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 | --- @@ -409,15 +409,15 @@ 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 | โ€” | + +The previously-listed `2.host-fallback` / `2.dev-fallback` not-a-gate rows were redundant โ€” they described the free-text fallback option of the gate above, not a separate prompt. --- diff --git a/plugins/power-pages/scripts/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 14937c674..4e1979930 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -442,19 +442,22 @@ function skillNameFromFile(file) { return parts[idx + 1]; } -function severityForSkill(skillName) { - // v3: every skill under plugins/power-pages/skills/ is enforced at hard-fail. - // `skillName` retained as a parameter so future per-skill policy can hook in - // without rewiring the call sites. - void skillName; - return 'error'; -} +// v3: every skill under plugins/power-pages/skills/ is enforced at hard-fail. +// Kept as a named constant rather than a no-op function so the call sites are +// honest about the lack of per-skill policy. If a future PR needs per-skill +// severity, restore a function here and pass the skill name in. +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 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 or underscored skill would fail GATE-must-be-in-catalog +// even with a correct catalog row. +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'); @@ -486,7 +489,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 }); } } @@ -521,8 +528,6 @@ 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); @@ -568,7 +573,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}) ` + @@ -624,7 +629,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 ` + @@ -707,26 +712,19 @@ 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; - } + // 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. + for (const f of findings) process.stderr.write(formatFinding(f, pluginRoot)); process.stderr.write( - `\nalm-lint: ${errors.length} error(s), ${warnings.length} warning(s) in ${pluginRoot}\n` + `\nalm-lint: ${findings.length} error(s) in ${pluginRoot}\n` ); return 1; } diff --git a/plugins/power-pages/skills/add-cloud-flow/SKILL.md b/plugins/power-pages/skills/add-cloud-flow/SKILL.md index 05ba3addc..324b2aee4 100644 --- a/plugins/power-pages/skills/add-cloud-flow/SKILL.md +++ b/plugins/power-pages/skills/add-cloud-flow/SKILL.md @@ -343,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 diff --git a/plugins/power-pages/skills/audit-permissions/SKILL.md b/plugins/power-pages/skills/audit-permissions/SKILL.md index 543b245b7..d079e4ca6 100644 --- a/plugins/power-pages/skills/audit-permissions/SKILL.md +++ b/plugins/power-pages/skills/audit-permissions/SKILL.md @@ -489,12 +489,6 @@ Follow the skill tracking instructions in the reference to record this skill's u ### 6.2 Present Summary -Present a summary to the user: - -1. **Critical findings count** โ€” these need immediate attention -2. **Warning findings count** โ€” should be addressed -3. **Report location** โ€” where the HTML file was saved - > ๐Ÿšฆ **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. @@ -503,6 +497,11 @@ Present a summary to the user: > **Blast radius if skipped:** 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 +2. **Warning findings count** โ€” should be addressed +3. **Report location** โ€” where the HTML file was saved 4. **Ask the user** using `AskUserQuestion`: "Would you like me to fix any of these issues? I can create or update table permissions to resolve the critical and warning findings." If the user wants fixes applied: diff --git a/plugins/power-pages/skills/create-site/SKILL.md b/plugins/power-pages/skills/create-site/SKILL.md index e7b4127d5..0832312ed 100644 --- a/plugins/power-pages/skills/create-site/SKILL.md +++ b/plugins/power-pages/skills/create-site/SKILL.md @@ -72,18 +72,18 @@ Write the file with the `Write` tool (atomic overwrite). You do not need to read **Actions**: -1. Create todo list with all 8 phases (see [Progress Tracking](#progress-tracking) table) -2. If site purpose is clear from arguments: - - Summarize understanding - - Identify site type (portal, dashboard, landing page, blog, etc.) -> ๐Ÿšฆ **Gate (plan ยท create-site:1.purpose):** Multi-question prompt collecting site name, framework, purpose, audience, and target directory. Determines what gets scaffolded. +> ๐Ÿšฆ **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`. > **Blast radius if skipped:** 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 + - Identify site type (portal, dashboard, landing page, blog, etc.) 3. If site purpose is unclear, use `AskUserQuestion`: | Question | Header | Options | @@ -227,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. +> **Blast radius if skipped:** 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`: @@ -237,14 +245,6 @@ Immediately after the dev server starts, verify the scaffold is working: Immediately after the user answers, `Write` the same file again with `"awaitingInput": false` so the banner disappears. - - -> ๐Ÿšฆ **Gate (plan ยท create-site:3.requirements):** Three sub-prompts (features multi-select, aesthetic, mood) โ€” shape the Phase 4 plan and the Phase 5 implementation. -> -> **Trigger:** Phase 3 entry; scaffold loader is up. -> **Blast radius if skipped:** 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. - 2. Use `AskUserQuestion` to collect feature and design requirements: | Question | Header | Options | @@ -566,6 +566,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. +> **Blast radius if skipped:** 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 @@ -581,15 +589,6 @@ Present a summary table to the user: ``` 3. Share the dev server URL with the user and list all available routes - - - -> ๐Ÿšฆ **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. -> -> **Trigger:** Phase 7 has verified all pages render via Playwright. -> **Blast radius if skipped:** User loses the chance to spot UI issues before deploy; broken pages get pushed. -> **Cancel leaves:** Nothing โ€” site files stay as-is on disk. - 4. Ask the user to review using `AskUserQuestion`: > "The site is ready for review at ``. Please check it out in your browser. Would you like any changes?" 5. If the user requests changes, apply them and re-verify by browsing via `browser_snapshot` @@ -604,6 +603,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. +> **Blast radius if skipped:** 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: @@ -612,14 +619,6 @@ Present a summary table to the user: Follow the skill tracking instructions in the reference to record this skill's usage. Use `--skillName "CreateSite"`. Note: `.powerpages-site` may not exist for first-time sites โ€” the script exits silently. - - -> ๐Ÿšฆ **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. -> -> **Trigger:** Phase 8 entry; Phase 7 review approved. -> **Blast radius if skipped:** 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. - 2. Use `AskUserQuestion` with options: **Deploy now (Recommended)**, **Skip for now**: > "Would you like to deploy your site to Power Pages now?" 3. If the user chooses to deploy, invoke the `/deploy-site` skill. diff --git a/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md b/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md index 6852386bc..1138ab2ff 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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). +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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..6c5c76a0f 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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..5667b3abe 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. +> **Blast radius if skipped:** 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. +> **Blast radius if skipped:** 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 15e2ebf24..b7a4603b1 100644 --- a/plugins/power-pages/skills/integrate-backend/SKILL.md +++ b/plugins/power-pages/skills/integrate-backend/SKILL.md @@ -268,7 +268,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: diff --git a/plugins/power-pages/skills/integrate-webapi/SKILL.md b/plugins/power-pages/skills/integrate-webapi/SKILL.md index a08ac4520..92609294d 100644 --- a/plugins/power-pages/skills/integrate-webapi/SKILL.md +++ b/plugins/power-pages/skills/integrate-webapi/SKILL.md @@ -328,6 +328,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. +> **Blast radius if skipped:** 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: @@ -346,14 +354,6 @@ If the user chooses to upload an existing diagram: 5. Generate a Mermaid flowchart from the parsed data (if the user provided an image or text) for visual confirmation. - - -> ๐Ÿšฆ **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. -> -> **Trigger:** Path A โ€” diagram parsed and Mermaid flowchart rendered. -> **Blast radius if skipped:** 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. - 6. Present the parsed permissions plan to the user for approval using `AskUserQuestion`: | Question | Options | diff --git a/plugins/power-pages/skills/plan-alm/SKILL.md b/plugins/power-pages/skills/plan-alm/SKILL.md index d75ee8431..b54be646c 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. +> **Blast radius if skipped:** 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/setup-auth/SKILL.md b/plugins/power-pages/skills/setup-auth/SKILL.md index 9fe0a4dc0..13018ca74 100644 --- a/plugins/power-pages/skills/setup-auth/SKILL.md +++ b/plugins/power-pages/skills/setup-auth/SKILL.md @@ -111,7 +111,7 @@ Read each file and compile a list of existing web roles (name, id, flags). > **Blast radius if skipped:** 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 if they want to create them first: +**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 | |----------|---------| diff --git a/plugins/power-pages/skills/setup-pipeline/SKILL.md b/plugins/power-pages/skills/setup-pipeline/SKILL.md index f2b2d0291..c5745ccde 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. +> **Blast radius if skipped:** 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`. +> **Blast radius if skipped:** 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..9b3d28efa 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. +> **Blast radius if skipped:** 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 | |----------|--------|---------| From 8d02547a0d366c522f6f31f8c0aed0143913b751 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 18:20:26 +0530 Subject: [PATCH 04/10] Address PR review feedback: lint upgrades + sweeping refinements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts an accidental scope expansion, adds two new lint rules, closes the runtime-loop coverage hole, renames the awkward 'Blast radius if skipped' field, and removes dead module-level code. All 9 feedback items from the pre-PR review are addressed. LINT: - CATALOG_GATE_ID_PATTERN: revert underscore that was silently added โ€” was [A-Za-z0-9_-]*, now [A-Za-z0-9-]* matching the original kebab-only intent. The case-insensitivity change to [A-Za-z] (from [a-z]) is kept, with a comment explaining the asymmetric grammar with GATE_MARKER_PATTERN. - New rule: CATALOG-row-must-have-marker โ€” reverse check (catalog row โ†’ SKILL.md marker). Catches the orphan-row class of bug v3 closed by hand. Parses ยง6 catalog tables for rows tagged 'gate' and verifies each ID has a matching marker in some SKILL.md. - New rule: GATE-prose-block-required โ€” every gate marker must be followed within 10 lines by a line carrying the ๐Ÿšฆ sentinel. Catches prose-block deletion without forcing structural rewrite of 80+ legacy v2 single-line markers. - Tightened m < promptLine (strict precede) with a new test asserting marker-on-prompt-line fails the rule. - Removed dead ALM_SKILLS export + its membership test โ€” no downstream consumer existed. - Simplified SKILL_SEVERITY comment (the wishful 'future per-skill policy' framing). CATALOG: - ยง3.2 plan-vs-consent: added a paragraph explaining why deploy-dispatch prompts ('Deploy now?') are tagged plan when the destructive consent lives inside the dispatched skill's own gate. Resolves the create-site:8.deploy (plan) vs deploy-site:3.confirm-env (consent) apparent inconsistency. - ยง6.4 / ยง6.7 / ยง6.9: removed archaeological commentary about merged / removed rows. Git blame has it. - ยง6.24a: rewritten to reflect that the runtime-loop coverage hole is now CLOSED โ€” manage-firewall and scan-site got real call-site anchors. - ยง10: added cross-plugin lint TODO for shared/skills/report-issue/; expanded the v3 PR changelog with the new lint rules + field rename. - ยง11: added phase-number-drift convention (any SKILL.md phase renumber must grep the catalog), runtime-loop coverage caveat (now mostly resolved), and lint-prose-block weakness (only ๐Ÿšฆ enforced, not the 3 structured labels). RUNTIME-LOOP ANCHORS (F13 โ€” closes the coverage hole): - manage-firewall: new gate manage-firewall:3.action-choice (plan, at '### Default approach') + manage-firewall:3.execute-consent (consent, at '### Plan-validate-execute'). The destructive WAF mutations (enable/disable/add/update/delete rule) are now lint-anchored. - scan-site: new gate scan-site:3.action-choice (plan, at '### Default approach'). The scan-trigger decision is now lint-anchored. - Both prose blocks updated to include the literal 'AskUserQuestion:' string that PROMPT_LINE_PATTERN requires. FIELD RENAME (F14): - 'Blast radius if skipped:' โ†’ 'Why we ask:' across all 60 prose blocks + ยง4.1 catalog template. The old label suggested the gate's purpose was to describe Cancel state; the actual content explains why the gate prompts in the first place. VERIFIED: lint reports 0 findings (with new reverse + prose-block rules active); 984/984 plugin tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../power-pages/references/approval-gates.md | 46 +++--- .../power-pages/scripts/lint-skills-alm.js | 137 ++++++++++++++---- .../scripts/tests/lint-skills-alm.test.js | 34 +++-- .../skills/add-cloud-flow/SKILL.md | 10 +- .../skills/add-sample-data/SKILL.md | 4 +- plugins/power-pages/skills/add-seo/SKILL.md | 2 +- .../skills/add-server-logic/SKILL.md | 16 +- .../skills/audit-permissions/SKILL.md | 2 +- .../power-pages/skills/create-site/SKILL.md | 10 +- .../skills/create-webroles/SKILL.md | 6 +- .../power-pages/skills/deploy-site/SKILL.md | 12 +- .../skills/ensure-pipelines-host/SKILL.md | 12 +- .../skills/export-solution/SKILL.md | 4 +- .../skills/force-link-environment/SKILL.md | 4 +- .../skills/integrate-backend/SKILL.md | 2 +- .../skills/integrate-webapi/SKILL.md | 10 +- .../skills/manage-firewall/SKILL.md | 20 ++- .../skills/manage-headers/SKILL.md | 2 +- plugins/power-pages/skills/plan-alm/SKILL.md | 2 +- plugins/power-pages/skills/scan-site/SKILL.md | 10 +- .../skills/security-review/SKILL.md | 4 +- .../power-pages/skills/setup-auth/SKILL.md | 10 +- .../skills/setup-datamodel/SKILL.md | 4 +- .../skills/setup-pipeline/SKILL.md | 4 +- plugins/power-pages/skills/test-site/SKILL.md | 2 +- 25 files changed, 242 insertions(+), 127 deletions(-) diff --git a/plugins/power-pages/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index 278078d62..b1d4b9641 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -80,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. @@ -151,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. ``` @@ -194,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.] @@ -340,9 +342,7 @@ Each section lists every `AskUserQuestion` in that skill. Catalog rows are marke | `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 | -The previously-listed `deploy-pipeline:6.1.pac-fallback-consent` row was merged into `6.0.final-consent` โ€” the 6.0 marker's prose explicitly covers both the `DeployPackageAsync` and `pac pipeline deploy` paths, so a separate ID would have been redundant and the new row produced no second prompt. - -(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.) --- @@ -385,7 +385,7 @@ The previously-listed `deploy-pipeline:6.1.pac-fallback-consent` row was merged |---|---|---|---|---|---| | `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"* โ€” the per-stage values matrix is built inside this same multi-question prompt, so a separate "confirm matrix" gate would be redundant. | 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 | --- @@ -417,8 +417,6 @@ The previously-listed `deploy-pipeline:6.1.pac-fallback-consent` row was merged | `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 | -The previously-listed `2.host-fallback` / `2.dev-fallback` not-a-gate rows were redundant โ€” they described the free-text fallback option of the gate above, not a separate prompt. - --- ### 6.10 `activate-site` (4 calls) @@ -607,22 +605,29 @@ When **removing** a gate, also remove its catalog row in the same PR. --- -### 6.24a Security skills โ€” runtime-loop pattern +### 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. -The four security skills introduced in PR #151 (`manage-firewall`, `manage-headers`, `scan-site`, `security-review`) use `AskUserQuestion` differently from the other skills: they don't have phase-numbered, statically-locatable prompt sites. Most calls happen inside a "recommend then ask" runtime loop that's described in prose (e.g., *"recommend the single most relevant action"* in `manage-firewall` ยง3 and `scan-site` ยง3). The marker convention treats these as follows: +v3 closed this coverage hole by surfacing the recommend-then-ask block as a real call site in the prose: -- **Meta-mention sections** (`### Option rules` in `manage-firewall` and `scan-site`) are tagged `` โ€” they document HOW to construct prompts, they aren't prompt sites themselves. -- **Concrete prompt sites** that exist as literal "`AskUserQuestion`:" blocks (e.g. `manage-headers` ยง3 per-finding loop, `security-review` ยง2.1 and ยง5.3) get full `` markers. +- `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. -Future hardening (out of scope for v3): the runtime-loop calls in `manage-firewall` and `scan-site` could be made statically locatable by moving the "recommend an action" block into a numbered subsection like `### 3.1 Recommend` with a literal AskUserQuestion code example. The PR #151 authors can revisit if the loop pattern proves hard to audit. +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` (1 lint-caught meta-mention + dynamic runtime loop) +### 6.25 `manage-firewall` (3 gate IDs) | ID | Kind | Category | Phase | Trigger / question | Cancel leaves | |---|---|---|---|---|---| -| `manage-firewall:3.option-rules-meta` | not-a-gate | โ€” | 3 (`### Option rules`) | Documentation describing how to structure `AskUserQuestion` options in this skill โ€” not a call site itself. The actual destructive firewall changes (enable/disable/add-rule/remove-rule) gate via the prose-described "apply only after user approval" rule in ยง3 Plan-validate-execute and ยง4 Apply the change. | โ€” | +| `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. | โ€” | --- @@ -634,11 +639,12 @@ Future hardening (out of scope for v3): the runtime-loop calls in `manage-firewa --- -### 6.27 `scan-site` (1 lint-caught meta-mention + dynamic runtime loop) +### 6.27 `scan-site` (2 gate IDs) | ID | Kind | Category | Phase | Trigger / question | Cancel leaves | |---|---|---|---|---|---| -| `scan-site:3.option-rules-meta` | not-a-gate | โ€” | 3 (`### Option rules`) | Documentation describing how to structure `AskUserQuestion` options. The actual prompt โ€” *"use existing report / run a fresh scan"* โ€” fires dynamically in ยง3 Default approach and triggers a long-running scan but no destructive site change. | โ€” | +| `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. | โ€” | --- @@ -696,7 +702,8 @@ These need explicit confirmation from the reviewer before SKILL.md edits land. R - 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). +- `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. --- @@ -708,3 +715,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 hole (manage-firewall, scan-site).** ยง6.24a documents this: the destructive WAF mutations in `manage-firewall` ยง3 Plan-validate-execute and the scan-trigger choices in `scan-site` ยง3 Default approach happen inside a "recommend then ask" loop with no statically-locatable `AskUserQuestion`:` call site. The not-a-gate markers in those skills satisfy lint without anchoring the actual destructive prompts. Tracked as a known gap โ€” restructuring the prose to surface real call sites is the path forward but was deferred from v3 to keep the PR scoped. +- **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/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 4e1979930..0aa77fac3 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -66,6 +66,26 @@ // 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: yes โ€” inline `` +// in the catalog row's section, or `.almlintignore` entry. +// +// 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 @@ -187,28 +207,8 @@ const KNOWN_RULES = new Set([ 'GATE-must-be-in-catalog', 'GATE-intent-must-call-helper', 'GATE-cancel-leaves-known-vocab', -]); - -// v2 had a separate ALM_SKILLS set used to flip lint severity between hard-fail -// (ALM) and warn-only (non-ALM). v3 removed the carve-out โ€” every skill under -// plugins/power-pages/skills/ is hard-fail because the catalog in -// references/approval-gates.md now covers all skills (see ยง10 landing history). -// The set is kept as the canonical "ALM skill family" list for reference and -// for downstream tooling that needs to enumerate ALM skills, but it no longer -// influences lint severity. -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. @@ -442,10 +442,7 @@ function skillNameFromFile(file) { return parts[idx + 1]; } -// v3: every skill under plugins/power-pages/skills/ is enforced at hard-fail. -// Kept as a named constant rather than a no-op function so the call sites are -// honest about the lack of per-skill policy. If a future PR needs per-skill -// severity, restore a function here and pass the skill name in. +// v3: hard-fail uniformly across every SKILL.md under plugins/power-pages/skills/. const SKILL_SEVERITY = 'error'; // Parse the catalog file (references/approval-gates.md) and extract all @@ -453,11 +450,13 @@ const SKILL_SEVERITY = 'error'; // present (downgrades GATE-must-be-in-catalog to no-op so the lint isn't // hard-broken when the catalog is removed/renamed). // -// Case-insensitive 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 or underscored skill would fail GATE-must-be-in-catalog -// even with a correct catalog row. -const CATALOG_GATE_ID_PATTERN = /`([A-Za-z][A-Za-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'); @@ -472,6 +471,26 @@ 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. +const CATALOG_GATE_ROW_PATTERN = + /^\|\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) { @@ -604,6 +623,37 @@ function collectFindings({ pluginRoot }) { } } + // GATE-prose-block-required โ€” every `` must be followed + // within 10 lines by a line carrying the ๐Ÿšฆ sentinel. 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. + if (!ignores.has('GATE-prose-block-required')) { + const lines = content.split(/\r?\n/); + for (const gm of gateMarkers) { + const startIdx = gm.lineNum - 1; // 0-based + const windowLines = lines.slice(startIdx + 1, startIdx + 11); + const hasSentinel = windowLines.some((l) => l.includes('๐Ÿšฆ')); + if (!hasSentinel) { + findings.push({ + rule: 'GATE-prose-block-required', + severity: 'error', + file, + message: + `Gate \`${gm.gateId}\` (line ${gm.lineNum}) is missing the ๐Ÿšฆ ` + + `prose block within 10 lines. 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) { @@ -664,6 +714,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: 'error', + 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'); @@ -740,7 +814,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 682fb1117..b156636dd 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, @@ -615,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( @@ -760,18 +780,6 @@ 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) { diff --git a/plugins/power-pages/skills/add-cloud-flow/SKILL.md b/plugins/power-pages/skills/add-cloud-flow/SKILL.md index 324b2aee4..273d78865 100644 --- a/plugins/power-pages/skills/add-cloud-flow/SKILL.md +++ b/plugins/power-pages/skills/add-cloud-flow/SKILL.md @@ -81,7 +81,7 @@ Look for the `.powerpages-site` folder in the project root. > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Cloud flow YAML written to a non-existent path will never deploy. +> **Why we ask:** Cloud flow YAML written to a non-existent path will never deploy. > **Cancel leaves:** Nothing โ€” no YAML files written. Use `AskUserQuestion`: @@ -203,7 +203,7 @@ Already registered (available for additional frontend integration): > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong flows get registered (new `.cloudflowconsumer.yml` files written) or wrong existing flows get re-wired into the frontend. +> **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`: @@ -365,7 +365,7 @@ Give a brief CLI summary: number of flows, scenarios, role count, any anonymous- > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong web role assignments committed (especially Anonymous Users on auth-protected flows); orphaned YAML files in `.powerpages-site/cloud-flow-consumer/`. +> **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`: @@ -613,7 +613,7 @@ Use `--skillName "AddCloudFlow"`. > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks wrong env. +> **Why we ask:** Auto-deploy picks wrong env. > **Cancel leaves:** Nothing โ€” artifacts stay on disk; no deploy fired. Use `AskUserQuestion`: @@ -627,7 +627,7 @@ Use `AskUserQuestion`: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Skipping is harmless (manual test still possible); auto-invoking `/test-site` adds runtime and Playwright traffic. +> **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`: diff --git a/plugins/power-pages/skills/add-sample-data/SKILL.md b/plugins/power-pages/skills/add-sample-data/SKILL.md index 6ec9e78a1..67ed908b7 100644 --- a/plugins/power-pages/skills/add-sample-data/SKILL.md +++ b/plugins/power-pages/skills/add-sample-data/SKILL.md @@ -85,7 +85,7 @@ Show the user the list of discovered tables with their columns so they can choos > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-selecting all tables can pollute production-shape tables (e.g. `contact`) with throwaway records. +> **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. @@ -97,7 +97,7 @@ Use `AskUserQuestion` to ask which tables they want to populate (use `multiSelec > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Inserting hundreds of records can blow past Dataverse storage / API throttling budgets. +> **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: diff --git a/plugins/power-pages/skills/add-seo/SKILL.md b/plugins/power-pages/skills/add-seo/SKILL.md index 1634e6997..6282df42f 100644 --- a/plugins/power-pages/skills/add-seo/SKILL.md +++ b/plugins/power-pages/skills/add-seo/SKILL.md @@ -120,7 +120,7 @@ Present the SEO additions that will be made as a clear, inline summary: > ๐Ÿšฆ **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). -> **Blast radius if skipped:** SEO assets land on disk with the wrong production URL, wrong exclusions, or unwanted OG tags โ€” fixable but noisy in git history. +> **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: diff --git a/plugins/power-pages/skills/add-server-logic/SKILL.md b/plugins/power-pages/skills/add-server-logic/SKILL.md index 08cd0db26..5aa8ec00f 100644 --- a/plugins/power-pages/skills/add-server-logic/SKILL.md +++ b/plugins/power-pages/skills/add-server-logic/SKILL.md @@ -108,7 +108,7 @@ Look for the `.powerpages-site` folder: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Server logic `.js`/`.yml` files written to a non-existent path won't deploy. +> **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`: @@ -195,7 +195,7 @@ If custom actions are found (`total > 0`), present a summary to the user grouped > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-wrapping could attach the wrong action; auto-skipping duplicates logic that already exists in Dataverse. +> **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`: @@ -271,7 +271,7 @@ If secrets were identified in Phase 2.3, ask the user now whether they want to u > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Plain env var creation can expose secrets in solution exports; auto-picking Key Vault forces additional Azure setup. +> **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`: @@ -400,7 +400,7 @@ Do not restate the per-server-logic breakdown, rationale, role assignments, or f > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Server logic files committed under wrong names / wrong roles; env var definitions created against the wrong secret-storage mode. +> **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`: @@ -897,7 +897,7 @@ 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. -> **Blast radius if skipped:** 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. +> **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`: @@ -1093,7 +1093,7 @@ Server logic creates the backend โ€” but without frontend code to call it, the e > ๐Ÿšฆ **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). -> **Blast radius if skipped:** Auto-integrating mutates UI files the user wanted to handle themselves; auto-skipping leaves the endpoints unreachable from the app. +> **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`: @@ -1231,7 +1231,7 @@ Present a summary of everything that was done: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks wrong env. +> **Why we ask:** Auto-deploy picks wrong env. > **Cancel leaves:** Nothing โ€” artifacts stay on disk; no deploy fired. Use `AskUserQuestion`: @@ -1245,7 +1245,7 @@ Use `AskUserQuestion`: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Skipping is harmless (manual test still possible); auto-invoking `/test-site` adds runtime. +> **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. diff --git a/plugins/power-pages/skills/audit-permissions/SKILL.md b/plugins/power-pages/skills/audit-permissions/SKILL.md index d079e4ca6..435042c3b 100644 --- a/plugins/power-pages/skills/audit-permissions/SKILL.md +++ b/plugins/power-pages/skills/audit-permissions/SKILL.md @@ -494,7 +494,7 @@ Follow the skill tracking instructions in the reference to record this skill's u > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Tooling could silently invoke the table-permissions-architect agent โ€” accept-by-default would write or mutate permission YAML against the user's intent. +> **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: diff --git a/plugins/power-pages/skills/create-site/SKILL.md b/plugins/power-pages/skills/create-site/SKILL.md index 0832312ed..e763593bf 100644 --- a/plugins/power-pages/skills/create-site/SKILL.md +++ b/plugins/power-pages/skills/create-site/SKILL.md @@ -77,7 +77,7 @@ Write the file with the `Write` tool (atomic overwrite). You do not need to read > ๐Ÿšฆ **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`. -> **Blast radius if skipped:** Wrong framework picked โ†’ wrong template copied into the wrong directory; cleanup is annoying. +> **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) @@ -232,7 +232,7 @@ Immediately after the dev server starts, verify the scaffold is working: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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**: @@ -365,7 +365,7 @@ Immediately after the user answers, `Write` the same file again with `"awaitingI > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Phase 5 rewrites the entire scaffold (theme.css, Layout, Home page, components, routes) โ€” undoing that touches every commit in the implementation phase. +> **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`: @@ -571,7 +571,7 @@ Present a summary table to the user: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** User loses the chance to spot UI issues before deploy; broken pages get pushed. +> **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**: @@ -608,7 +608,7 @@ Present a summary table to the user: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks whatever env PAC CLI happens to be pointing at โ€” wrong-env first deploy is messy to undo. +> **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**: diff --git a/plugins/power-pages/skills/create-webroles/SKILL.md b/plugins/power-pages/skills/create-webroles/SKILL.md index 4a7356cd8..eb3e27a64 100644 --- a/plugins/power-pages/skills/create-webroles/SKILL.md +++ b/plugins/power-pages/skills/create-webroles/SKILL.md @@ -51,7 +51,7 @@ Create web roles for a Power Pages code site. Web roles define the permissions a > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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:** Ask the user to deploy first via `AskUserQuestion` (options: "Yes, deploy now (Recommended)", "No, I'll do it later"). If yes, invoke `/deploy-site` then resume from Phase 2. If no, stop. @@ -106,7 +106,7 @@ Create web roles for a Power Pages code site. Web roles define the permissions a > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong roles get created locally โ€” fixable but adds churn to the `.powerpages-site/web-roles/` folder. +> **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. @@ -224,7 +224,7 @@ name: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-invoking `/deploy-site` would push the site to whatever env PAC CLI happens to point at โ€” wrong-env push is messy to undo. +> **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. Then ask the user if they want to deploy the site to apply the new roles: diff --git a/plugins/power-pages/skills/deploy-site/SKILL.md b/plugins/power-pages/skills/deploy-site/SKILL.md index c80270fe5..25b5dd80f 100644 --- a/plugins/power-pages/skills/deploy-site/SKILL.md +++ b/plugins/power-pages/skills/deploy-site/SKILL.md @@ -121,7 +121,7 @@ Guide the user through deploying an existing Power Pages code site to a Power Pa > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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. @@ -175,7 +175,7 @@ Determine the project root directory. The project root is the directory containi > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong project uploaded โ€” pollutes the target env with files from a different site. +> **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`. @@ -191,7 +191,7 @@ If `.powerpages-site` already exists (i.e., this is not the first deployment), t > ๐Ÿšฆ **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). -> **Blast radius if skipped:** Stale permission YAML deploys, causing 403s for users until next audit run. +> **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`: @@ -286,7 +286,7 @@ Evaluate the JSON result: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-activating writes the wrong subdomain (permanent for the site); auto-skipping leaves the site without a live URL. +> **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`: @@ -309,7 +309,7 @@ After confirming the site is activated (either it was already activated in step > ๐Ÿšฆ **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). -> **Blast radius if skipped:** Production users see stale content for several minutes until cache TTL expires. +> **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: @@ -355,7 +355,7 @@ Tell the user: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-unblocking modifies a tenant security setting without explicit consent โ€” visible across the entire environment, not just this site. +> **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`: diff --git a/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md b/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md index 1138ab2ff..03866e39c 100644 --- a/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md +++ b/plugins/power-pages/skills/ensure-pipelines-host/SKILL.md @@ -427,7 +427,7 @@ A PE already exists in the tenant (one is provisioned automatically the first ti > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking provisions an env (PE or Custom Host) without consent; PE is tenant-singleton and admin-non-deletable. +> **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. @@ -435,7 +435,7 @@ A PE already exists in the tenant (one is provisioned automatically the first ti > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking the wrong env routes pipelines through a host the user didn't intend. +> **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. @@ -591,7 +591,7 @@ Surface the specific failure to the user. Out of automated remediation scope. Re > ๐Ÿšฆ **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). -> **Blast radius if skipped:** PE provisioned in wrong tenant; cannot be deleted by tenant admin. +> **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`. @@ -634,7 +634,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/lib/provision-platform-host.js" \ > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Custom Host provisioned in wrong tenant or wrong region; consumes Azure capacity quota; potentially attributes the env to the wrong organization. +> **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. @@ -642,7 +642,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/lib/provision-platform-host.js" \ > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Installing Pipelines onto a Sandbox env without consent โ€” sandbox envs are deletable on inactivity and have reduced capacity; pipelines may break unexpectedly. +> **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`. @@ -796,7 +796,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/lib/install-pipelines-app.js" \ > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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). +> **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."* diff --git a/plugins/power-pages/skills/export-solution/SKILL.md b/plugins/power-pages/skills/export-solution/SKILL.md index 6c5c76a0f..1e53baea9 100644 --- a/plugins/power-pages/skills/export-solution/SKILL.md +++ b/plugins/power-pages/skills/export-solution/SKILL.md @@ -134,7 +134,7 @@ Cap this step at ~30 seconds. If MCP search / fetch errors out, log a one-line n > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking the wrong solution exports a managed zip that ships the wrong table/site/flow set to staging. +> **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')`) @@ -240,7 +240,7 @@ Use the answer to set `"Managed": true` or `"Managed": false` in the `ExportSolu > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-overwriting replaces a previous export that may have been hand-tested already. +> **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`: diff --git a/plugins/power-pages/skills/force-link-environment/SKILL.md b/plugins/power-pages/skills/force-link-environment/SKILL.md index 5667b3abe..0083f44df 100644 --- a/plugins/power-pages/skills/force-link-environment/SKILL.md +++ b/plugins/power-pages/skills/force-link-environment/SKILL.md @@ -93,7 +93,7 @@ Confirm the *"Using Force Linkโ€ฆ"* section's current warnings before proceeding > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking the wrong host runs `ManageEnvironmentStamp` against the wrong tenant and moves the stamp irreversibly without consent. +> **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. @@ -101,7 +101,7 @@ Confirm the *"Using Force Linkโ€ฆ"* section's current warnings before proceeding > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking the wrong dev env relinks a different env to the new host โ€” makers of the wrong env lose pipeline access. +> **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`: diff --git a/plugins/power-pages/skills/integrate-backend/SKILL.md b/plugins/power-pages/skills/integrate-backend/SKILL.md index b7a4603b1..f177118bd 100644 --- a/plugins/power-pages/skills/integrate-backend/SKILL.md +++ b/plugins/power-pages/skills/integrate-backend/SKILL.md @@ -302,7 +302,7 @@ In the CLI, give only a brief summary: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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`: diff --git a/plugins/power-pages/skills/integrate-webapi/SKILL.md b/plugins/power-pages/skills/integrate-webapi/SKILL.md index 92609294d..e96421271 100644 --- a/plugins/power-pages/skills/integrate-webapi/SKILL.md +++ b/plugins/power-pages/skills/integrate-webapi/SKILL.md @@ -159,7 +159,7 @@ Show the user: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-selecting all tables can generate orphaned TypeScript files for tables the user never intended to expose via Web API. +> **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. Use `AskUserQuestion` to confirm: @@ -292,7 +292,7 @@ Both agents require the `.powerpages-site` folder. If it 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. -> **Blast radius if skipped:** Auto-skipping leaves the integration broken (no permissions to back the Web API calls); auto-deploying picks the wrong env. +> **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`: @@ -312,7 +312,7 @@ Use `AskUserQuestion`: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Path A produces table permissions matching a stale or wrong diagram; Path B can take minutes to query Dataverse. +> **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. Ask the user how they want to define the permissions using the `AskUserQuestion` tool: @@ -333,7 +333,7 @@ Route to the appropriate path: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong scope / wrong CRUD flags get committed to `.powerpages-site/table-permissions/` โ€” fixable but noisy in git history. +> **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: @@ -515,7 +515,7 @@ Present a summary of everything that was done: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks whatever env PAC CLI happens to point at. +> **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. Use `AskUserQuestion`: diff --git a/plugins/power-pages/skills/manage-firewall/SKILL.md b/plugins/power-pages/skills/manage-firewall/SKILL.md index 044fb2631..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. @@ -153,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. @@ -161,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 2a7becbd7..d326f22b0 100644 --- a/plugins/power-pages/skills/manage-headers/SKILL.md +++ b/plugins/power-pages/skills/manage-headers/SKILL.md @@ -102,7 +102,7 @@ Read `references/headers-reference.md` for recommended values and guidance. **Pr > ๐Ÿšฆ **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`. -> **Blast radius if skipped:** 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. +> **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`: diff --git a/plugins/power-pages/skills/plan-alm/SKILL.md b/plugins/power-pages/skills/plan-alm/SKILL.md index b54be646c..88e5eccc1 100644 --- a/plugins/power-pages/skills/plan-alm/SKILL.md +++ b/plugins/power-pages/skills/plan-alm/SKILL.md @@ -450,7 +450,7 @@ For each stage, populate `envName` from `ENV_LIST` (gathered in Phase 1 Step 5 v > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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):** diff --git a/plugins/power-pages/skills/scan-site/SKILL.md b/plugins/power-pages/skills/scan-site/SKILL.md index 28e2d1c58..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. diff --git a/plugins/power-pages/skills/security-review/SKILL.md b/plugins/power-pages/skills/security-review/SKILL.md index a0eb053f5..92842103a 100644 --- a/plugins/power-pages/skills/security-review/SKILL.md +++ b/plugins/power-pages/skills/security-review/SKILL.md @@ -99,7 +99,7 @@ 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. -> **Blast radius if skipped:** Auto-picking `release` runs ALL sub-skills (slow; possibly hits scan/firewall endpoints unnecessarily); auto-picking the wrong goal mis-scopes the review. +> **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. @@ -269,7 +269,7 @@ Open `` in the user's default browser. > ๐Ÿšฆ **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. -> **Blast radius if skipped:** 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. +> **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`: diff --git a/plugins/power-pages/skills/setup-auth/SKILL.md b/plugins/power-pages/skills/setup-auth/SKILL.md index 13018ca74..1a2b78e91 100644 --- a/plugins/power-pages/skills/setup-auth/SKILL.md +++ b/plugins/power-pages/skills/setup-auth/SKILL.md @@ -80,7 +80,7 @@ Look for the `.powerpages-site` folder: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks the wrong env; skipping leaves auth wiring broken. +> **Why we ask:** Auto-deploy picks the wrong env; skipping leaves auth wiring broken. > **Cancel leaves:** Nothing โ€” no auth files written yet. Use `AskUserQuestion`: @@ -108,7 +108,7 @@ Read each file and compile a list of existing web roles (name, id, flags). > ๐Ÿšฆ **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/`. -> **Blast radius if skipped:** Auto-invoking `/create-webroles` runs another full skill; auto-skipping leaves RBAC checks against an empty role set. +> **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: @@ -155,7 +155,7 @@ If auth files already exist, present them to the user and ask whether to overwri > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong feature set gets generated โ€” e.g. building RBAC files when the user only wanted login. +> **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. Use `AskUserQuestion` to determine the scope: @@ -184,7 +184,7 @@ Present the implementation plan inline: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Wrong components generated; site settings written; ProfileRedirectEnabled flipped โ€” fixable but adds churn. +> **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: @@ -551,7 +551,7 @@ Present a summary of everything created: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-deploy picks the wrong env. +> **Why we ask:** Auto-deploy picks the wrong env. > **Cancel leaves:** Nothing โ€” auth artifacts stay on disk; no deploy fired. Use `AskUserQuestion`: diff --git a/plugins/power-pages/skills/setup-datamodel/SKILL.md b/plugins/power-pages/skills/setup-datamodel/SKILL.md index dbf85833b..0495d3627 100644 --- a/plugins/power-pages/skills/setup-datamodel/SKILL.md +++ b/plugins/power-pages/skills/setup-datamodel/SKILL.md @@ -49,7 +49,7 @@ Guide the user through creating Dataverse tables, columns, and relationships for > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-picking either path can run a multi-minute architect agent against the wrong intent (Path B) or skip Dataverse-existence checks (Path A). +> **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: @@ -150,7 +150,7 @@ Present the data model proposal directly to the user as a formatted message, inc > ๐Ÿšฆ **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). -> **Blast radius if skipped:** Tables and columns get created in Dataverse against the user's actual schema intent โ€” column types and relationship cardinalities are awkward to undo. +> **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: diff --git a/plugins/power-pages/skills/setup-pipeline/SKILL.md b/plugins/power-pages/skills/setup-pipeline/SKILL.md index c5745ccde..6adb631ca 100644 --- a/plugins/power-pages/skills/setup-pipeline/SKILL.md +++ b/plugins/power-pages/skills/setup-pipeline/SKILL.md @@ -288,7 +288,7 @@ 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. -> **Blast radius if skipped:** Either a foreign pipeline gets its stages overwritten, or a duplicate pipeline gets created that pollutes the host env's pipeline list. +> **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. @@ -491,7 +491,7 @@ 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`. -> **Blast radius if skipped:** Auto-migrating deletes Dataverse pipeline records โ€” destructive against host env state, irreversible without re-running setup-pipeline. +> **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. diff --git a/plugins/power-pages/skills/test-site/SKILL.md b/plugins/power-pages/skills/test-site/SKILL.md index 9b3d28efa..478f17de5 100644 --- a/plugins/power-pages/skills/test-site/SKILL.md +++ b/plugins/power-pages/skills/test-site/SKILL.md @@ -405,7 +405,7 @@ For each failed API request, provide specific remediation: > ๐Ÿšฆ **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. -> **Blast radius if skipped:** Auto-submitting test data into production records pollutes real customer data. +> **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: From f4c8ced07dbbc4817c7ceace06ca088f1c7f659f Mon Sep 17 00:00:00 2001 From: T-Nid Date: Tue, 26 May 2026 18:46:04 +0530 Subject: [PATCH 05/10] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- plugins/power-pages/references/approval-gates.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/power-pages/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index b1d4b9641..026f51c08 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -6,7 +6,7 @@ > > **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 the five GATE 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`) at error severity on every SKILL.md under `plugins/power-pages/skills/`. +> **Lint is hard-fail for every skill.** `scripts/lint-skills-alm.js` enforces seven 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`, `CATALOG-row-must-have-marker`, `GATE-prose-block-required`) at error severity on every SKILL.md under `plugins/power-pages/skills/`. --- From b42ddc6d43b64cffc60b53dab3a3647214c300fe Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 21:46:57 +0530 Subject: [PATCH 06/10] Address PR review feedback round 2: lint robustness + test coverage + doc consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 10 review-skill findings + 9 Copilot inline comments + the Copilot Autofix on the rule-count wording. ## Lint script (scripts/lint-skills-alm.js) LINT REGEX: - CATALOG_GATE_ROW_PATTERN now tolerates up to 3 leading spaces before the pipe (GFM table indentation tolerance). Was `^\|`, now `^\s{0,3}\|`. Without this, a future markdown reformat that nests ยง6 tables under a parent list silently disables orphan detection for those rows. GATE-prose-block-required: - Window now INCLUDES the marker's own line (slice(startIdx, startIdx+10), was slice(startIdx+1, startIdx+11)). Single-line compact-style markers with marker + ๐Ÿšฆ on one line now pass. - The ๐Ÿšฆ search now skips code-fence regions. Was a plain `.includes('๐Ÿšฆ')` per line; now pre-computes per-line fence state across the full file and ignores ๐Ÿšฆ inside ```...``` blocks. Closes the "future contributor deletes the real Gate prose block but leaves an example ๐Ÿšฆ in a bash example within the window" silent-pass case. CATALOG-row-must-have-marker: - Docstring updated. Previously claimed "Waivable: yes โ€” inline in the catalog row's section". The implementation never called extractIgnores on the catalog file, so inline-ignore never worked. New docstring says .almlintignore allowlist is the only suppression mechanism (the rule operates on the catalog as a whole, not per SKILL.md, so per-row inline ignore would be confusing to model). Matches actual behavior. CLEANUPS: - Removed dead `skillNameFromFile()` function. No callers since severityForSkill was deleted; not in module.exports. - Removed `SKILL_SEVERITY` constant. Half-applied (2 of 9 severity: sites used it) โ€” inlined `'error'` everywhere for consistency. - Updated Usage header comment to match v3 exit-code behavior. - main() now concatenates findings into one string + single process.stderr.write, then sets process.exitCode instead of calling process.exit. Prevents stderr-truncation on Windows when piping to a log file with large finding counts. ## Tests (scripts/tests/lint-skills-alm.test.js) Added 7 new tests for the two rules introduced in commit 8d02547 that shipped with zero coverage: CATALOG-row-must-have-marker: - fires when a catalog gate row has no SKILL.md marker - passes when every gate row has a marker - skips not-a-gate rows (no marker required for them) - tolerates leading whitespace on table rows (GFM compliance) GATE-prose-block-required: - fires when marker has no ๐Ÿšฆ within 10 lines - passes when ๐Ÿšฆ sentinel follows within window - ignores ๐Ÿšฆ inside a fenced code block - tolerates ๐Ÿšฆ on the same line as the marker (single-line style) Test count: 45 โ†’ 52 (+7). Plugin-wide: 984 โ†’ 992. ## Docs approval-gates.md / AGENTS.md / PLUGIN_DEVELOPMENT_GUIDE.md: - Updated "five GATE rules" โ†’ "seven gate-related rules" (Copilot's finding). Lists all 7 enforced rules now. Includes the v3 additions CATALOG-row-must-have-marker and GATE-prose-block-required. approval-gates.md ยง9 Decisions row 4: - Marked the v2 "ALM: hard-fail. Non-ALM: warn-only" decision as superseded by ยง10. The ยง10 history correctly captures the v2โ†’v3 transition but ยง9 read as a current recommendation in isolation. Verified: lint 0 findings; 992/992 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- plugins/power-pages/AGENTS.md | 2 +- .../power-pages/PLUGIN_DEVELOPMENT_GUIDE.md | 2 +- .../power-pages/references/approval-gates.md | 4 +- .../power-pages/scripts/lint-skills-alm.js | 106 ++++++---- .../scripts/tests/lint-skills-alm.test.js | 183 ++++++++++++++++++ 5 files changed, 257 insertions(+), 40 deletions(-) diff --git a/plugins/power-pages/AGENTS.md b/plugins/power-pages/AGENTS.md index 92f27b533..655967d24 100644 --- a/plugins/power-pages/AGENTS.md +++ b/plugins/power-pages/AGENTS.md @@ -241,7 +241,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 **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 five 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`). ยง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. +- `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. diff --git a/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md b/plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md index eb9bab943..197bd13cf 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`). **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 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 026f51c08..b0e38c254 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -6,7 +6,7 @@ > > **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 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`, `CATALOG-row-must-have-marker`, `GATE-prose-block-required`) at error severity on every SKILL.md under `plugins/power-pages/skills/`. +> **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`). --- @@ -685,7 +685,7 @@ 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. | diff --git a/plugins/power-pages/scripts/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 0aa77fac3..bf8ce4d6e 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -73,8 +73,11 @@ // 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: yes โ€” inline `` -// in the catalog row's section, or `.almlintignore` entry. +// 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. @@ -88,8 +91,9 @@ // // 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. @@ -434,17 +438,6 @@ 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]; -} - -// v3: hard-fail uniformly across every SKILL.md under plugins/power-pages/skills/. -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 @@ -475,8 +468,13 @@ function loadCatalogGateIds(pluginRoot) { // 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*`([A-Za-z][A-Za-z0-9-]*:[A-Za-z0-9._-]+)`\s*\|\s*gate\s*\|/gm; + /^\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'); @@ -592,7 +590,7 @@ function collectFindings({ pluginRoot }) { for (const u of unmatched) { findings.push({ rule: 'GATE-must-have-marker', - severity: SKILL_SEVERITY, + severity: 'error', file, message: `Phase section "${u.heading}" contains an \`AskUserQuestion\` prompt (line ${u.lineNum}) ` + @@ -624,20 +622,47 @@ function collectFindings({ pluginRoot }) { } // GATE-prose-block-required โ€” every `` must be followed - // within 10 lines by a line carrying the ๐Ÿšฆ sentinel. 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. + // 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? + const inFence = new Array(lines.length).fill(false); + let inside = false; + for (let i = 0; i < lines.length; i++) { + // 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 โ€” we approximate with /^\s{0,3}```/). + if (/^\s{0,3}```/.test(lines[i])) inside = !inside; + inFence[i] = inside; + } for (const gm of gateMarkers) { const startIdx = gm.lineNum - 1; // 0-based - const windowLines = lines.slice(startIdx + 1, startIdx + 11); - const hasSentinel = windowLines.some((l) => l.includes('๐Ÿšฆ')); + 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', @@ -645,9 +670,10 @@ function collectFindings({ pluginRoot }) { file, message: `Gate \`${gm.gateId}\` (line ${gm.lineNum}) is missing the ๐Ÿšฆ ` + - `prose block within 10 lines. Every gate marker must be followed ` + - `by a \`> ๐Ÿšฆ **Gate (...)**\` line so humans see the same ` + - `context the lint sees.`, + `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.', }); } @@ -679,7 +705,7 @@ function collectFindings({ pluginRoot }) { if (!catalogGateIds.has(gm.gateId)) { findings.push({ rule: 'GATE-must-be-in-catalog', - severity: SKILL_SEVERITY, + severity: 'error', file, message: `Gate \`${gm.gateId}\` is declared in SKILL.md but is not in the catalog ` + @@ -796,15 +822,23 @@ function main(argv) { // 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. - for (const f of findings) process.stderr.write(formatFinding(f, pluginRoot)); - process.stderr.write( - `\nalm-lint: ${findings.length} error(s) in ${pluginRoot}\n` - ); + // + // 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 = { 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 b156636dd..57c86370b 100644 --- a/plugins/power-pages/scripts/tests/lint-skills-alm.test.js +++ b/plugins/power-pages/scripts/tests/lint-skills-alm.test.js @@ -786,3 +786,186 @@ test('CANCEL_LEAVES_VOCAB export has the documented values', () => { 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: 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); +}); From 371a6e161dc0a964a4a6dc72b67a2b7e70659ff8 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 22:04:13 +0530 Subject: [PATCH 07/10] Fix readSettingsFile to handle the keyed-object stages shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovered while validating PR #170 against the real Citizens portal site (C:\Projects\Citizens portal). The site's deployment-settings.json uses the Microsoft-standard 2024 schema shape: { "$schema": "https://schemas.microsoft.com/power-platform/deployment-settings/2024", "stages": { "Deploy to Staging": { "EnvironmentVariables": [...] } } } โ€” a keyed OBJECT of stages. readSettingsFile only handled two shapes: 1. Top-level `EnvironmentVariables: []` (single-stage) 2. `Stages: []` array with `{ Name, EnvironmentVariables }` entries The keyed-object shape returned 0 entries, which meant validate-deployment-settings.js silently passed even on known-broken values like `@KeyVault(vaultName=...;secretName=...)`. Real-world evidence: the Citizens portal's docs/alm/last-deploy.json records a failed deploy attempt (2026-05-21) where deployment-settings.json contained `@KeyVault(...)` for c311_api_secret. The pre-deploy validator "validation passed (validation does not check Secret reference format)" โ€” because the parser couldn't read the file. The deploy then waited ~4h in the host queue before failing with `ImportAsHolding failed: The value provided as a secret reference does not match a valid secret reference format`. This is exactly what the v3 deploy-pipeline:7.6.4.strip-secret- values gate is supposed to prevent โ€” but the gate's underlying validator was broken. Changes: - readSettingsFile now handles all three shapes. Each returned entry now carries `stageLabel` (null for shape 1, the stage name for shapes 2 and 3), so downstream consumers can attribute findings to the right stage without re-parsing the file. - validate-deployment-settings.js had a duplicated read-settings parser (readEntriesPreservingStage) with the same bug. Deleted โ€” it now uses readSettingsFile directly. - Tests: - Existing readSettingsFile tests updated to expect the new stageLabel field on entries. - New test: keyed-object shape (Microsoft 2024 schema). Validates the real-world fix against a fixture matching the Citizens portal file. - validate-deployment-settings.test.js renamed readEntriesPreservingStage references to readSettingsFile. Verified end-to-end against the real Citizens portal file: node validate-deployment-settings.js --settingsFile โ†’ summary { invalid: 1 }, one finding flagging the @KeyVault(...) value on c311_api_secret as 'kv-placeholder' / 'invalid'. Plugin tests: 993/993 (was 992; +1 new shape-3 test). Lint: 0 findings. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/validate-deployment-settings.js | 43 ++--------- .../scripts/lib/verify-env-var-values.js | 73 ++++++++++++++++--- .../validate-deployment-settings.test.js | 17 +++-- .../tests/verify-env-var-values.test.js | 49 ++++++++++++- 4 files changed, 124 insertions(+), 58 deletions(-) diff --git a/plugins/power-pages/scripts/lib/validate-deployment-settings.js b/plugins/power-pages/scripts/lib/validate-deployment-settings.js index 17c5edfd2..5223d06fd 100644 --- a/plugins/power-pages/scripts/lib/validate-deployment-settings.js +++ b/plugins/power-pages/scripts/lib/validate-deployment-settings.js @@ -326,45 +326,13 @@ 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 now returns +// `{ schemaName, value, stageLabel }` on each entry (so we don't need a +// duplicate parser to preserve stage attribution). 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); // Collect unique schema names for the type lookup pass. const uniqueSchemas = Array.from(new Set(entries.map((e) => e.schemaName).filter(Boolean))); @@ -452,7 +420,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..00d0539dc 100644 --- a/plugins/power-pages/scripts/lib/verify-env-var-values.js +++ b/plugins/power-pages/scripts/lib/verify-env-var-values.js @@ -87,10 +87,17 @@ 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. +// 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.) +// Returns `[{ schemaName, value, stageLabel }]` filtered to stageLabel when provided. +// stageLabel is included on each entry for shape 2/3; shape 1 sets stageLabel: null. function readSettingsFile(filePath, stageLabel) { let raw; try { @@ -105,35 +112,81 @@ 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); } 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 (`stages: { "": {...} }`). + // This is the schema emitted by configure-env-variables and the one + // Power Platform Pipelines actually accepts. Accept either casing of + // the `stages`/`Stages` key + match stage labels case-insensitively. + const stagesObj = parsed.stages || parsed.STAGES; + 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 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/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..9dbfb1786 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,47 @@ test('readSettingsFile with no stageLabel flattens Stages[]', async (t) => { assert.equal(entries[1].schemaName, 'foo_b'); }); +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')); }); From 5f7ebeb8b22817469d9f5ea34a4a94c60900a7e4 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 22:28:06 +0530 Subject: [PATCH 08/10] Fix two readSettingsFile regressions surfaced by Copilot review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Dedupe loss in validateSettings โ†’ readSettingsFile path readSettingsFile dedupes by schemaName when no stageLabel filter is provided. This is correct for callers like verify-env-var-values that want a single "configured value of X" per schema. But it is WRONG for validate-deployment-settings, which must inspect every stage's value independently: - Staging: c311_api_secret = "https://kv.../secrets/api-secret" (valid) - Production: c311_api_secret = "@KeyVault(vaultName=...)" (invalid) Pre-fix, dedupe kept Staging's value and Production's broken value was silently skipped. validateSettings would report invalid: 0 even though the deploy would fail at ImportAsHolding on Production. Fix: added a `preserveAllStages` option to readSettingsFile. validate-deployment-settings now calls it with that flag set so every per-stage entry is inspected. 2. Mixed-case `Stages` key when value is an object Comment said "either casing of stages/Stages key" but code only checked `parsed.stages || parsed.STAGES`. A hand-authored file using `Stages: { ... }` (capital-S + object form โ€” falls through shape 2's array check because the value isn't an array) returned 0 entries. Fix: added `parsed.Stages` to the fallback chain. All three casings (`stages` / `Stages` / `STAGES`) now resolve to the object-shape path when the value is a plain object. 3. Regression tests (+3, total 996/996) - Mixed-case `Stages` object is read correctly. - readSettingsFile dedupe (default) vs preserveAllStages behavior pinned with concrete expectations. - End-to-end: validateSettings catches a Production-only invalid value even when Staging is valid (the dedupe-loss scenario). Verified against real Citizens portal site โ€” still correctly catches the @KeyVault(...) placeholder on c311_api_secret. Lint: 0 findings. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/validate-deployment-settings.js | 15 ++- .../scripts/lib/verify-env-var-values.js | 35 ++++-- .../tests/verify-env-var-values.test.js | 101 ++++++++++++++++++ 3 files changed, 138 insertions(+), 13 deletions(-) diff --git a/plugins/power-pages/scripts/lib/validate-deployment-settings.js b/plugins/power-pages/scripts/lib/validate-deployment-settings.js index 5223d06fd..966cb54d4 100644 --- a/plugins/power-pages/scripts/lib/validate-deployment-settings.js +++ b/plugins/power-pages/scripts/lib/validate-deployment-settings.js @@ -327,12 +327,19 @@ function parseArgs(argv) { } // Entry reader: delegates to verify-env-var-values.js#readSettingsFile, -// which handles all three deployment-settings.json shapes and now returns -// `{ schemaName, value, stageLabel }` on each entry (so we don't need a -// duplicate parser to preserve stage attribution). +// 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 = readSettingsFile(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))); 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 00d0539dc..d597ff684 100644 --- a/plugins/power-pages/scripts/lib/verify-env-var-values.js +++ b/plugins/power-pages/scripts/lib/verify-env-var-values.js @@ -96,9 +96,22 @@ function parseArgs(argv) { // 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.) -// Returns `[{ schemaName, value, stageLabel }]` filtered to stageLabel when provided. -// stageLabel is included on each entry for shape 2/3; shape 1 sets stageLabel: null. -function readSettingsFile(filePath, stageLabel) { +// +// 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'); @@ -129,7 +142,7 @@ function readSettingsFile(filePath, stageLabel) { } } } - return dedupeBySchemaName(all); + return preserveAllStages ? all : dedupeBySchemaName(all); } const stage = parsed.Stages.find( (s) => (s.Name || '').toLowerCase() === lowerLabel @@ -142,11 +155,15 @@ function readSettingsFile(filePath, stageLabel) { })); } - // Shape 3: per-stage keyed object (`stages: { "": {...} }`). + // 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 either casing of - // the `stages`/`Stages` key + match stage labels case-insensitively. - const stagesObj = parsed.stages || parsed.STAGES; + // 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' && @@ -165,7 +182,7 @@ function readSettingsFile(filePath, stageLabel) { } } } - return dedupeBySchemaName(all); + return preserveAllStages ? all : dedupeBySchemaName(all); } // Case-insensitive key match const matchKey = Object.keys(stagesObj).find( 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 9dbfb1786..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 @@ -153,6 +153,107 @@ 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 From 574dedbaecf8dd378306eb85f2ebe2807aae3b0c Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Tue, 26 May 2026 22:52:26 +0530 Subject: [PATCH 09/10] Restore SKILL_SEVERITY const and apply uniformly across all severity sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User review feedback: "why are we reverting from a const to a string here?" on lint-skills-alm.js line 561. My G7 cleanup in `b42ddc6` went the wrong direction. The original review finding was that `SKILL_SEVERITY` was half-applied (2 of 9 severity sites used it, 7 hard-coded `'error'`). I responded by REMOVING the constant entirely and inlining `'error'` everywhere โ€” eliminating the inconsistency but also the indirection. The right fix is the other direction: keep the constant and use it everywhere. That preserves single source of truth for "what severity does this plugin's lint emit" (one line to change for a future rule class that needs a different severity) without the half-applied awkwardness. All 10 finding-push sites in collectFindings now use SKILL_SEVERITY. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../power-pages/scripts/lint-skills-alm.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/plugins/power-pages/scripts/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index bf8ce4d6e..9b3d05036 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -438,6 +438,12 @@ function findPromptLines(content) { return out; } +// 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 @@ -551,7 +557,7 @@ function collectFindings({ pluginRoot }) { 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`. ' + @@ -568,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 ` + @@ -590,7 +596,7 @@ function collectFindings({ pluginRoot }) { for (const u of unmatched) { findings.push({ rule: 'GATE-must-have-marker', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Phase section "${u.heading}" contains an \`AskUserQuestion\` prompt (line ${u.lineNum}) ` + @@ -608,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) ` + @@ -666,7 +672,7 @@ function collectFindings({ pluginRoot }) { if (!hasSentinel) { findings.push({ rule: 'GATE-prose-block-required', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Gate \`${gm.gateId}\` (line ${gm.lineNum}) is missing the ๐Ÿšฆ ` + @@ -688,7 +694,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 ` + @@ -705,7 +711,7 @@ function collectFindings({ pluginRoot }) { if (!catalogGateIds.has(gm.gateId)) { findings.push({ rule: 'GATE-must-be-in-catalog', - severity: 'error', + severity: SKILL_SEVERITY, file, message: `Gate \`${gm.gateId}\` is declared in SKILL.md but is not in the catalog ` + @@ -731,7 +737,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}. ` + @@ -753,7 +759,7 @@ function collectFindings({ pluginRoot }) { if (markerIdSet.has(gateId)) continue; findings.push({ rule: 'CATALOG-row-must-have-marker', - severity: 'error', + severity: SKILL_SEVERITY, file: catalogFile, message: `Catalog row \`${gateId}\` (kind: gate) has no matching ` + @@ -774,7 +780,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) ' + From 8ddcc36913e4b82bd951ce3e25103099040592c6 Mon Sep 17 00:00:00 2001 From: Nidhi Tyagi Date: Wed, 27 May 2026 10:47:57 +0530 Subject: [PATCH 10/10] =?UTF-8?q?Fix=20two=20Copilot=20review=20findings:?= =?UTF-8?q?=20fence-tracking=20asymmetry=20+=20=C2=A76.24a/=C2=A711=20doc?= =?UTF-8?q?=20conflict?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Fence-boundary classification was asymmetric (lint-skills-alm.js:660) The GATE-prose-block-required fence-tracking loop toggled `inside` BEFORE recording `inFence[i]`, which made: - Opening ``` line โ†’ marked INSIDE the fence - Closing ``` line โ†’ marked OUTSIDE the fence Inconsistent and counter to CommonMark intent. Both delimiter lines are markdown syntax, not code content; the rule should treat them the same. Fixed by marking both delimiter lines as OUTSIDE and only the strictly-between content lines as INSIDE. Regression test added: a ๐Ÿšฆ placed on the opening ```๐Ÿšฆ fence line itself now satisfies the rule (delimiter lines are not code content). 2. ยง6.24a says "closed" but ยง11 still said "deferred" (approval-gates.md) ยง6.24a was updated in commit 8d02547 to reflect that v3 surfaced the runtime-loop prompts as real call sites (manage-firewall:3.action-choice, 3.execute-consent, scan-site:3.action-choice). But the ยง11 Open Questions bullet still described it as a "known gap" / "deferred from v3" โ€” a stale draft from the earlier iteration. Reworded ยง11 to acknowledge the v3 fix (forward-pointer to ยง6.24a) and retain only the historical context. Plugin tests: 997/997 (was 996; +1 fence-boundary regression test). Lint: 0 findings. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../power-pages/references/approval-gates.md | 2 +- .../power-pages/scripts/lint-skills-alm.js | 24 ++++++++++---- .../scripts/tests/lint-skills-alm.test.js | 31 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/plugins/power-pages/references/approval-gates.md b/plugins/power-pages/references/approval-gates.md index b0e38c254..c5955743e 100644 --- a/plugins/power-pages/references/approval-gates.md +++ b/plugins/power-pages/references/approval-gates.md @@ -716,5 +716,5 @@ These are honest unresolved questions โ€” not necessary to answer before v2 land - **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 hole (manage-firewall, scan-site).** ยง6.24a documents this: the destructive WAF mutations in `manage-firewall` ยง3 Plan-validate-execute and the scan-trigger choices in `scan-site` ยง3 Default approach happen inside a "recommend then ask" loop with no statically-locatable `AskUserQuestion`:` call site. The not-a-gate markers in those skills satisfy lint without anchoring the actual destructive prompts. Tracked as a known gap โ€” restructuring the prose to surface real call sites is the path forward but was deferred from v3 to keep the PR scoped. +- **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/lint-skills-alm.js b/plugins/power-pages/scripts/lint-skills-alm.js index 9b3d05036..fca79873b 100644 --- a/plugins/power-pages/scripts/lint-skills-alm.js +++ b/plugins/power-pages/scripts/lint-skills-alm.js @@ -649,15 +649,27 @@ function collectFindings({ pluginRoot }) { 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++) { - // 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 โ€” we approximate with /^\s{0,3}```/). - if (/^\s{0,3}```/.test(lines[i])) inside = !inside; - inFence[i] = inside; + 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 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 57c86370b..1de627bd4 100644 --- a/plugins/power-pages/scripts/tests/lint-skills-alm.test.js +++ b/plugins/power-pages/scripts/tests/lint-skills-alm.test.js @@ -951,6 +951,37 @@ Ask via \`AskUserQuestion\`: 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).