Skip to content

Add SPA template flow to create-site - #329

Open
Priyanshu Agrawal (priyanshu92) wants to merge 77 commits into
mainfrom
priyanshu92-spa-templates-create-site
Open

Add SPA template flow to create-site#329
Priyanshu Agrawal (priyanshu92) wants to merge 77 commits into
mainfrom
priyanshu92-spa-templates-create-site

Conversation

@priyanshu92

Copy link
Copy Markdown
Collaborator

Why

create-site could only start from a blank SPA scaffold. This adds a template path so users can browse installable Power Pages SPA templates, import one into their environment, seed it, activate it, and open the live site without leaving the skill flow.

What changed

  • Splits discovery so users first choose whether to browse templates or create from scratch.
  • Fetches the template catalog from microsoft/power-pages-samples, resolves release refs to immutable SHAs, and normalizes manifest-relative artifact paths before downloads.
  • Adds a template browser HTML surface with one sidebar tab per template and full-width preview images stacked in the page.
  • Imports selected template solutions unmanaged, detects existing installs, supports update or clone choices, and activates the imported site.
  • Adds pre-import checks for .js blocked attachments and en-US language availability (LCID 1033).
  • Applies template seed data, including Dataverse file-column attachments through __files.
  • Emits non-PII template outcome telemetry and updates Power Pages docs/metadata.

Validation

  • node plugins/power-pages/scripts/lint-skills-alm.js
  • Focused node --test coverage for catalog fetch, template browser rendering, import context, pages diffing, seed data, telemetry, preflights, reinstall policy, and URL opening.
  • Live catalog smoke checks against the released samples manifest, including preview artifact downloads.

The full Power Pages script suite still reports an unrelated validator cluster around deployment markers and project-root fallback behavior. The focused tests covering this change pass.

Split create-site discovery into path-agnostic inputs and add a no-op from-scratch branch seam for future SPA template selection. Document the task-tracking pattern for branching skills so progress remains accurate without permanently skipped tasks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Add SHA-pinned Power Pages template catalog fetching, artifact caching, solution validation, and browser preview rendering for create-site's template branch seam. Wire Phase 1.5 to offer template selection with from-scratch fallback while stopping before org mutation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Wire create-site's selected template branch through import consent, unmanaged ImportSolutionAsync, and inactive site identification via pac pages list diff. Add deterministic helpers for import context resolution, pages-list snapshots, and before/after site detection with node:test coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Invoke activate-site with imported template identity, open the activated URL in the user's default browser, and surface DNS propagation guidance. Add a shared default-browser helper used by template previews and live-site opening.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Add deterministic seed-data fetching and application for selected create-site templates. Seed files are applied best-effort after import and before activation, with duplicate conflicts skipped and summaries surfaced without blocking go-live.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Add template solution inspection and reinstall policy decisions for create-site templates. Document update, clone, unknown-detection, import failure, activation failure, and seed failure branches with approval gates and tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Add a template_outcome telemetry event for create-site's template and scratch branches using the shared telemetry event builder and sanitized eventInfo payload. Wire terminal template outcomes and scratch completion while preserving opt-out and fail-closed behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Update user and contributor docs for create-site's SPA template path, including local vs installable template terminology and approval-gate behavior for the template branch.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Move create-site template_outcome event construction out of shared telemetry and keep the Power Pages-specific schema local to the create-site telemetry helper, while preserving sanitized eventInfo and fail-closed dispatch.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Resolve the latest power-pages-samples GitHub release to its tag, pin that tag to an immutable commit SHA for each create-site run, and keep explicit --ref overrides for tests or rollback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Extend template seed data to support Dataverse file-column attachments via reserved __files metadata, explicit primary keys, chunked 4 MiB file-column uploads, and best-effort attachment failures. Download all seed-data artifacts and add coverage for path safety, duplicate records, LFS pointers, and disallowed extensions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Add a create-site template import preflight that checks target environment blockedattachments for .js, gates environment-level unblocking consent, removes only js on approval, and stops or falls back before import when declined.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Manifest paths are relative to templates/manifest.json, but create-site treated them as repo-root paths. That made release preview images look unpublished even though they were present.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
@priyanshu92
Priyanshu Agrawal (priyanshu92) requested a review from a team as a code owner July 28, 2026 15:37
Copilot AI lite review requested due to automatic review settings July 28, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Power Pages /create-site skill to support a template-driven SPA creation path in addition to the existing “from scratch” scaffold flow. It adds catalog/artifact fetching from microsoft/power-pages-samples, a local HTML browser surface for previewing templates, import/activation preflights, seed-data application (including file-column attachments), and non-PII telemetry for outcome tracking.

