Fix two readSettingsFile regressions surfaced by Copilot review

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) <noreply@anthropic.com>

Author: tyaginidhi

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)));

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: { "<name>": {...} }`).
+  // Shape 3: per-stage keyed object (e.g. `stages: { "<name>": {...} }`).
   // 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(

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