Changes:

  • Add a Phase 1.5 branch decision flow to /create-site for browsing/importing curated SPA templates (with preflights, activation, and live URL opening) vs. continuing with the existing from-scratch scaffold path.
  • Introduce Node scripts/libs for template catalog resolution + SHA pinning, artifact caching/downloading, template browser HTML rendering, import context resolution, pages list diffing, seed data application, and template-outcome telemetry.
  • Add focused node:test coverage for the new catalog/render/import/seed/telemetry utilities and update plugin docs/metadata to reflect the new creation path.
Show a summary per file
File Description
plugins/power-pages/skills/create-site/SKILL.md Adds Phase 1.5 routing + full template import/activation/seed flow and scratch telemetry emission instructions.
plugins/power-pages/skills/create-site/assets/template-browser.html Adds static HTML shell used to render the template preview browser surface.
plugins/power-pages/skills/activate-site/SKILL.md Allows /activate-site to accept explicit imported-site identity via $ARGUMENTS and skip local-project discovery.
plugins/power-pages/scripts/tests/template-reinstall-policy.test.js Tests solution zip inspection + reinstall decision matrix and CLI argument handling.
plugins/power-pages/scripts/tests/template-import-context.test.js Tests template import context resolution and pages list capture script behavior.
plugins/power-pages/scripts/tests/template-catalog.test.js Extensive tests for catalog fetching, SHA pinning, caching, artifact downloads, and CLI arg parsing.
plugins/power-pages/scripts/tests/render-template-browser.test.js Tests HTML rendering output for template preview browser, including escaping and tab/preview structure.
plugins/power-pages/scripts/tests/pages-list-diff.test.js Tests PAC pages list parsing + diffing to identify imported sites.
plugins/power-pages/scripts/tests/open-url.test.js Tests default-browser opener selection and open-url CLI validation/failure handling.
plugins/power-pages/scripts/tests/create-site-template-telemetry.test.js Tests non-PII telemetry event shaping, sanitization, and CLI parsing.
plugins/power-pages/scripts/tests/check-available-languages.test.js Tests Dataverse language preflight for en-US (LCID 1033).
plugins/power-pages/scripts/tests/apply-seed-data.test.js Tests seed data ingestion, duplicate handling, file-column attachment upload, and safety validation.
plugins/power-pages/scripts/resolve-template-import-context.js CLI entrypoint to resolve environment URL + auth token for template import.
plugins/power-pages/scripts/render-template-browser.js Renders the template preview HTML output from JSON + optionally opens it locally.
plugins/power-pages/scripts/open-url.js CLI to open a URL in the OS default browser with fail-open JSON output.
plugins/power-pages/scripts/lib/template-reinstall-policy.js Implements solution.xml inspection and import/update/clone decision logic.
plugins/power-pages/scripts/lib/template-import-context.js Resolves env URL + token using shared validation helpers for import operations.
plugins/power-pages/scripts/lib/template-cli-args.js Shared best-effort JSON CLI utilities + shared template repo arg parsing.
plugins/power-pages/scripts/lib/template-catalog.js Implements GitHub ref→SHA pinning, catalog validation, caching, and artifact download helpers.
plugins/power-pages/scripts/lib/render-template.js Enhances HTML escaping and exports escapeHtml for reuse by renderers.
plugins/power-pages/scripts/lib/pages-list-diff.js Parses and diffs pac pages list -v output to locate imported site identity.
plugins/power-pages/scripts/lib/default-browser.js Adds cross-platform default browser opener helper.
plugins/power-pages/scripts/lib/create-site-template-telemetry.js Adds non-PII template outcome telemetry shaping, sanitization, and emission.
plugins/power-pages/scripts/lib/apply-seed-data.js Adds seed-data application logic, including file-column attachment upload via Dataverse actions.
plugins/power-pages/scripts/inspect-template-solution.js CLI wrapper over solution inspection + reinstall decision.
plugins/power-pages/scripts/fetch-template-solution.js CLI wrapper for downloading/validating template solution zips.
plugins/power-pages/scripts/fetch-template-seed-data.js CLI wrapper for discovering/downloading seed-data directories from pinned SHAs.
plugins/power-pages/scripts/fetch-template-catalog.js CLI wrapper to fetch/validate catalog with fail-open behavior for create-site fallback.
plugins/power-pages/scripts/fetch-template-artifact.js CLI wrapper to download an arbitrary artifact and return a file:// URL for previews.
plugins/power-pages/scripts/emit-create-site-template-outcome.js CLI wrapper to emit template outcome telemetry.
plugins/power-pages/scripts/diff-pages-list.js CLI wrapper to diff before/after pages list snapshots and emit JSON result.
plugins/power-pages/scripts/check-available-languages.js CLI + implementation for RetrieveAvailableLanguages preflight.
plugins/power-pages/scripts/capture-pages-list.js CLI to capture pac pages list -v output to a file for later diffing.
plugins/power-pages/scripts/apply-seed-data.js CLI wrapper that runs seed-data application as best-effort JSON output.
plugins/power-pages/references/approval-gates.md Updates the approval-gates catalog to include the new create-site Phase 1.5 gates/not-a-gates.
plugins/power-pages/README.md Updates skill docs to describe template vs. from-scratch creation paths.
plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md Updates task-tracking guidance for branching skills (create common tasks first, append branch tasks later).
plugins/power-pages/AGENTS.md Clarifies distinction between local scaffold templates vs. installable templates from power-pages-samples.
plugins/power-pages/.plugin/plugin.json Bumps plugin version to 2.7.0.
plugins/power-pages/.claude-plugin/plugin.json Mirrors plugin version bump to 2.7.0 for legacy compatibility.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 40/40 changed files
  • Comments generated: 2
  • Review effort level: Low
Comment thread plugins/power-pages/scripts/lib/apply-seed-data.js Outdated
Comment thread plugins/power-pages/scripts/render-template-browser.js
…ates-create-site

# Conflicts:
#	plugins/power-pages/.claude-plugin/plugin.json
#	plugins/power-pages/.plugin/plugin.json
Copilot AI review requested due to automatic review settings July 29, 2026 06:21
Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/skills/create-site/assets/template-import-status.html:307

  • __TEMPLATE_NAME__ is injected as an HTML-escaped string (via escapeHtml(templateName) in render-template-import-status.js), but it's also embedded inside JS string literals used for textContent updates. That causes user-visible headings like Company &quot;Portal&quot; / &#39; instead of real quotes, and it conflates HTML and JS escaping contexts. Prefer injecting the template name as a JS string literal (e.g., __TEMPLATE_NAME_JSON__) and building headings from that value (and setting document.title from JS) while keeping HTML markup free of untrusted raw text.
    plugins/power-pages/scripts/lib/template-reinstall-policy.js:24
  • inspectSolutionZip() shells out to unzip -p ... solution.xml. If unzip isn't available on PATH (common on Windows / some CI images), the error will surface as a generic spawn failure, and the create-site template flow loses the ability to identify solution uniqueName/version for install/update decisions. At minimum, detect the missing-executable case and return an actionable error message so callers can present a clear fallback path.
function readSolutionXml(zipPath, deps = {}) {
  const execFile = deps.execFileSync || execFileSync;
  // `unzip -p <solution.zip> solution.xml` returns the Dataverse solution
  // descriptor, which includes fields like:
  //   <UniqueName>contoso_template</UniqueName>
  //   <Version>1.0.0.0</Version>
  //   <Managed>0</Managed>
  // Tags can have surrounding whitespace/newlines, so extract by tag name
  // rather than relying on line positions.
  return execFile('unzip', ['-p', zipPath, 'solution.xml'], { encoding: 'utf8', maxBuffer: 5 * 1024 * 1024 });
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

plugins/power-pages/scripts/lib/template-reinstall-policy.js:24

  • inspectSolutionZip() depends on the external unzip binary to read solution.xml. This repo already removed the unzip dependency for template ZIP validation (see template-catalog zip parsing), and unzip is frequently missing on Windows/plugin hosts. When missing, create-site will be forced into the "detection failed" reinstall path every time and lose version/managed detection. At minimum, translate ENOENT into a clear actionable error; longer-term consider extracting solution.xml using the same in-process ZIP parsing approach used elsewhere.
function readSolutionXml(zipPath, deps = {}) {
  const execFile = deps.execFileSync || execFileSync;
  // `unzip -p <solution.zip> solution.xml` returns the Dataverse solution
  // descriptor, which includes fields like:
  //   <UniqueName>contoso_template</UniqueName>
  //   <Version>1.0.0.0</Version>
  //   <Managed>0</Managed>
  // Tags can have surrounding whitespace/newlines, so extract by tag name
  // rather than relying on line positions.
  return execFile('unzip', ['-p', zipPath, 'solution.xml'], { encoding: 'utf8', maxBuffer: 5 * 1024 * 1024 });
}
  • Files reviewed: 49/49 changed files
  • Comments generated: 2
  • Review effort level: Lite
Comment thread plugins/power-pages/scripts/render-template-browser.js
Comment thread plugins/power-pages/scripts/render-template-import-status.js
Update create-site so a no-match template search tells the user no matching templates are available and continues down the from-scratch path instead of showing the full catalog.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() sends the 200 response headers before the file stream is successfully opened. If the file disappears or becomes unreadable between isServableFile() and createReadStream(), streamFile() can only res.end() (headers already sent) and the client receives a 200 with an error body. Delay writing the 200 headers until the read stream is opened, and send a 404 if the stream errors before headers are sent.
    plugins/power-pages/scripts/lib/template-reinstall-policy.js:43
  • inspectSolutionZip() relies on an external unzip executable to read solution.xml. On environments where unzip is not installed (common on Windows), the current error will be a low-level ENOENT message. Convert the missing-executable case into an actionable error so the caller can prompt the user to install the dependency or choose a fallback path.
  } catch (err) {
    return { ok: false, error: err.message };
  }
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Read solution.xml directly from ZIP headers for template re-install detection so Windows hosts without unzip can still inspect template packages.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Replace the standalone import progress line with motion on the active phase pill and tighten the status page layout so the Power Pages logo cannot overlap the preview on shorter windows.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (4)

plugins/power-pages/scripts/render-template-browser.js:190

  • This script writes JSON twice: renderTemplate() already prints { status: 'ok', output: ... } to stdout, and then main() prints its own JSON result. That makes the CLI output non-JSON and can break any caller that parses stdout as a single JSON object.
function main() {
  try {
    const result = renderTemplateBrowser(parseArgs(process.argv.slice(2)));
    process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
  } catch (err) {

plugins/power-pages/scripts/render-template-import-status.js:71

  • Like render-template-browser, this script ends up emitting JSON twice on stdout because renderTemplate() logs a JSON status and main() prints another JSON object. Any caller expecting a single JSON document will fail to parse the output.
  renderTemplate({
    templatePath,
    outputPath,
    dataObject: {
      TEMPLATE_NAME: escapeHtml(templateName),

plugins/power-pages/scripts/serve-static-dir.js:84

  • startServer() sends the 200 response headers before streamFile() attempts to open the file. If createReadStream() then throws (file deleted/permission change), streamFile() can't switch the status to 404 because res.headersSent is already true, so clients can receive a 200 with a "Not found" body. Delay sending headers by using res.statusCode/res.setHeader instead of writeHead(200, …) here.
    plugins/power-pages/skills/create-site/assets/template-browser.html:90
  • The template sidebar behaves like a tab UI, but it doesn't set any ARIA state/relationships (aria-selected, aria-controls, hidden/aria-labelledby). This makes the template chooser harder to use with screen readers and keyboard navigation. Consider initializing and updating ARIA attributes in the click handler (you already have stable IDs via data-tab).
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() sends the 200 response headers before streamFile() has successfully opened the file. If createReadStream() throws (or errors before headers are flushed), streamFile() can’t downgrade to a 404 because headersSent is already true, so clients may get a 200 for a missing/unreadable file.
    plugins/power-pages/scripts/render-template-import-status.js:45
  • previewImageUrl() can still throw if the source preview file exists but is unreadable (or the copy fails). That would abort rendering the entire import status page even though preview images are optional. Treat copy failures as best-effort and keep the original URL/path instead of throwing.
  fs.copyFileSync(sourcePath, destPath);
  return `preview-images/${destName}`;
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Match existing template sites when solution updates add framework suffixes to PAC site names.

Add a tenant-alignment preflight so PAC CLI, Azure CLI, and the Dataverse token must point to the same tenant before template import.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

plugins/power-pages/scripts/dataverse-request.js:98

  • This error message always recommends az login --allow-no-subscriptions, but the plugin guidance is to suggest plain az login first and only mention --allow-no-subscriptions as a fallback for accounts without subscriptions. As written, this can send users to the less-common login variant unnecessarily.
  let token = getAuthToken(envUrl);
  if (!token) {
    process.stderr.write('Failed to get Azure CLI token. Run `az login --allow-no-subscriptions` first.\n');
    process.exit(1);
  }

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() writes the 200 headers before the read stream has opened. If the stream fails to open (e.g., file deleted between isServableFile() and createReadStream()), the error path can no longer return a proper 404 because headers are already sent.

Wait for the stream open event before sending the 200 headers, and send a 404 when the stream errors before opening.
plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21

  • The usage string implies --envUrl is always required, but run() accepts a token-only invocation (and the tests cover run(['--token', ...])). Updating the usage text will avoid confusing users when they validate alignment from a bearer token alone.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Pass the browser target through an environment variable instead of as a positional PowerShell argument after -Command. This prevents Windows PowerShell from rejecting status-page URLs as extra command tokens.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Addressed review comments:

- Suppressed the shared renderTemplate status line in render-template-browser.js so the CLI prints one JSON result.

- Suppressed the shared renderTemplate status line in render-template-import-status.js so the CLI prints one JSON result.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Handle the current pac pages list -v table shape where Website Id is followed by Portal Id, Friendly Name, and Is Site Active. Treat Is Site Active = No as inactive so template import can continue to seed data and activation.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/serve-static-dir.js:90

  • startServer() runs indefinitely with no shutdown path. Since main() spawns the child as a background process and nothing appears to terminate it, repeated /create-site runs can leave behind multiple long-lived localhost servers serving temp directories. Consider adding an idle TTL so the child exits after the browser tab is closed / no requests arrive for a while.
    plugins/power-pages/scripts/validate-cli-tenant-alignment.js:20
  • The usage text implies --envUrl is required, but the implementation (and tests) intentionally allow --token-only runs. Update the usage string so callers understand that at least one of --envUrl or --token must be provided.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21

  • The usage text here implies --envUrl is required, but run() actually allows --token-only execution (and the test suite relies on that). This mismatch can confuse callers and make troubleshooting harder. Update the usage string to reflect that either --envUrl or --token is acceptable (at least one required).
    plugins/power-pages/scripts/lib/apply-seed-data.js:403
  • The body of this loop is under-indented starting at const context / try {, which makes the nesting structure hard to read and may fail formatting/lint checks. Please reformat this entire block so everything inside the for (const record ...) loop is consistently indented.
        for (const record of seedEntry.records) {
        const context = { file: path.basename(filePath), entitySetName: seedEntry.entitySetName };
        try {
          const validationError = validateFilesContract({ seedDir, seed: seedEntry, record }, deps);
          const { recordBody, files } = splitReservedFiles(record);
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Use cmd.exe to invoke az.cmd and pac.cmd on Windows so tenant alignment works with the same shims users run in their terminal.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
PAC is installed as pac.exe in the Windows environment, while Azure CLI uses az.cmd. Invoke each through cmd.exe with the correct executable name so tenant alignment matches manual commands.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60
Make non-Windows tenant alignment test cases pass an explicit platform so CI does not route their direct az/pac fakes through the Windows cmd.exe shim path.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (4)

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() sends the 200 response headers before calling streamFile(). If createReadStream fails synchronously or emits an error before any bytes are written, streamFile() will end the response but can no longer switch to a 404, so the client can receive a misleading HTTP 200 with a not-found body.
    plugins/power-pages/scripts/check-available-languages.js:29
  • parseArgs() turns empty --requiredLocaleIds segments (e.g. a trailing comma) into 0 via Number(''), and both the parser and normalizeRequiredLocaleIds() currently accept 0 as a valid LCID. That can cause false “missing language” failures for locale id 0.
    else if (argv[i] === '--requiredLocaleIds') {
      args.requiredLocaleIds = argv[++i]
        .split(',')
        .map((value) => Number(value.trim()))
        .filter((value) => Number.isInteger(value));
    }

plugins/power-pages/scripts/lib/template-catalog.js:370

  • resolveRefToSha() takes the first SHA from git ls-remote. For annotated tags, ls-remote commonly returns two rows (tag object + peeled commit ^{}), and taking the first SHA can yield the tag object SHA, which is not guaranteed to be usable as a treeish for raw content downloads. Prefer the peeled refs/tags/<tag>^{} SHA when present.
function resolveRefToSha({ owner = DEFAULT_OWNER, repo = DEFAULT_REPO, ref = DEFAULT_REF }, deps = {}) {
  const remoteUrl = buildGitRemoteUrl({ owner, repo });
  const output = runGitLsRemote([remoteUrl, ref], deps);
  // `git ls-remote <remote> <ref>` returns tab-delimited rows:
  //   49b0e74b386206c7682019110434f034fca2e129\trefs/heads/main
  // A release tag can return multiple rows for annotated tags; any leading
  // 40-character SHA is enough because raw.githubusercontent.com accepts the
  // object id for immutable content fetches.
  const match = String(output || '').match(/^([0-9a-f]{40})\s+/im);
  if (!match) {
    throw new Error(`git ls-remote did not resolve ${owner}/${repo}@${ref}`);
  }
  return match[1];

plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21

  • The usage string says --envUrl is required, but the runner accepts token-only invocation (and the tests cover that). Update the message to reflect “provide at least one of --envUrl or --token” so users aren’t misled.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/render-template-browser.js:38

  • renderPreviewImages() returns a preview-empty CSS class when there are no images, but template-browser.html does not define any styling for .preview-empty. This makes the empty state inconsistent and leaves a dead/undefined class in the rendered HTML.
function renderPreviewImages(template) {
  const images = Array.isArray(template.previewImages) ? template.previewImages : [];
  if (images.length === 0) {
    return '<div class="preview-empty">No preview image yet</div>';
  }

plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21

  • The usage message implies --envUrl is required, but run() accepts --token-only invocations (and the tests cover this). The current message is misleading for callers who want to validate alignment from a token without specifying an environment URL.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
When solution import succeeds, update the completed phase pill text from Importing solution to Imported solution. Also update later completed phase labels for seed and activation.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (5)

plugins/power-pages/scripts/serve-static-dir.js:118

  • main() also writes args.urlFile without creating parent directories. This can make the parent process fail even though the server could otherwise start successfully.
    plugins/power-pages/scripts/lib/apply-seed-data.js:403
  • The inner for (const record ...) block is missing indentation for context and the entire try/catch body, which makes the control flow hard to read and easy to mis-edit (it currently looks like context is outside the loop). Please re-indent this whole block to match the file’s consistent 2-space style.
      for (const seedEntry of (Array.isArray(seed) ? seed : [seed])) {
        indexSeedRecords(seedEntry, idToEntitySet);
        for (const record of seedEntry.records) {
        const context = { file: path.basename(filePath), entitySetName: seedEntry.entitySetName };
        try {
          const validationError = validateFilesContract({ seedDir, seed: seedEntry, record }, deps);
          const { recordBody, files } = splitReservedFiles(record);

plugins/power-pages/skills/create-site/assets/template-browser.html:57

  • In the mobile layout rules, .nav-btn sets border-bottom to the accent color for every tab, which removes the active/selected affordance (all tabs look selected). Make the default border transparent and apply the accent color only for .nav-btn.active in the media query.
    plugins/power-pages/scripts/serve-static-dir.js:89
  • startServer() writes urlFile without ensuring its parent directory exists. If the caller passes a path in a not-yet-created folder, the child server will throw in the listen callback and exit, leaving the caller with a stale/missing URL file.

This issue also appears on line 115 of the same file.
plugins/power-pages/scripts/validate-cli-tenant-alignment.js:22

  • The usage string implies --envUrl is required, but run() accepts --token alone (and the tests rely on token-only being valid). Update the usage text to match the actual accepted CLI shapes so callers aren’t misled.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
…ates-create-site

# Conflicts:
#	plugins/power-pages/scripts/lib/render-template.js
Satisfy the secure process execution validator by replacing the generic tenant CLI runner with fixed literal execFileSync call sites for pac, az, and cmd.exe.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() sends the 200 response headers before attempting to create/pipe the read stream. If createReadStream throws (or errors before headers are sent), streamFile() cannot change the already-sent 200 to a 404, so the server can incorrectly return HTTP 200 for missing/unreadable files. Delay writeHead(200, ...) until after the stream is successfully opened (and handle open/stream errors locally).
    plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21
  • The usage string says --envUrl <url> [--token <bearer-token>], but run() actually accepts either --envUrl or --token (tests cover token-only). Update the usage text so callers aren't misled into thinking --envUrl is required when a token is provided.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

plugins/power-pages/scripts/serve-static-dir.js:83

  • startServer() sends a 200 response header before calling streamFile(). If createReadStream() throws (or the stream errors before piping), streamFile() will fall back to res.end('Not found') but can no longer change the status code because headers are already sent, so clients can get a 200 with a not-found body. Avoid marking headers as sent until the stream is established by setting statusCode/headers without writeHead() and letting the stream write trigger the header flush.
    plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21
  • The usage text says --envUrl is required, but run() actually allows calling with only --token (and tests cover that). This mismatch makes the error message misleading for valid invocations that omit envUrl (or for future callers that want to validate tenant alignment purely from a token). Update the usage string to reflect the actual contract: at least one of --envUrl or --token is required.
  • Files reviewed: 52/52 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Use the active pac auth list Environment Url when pac env who does not expose a usable Org URL. This avoids blocking template import when PAC has an active profile but pac env who cannot resolve it.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

plugins/power-pages/scripts/validate-cli-tenant-alignment.js:20

  • The usage string implies --envUrl is required, but run() actually accepts either --envUrl or --token (it only errors when both are missing). This mismatch is confusing for callers and contradicts the behavior covered by cli-tenant-alignment.test.js (token-only run succeeds). Update the usage text to reflect the real contract (at least one of envUrl/token is required).
  • Files reviewed: 54/54 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
After semantic template matching, ask whether to show matching templates, browse all templates, or continue from scratch instead of forcing the match/no-match route automatically.

Copilot-Session: a9952624-0a25-4d6e-ae67-537c962a8a60

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

plugins/power-pages/scripts/validate-cli-tenant-alignment.js:21

  • The usage string implies --envUrl is required, but the CLI (and its tests) allow running with only --token (no envUrl). This will confuse users when they hit this error.

Update the message to reflect the actual contract: require at least one of --envUrl or --token (both optional individually).

  • Files reviewed: 54/54 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants