Fix marketplace install parse failure by replacing legacy symlink manifests with JSON mirrors - #203
Merged
Merged
Conversation
Copilot
AI
changed the title
[WIP] Fix JSON parse error when installing power platform skills marketplace
Fix marketplace install parse failure by replacing legacy symlink manifests with JSON mirrors
Jun 22, 2026
T-Nid (tyaginidhi)
marked this pull request as ready for review
June 22, 2026 09:48
T-Nid (tyaginidhi)
approved these changes
Jun 22, 2026
T-Nid (tyaginidhi)
enabled auto-merge (squash)
June 22, 2026 09:49
Priyanshu Agrawal (priyanshu92)
approved these changes
Jun 22, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a cross-platform marketplace install failure by moving legacy .claude-plugin manifests from filesystem-dependent symlinks to committed JSON mirrors, and updating repo validation/docs to ensure legacy compatibility stays in sync with the Open Plugins manifests.
Changes:
- Updated the legacy compatibility validator to compare legacy manifests as JSON mirrors of the Open Plugins manifests.
- Updated repository documentation to describe legacy manifests as committed mirrors (not symlinks) and to keep them in sync.
- Refined the validator’s guardrails around legacy marketplace/plugin metadata consistency.
Reviewed changes
Copilot reviewed 9 out of 15 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/validate-legacy-compatibility.js | Switches legacy manifest validation from symlink-target checks to JSON mirror equivalence checks. |
| README.md | Updates documentation to describe .claude-plugin manifests as compatibility mirrors. |
| AGENTS.md | Updates contributor guidance to describe/require mirror-based legacy manifest compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+36
to
38
| function assertJsonMirror(legacyPath, sourcePath) { | ||
| assert.deepEqual(readJson(legacyPath), readJson(sourcePath)); | ||
| } |
T-Nid (tyaginidhi)
added a commit
that referenced
this pull request
Jun 22, 2026
main #203 replaced the legacy .claude-plugin symlinks with committed JSON mirror files and changed validate-legacy-compatibility.js to require each mirror to JSON-deep-equal its source. This branch bumped the power-pages sources (marketplace.json + .plugin/plugin.json) to 2.5.0 before #203 landed, so merging main brought the mirrors back as regular files frozen at 2.4.0 — failing validate-repository-metadata. Update the two power-pages mirrors (.claude-plugin/marketplace.json entry + plugins/power-pages/.claude-plugin/plugin.json) to 2.5.0 so they match their sources. Only the version field drifted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
T-Nid (tyaginidhi)
added a commit
that referenced
this pull request
Jun 22, 2026
…rging main) The merge resolution left .claude-plugin/marketplace.json and plugins/power-pages/.claude-plugin/plugin.json at 2.5.0 while the sources (marketplace.json + .plugin/plugin.json) are bumped to 2.6.0 for this PR. validate-legacy-compatibility requires each mirror to JSON-deep-equal its source (main #203), so bump both mirrors to 2.6.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
T-Nid (tyaginidhi)
added a commit
that referenced
this pull request
Jun 22, 2026
…DM-site reliability fixes + siteType→declarative rename (#202) * plan-alm: convert to strict plan-only planner (execution skills self-maintain the plan) plan-alm previously executed the whole deployment from a single "approve and execute" answer, which under autopilot ran an entire unattended deployment with a silently git-captured approver. Since a skill cannot reliably detect it is running unattended, the only robust fix is structural: make NOT executing the default. - plan-alm is now a 4-phase planner (Detect → Gather → Generate → Approve & save). Phases 5–8 (the execute orchestration) are removed; Phase 4 offers Save-approved / Save-draft / Change — there is no "execute" path. Adds PLAN_MODE, PLAN_QUALITY (degraded on auth/discovery failure), decisionsLog, an always-interactive approver capture, and a completeness gate. - Execution skills are self-sufficient: each ALM skill's Phase 0 recommends creating the plan via plan-alm if missing, and its final phase refreshes the plan + prints the next recommended step (refresh-alm-plan-data.js now emits `nextStep`). No auto-chaining — sequencing is user-driven. - Reverts an autopilot-defaults policy + an uncatalogued q2-manual-confirm gate that a prior session added (a skill can't detect autopilot; it also broke the gate lint); keeps the "(Recommended)" Q2 labels + inline recommendation. - Gate catalog, render comment, README, and AGENTS.md updated to match. PR 1 of 4 (stacked): plan-only → EDM → table-discovery → refresh-enforcement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address Copilot review on #191 - refresh-alm-plan-data.js reconcile(): per-phase refresh failures are no longer swallowed by an empty catch. Each failure is captured into result.failed = [{ phase, error }] and written to stderr, so a marker-schema break that makes a refresh throw is diagnosable instead of silently skipped. `reconciled` now lists only the phases that actually healed; `failed` is present (array) on every return path. Contract-guard test added. - setup-solution SKILL.md: the next-step guidance documented `nextStep: { name, skill }` but `skill` can be null for an internal step (e.g. Finalize) — the prose would tell the user to "run null". Contract corrected to `skill: string | null` and the guidance now branches: print the command only when skill is non-null, else name the step alone. - plan-alm SKILL.md: the finalize commit step always showed the Approved message with a vague "(use the (draft) suffix for option 2)" aside. Now shows two explicit commit commands — Approved vs Draft — so the Draft path can't be followed incorrectly. 1183 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Self-review fixes: PP-path activate step, manual steps[], EDM error rule From a full review pass of the plan-only conversion: - refreshDeployPipeline now completes the "Activate site in {stage}" step when the deploy marker evidences activation (activationStatus set, deploy not failed). The PP deploy flow activates the site internally, so leaving the step pending made computeNextStep redundantly nudge the user to run /activate-site for work the deploy already did. Testing stays a separate step (test-site). When the marker has no activationStatus, the Activate step is left pending. - Next-steps guidance corrected: the PP path does activation inside the deploy flow but NOT testing — testing is the separate /power-pages:test-site step the plan already lists. Guidance now ends "... → /power-pages:test-site". - steps[] template: documented the MANUAL-path shape (Setup solution → Export solution → per-target Import/Activate/Test) alongside the PP-path example, so a manual-strategy plan emits step names that step-sync + computeNextStep match — previously only the PP shape was shown. - Error Handling: the "No powerpages.config.json: stop" rule contradicted Phase 1, which resolves data-model/EDM sites from .powerpages-site/website.yml (no config.json). Now stops only when BOTH markers are absent. 1185 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address Copilot second-pass review on #191 - refreshDeployPipeline: gate the Activate-step auto-completion on the explicit "Activated" outcome, not mere truthiness. deploy-pipeline writes activationStatus: "Pending" when the user DEFERS activation — that's truthy but means /power-pages:activate-site is still required, so the previous check would have wrongly completed the Activate step and dropped it from nextStep. Now any non-"Activated" value (Pending, null, a failure note) leaves the step pending. Regression test extended to cover undefined/null/"Pending"/"Failed". - nextStep skill:null guidance: the same "run {nextStep.skill}" guidance lived in 7 other execution skills (activate-site, configure-env-variables, deploy-pipeline, export-solution, import-solution, setup-pipeline, test-site) — the prior fix only updated setup-solution. All 7 now document `skill: string | null` and branch so an internal step (e.g. Finalize) names the step without ever printing `run null`. 1185 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix step-sync stage matching: marker "Deploy to {label}" vs plan step label priyanshu92 review on #191: a finished PP deploy never advanced its plan step. setup-pipeline names pipeline stages "Deploy to {targetLabel}", and deploy-pipeline writes that verbatim as last-deploy.json's stageName ("Deploy to Staging"). The plan step is "Deploy via pipeline to Staging", so setStepStatus's substring match of "deploy to staging" against the step name failed and the step (and the Activate step my earlier change keys off the same stage) stayed pending forever — defeating the self-maintaining-plan goal. Every existing fixture used the bare "Staging", so the suite gave false confidence. setStepStatus now strips a leading "Deploy to " from the stage filter to recover the bare label before matching; callers that already pass the label (e.g. test-site --stageName "Staging") are unaffected (no-op strip). Added regression tests using the REAL "Deploy to Staging" marker shape (step + activate flip; Production untouched) plus a direct setStepStatus normalization test. Also removed a duplicate inline comment on the 'ensure-pipelines-host' PHASES entry (per review — the explanation already lives at the refreshEnsurePipelinesHost def). 1187 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address Copilot third-pass: stable reconcile() nextStep contract reconcile() documents a `nextStep` field but three return paths omitted it, forcing callers to special-case a missing property: - .alm-deferred early return -> now nextStep: null (nothing to guide toward) - no-plan early return -> now nextStep: null (no plan to compute from) - nothing-pending return -> the plan is current with all markers but may still have unfinished checklist steps, so load the plan and return the computed nextStep instead of dropping it. The plan is now parsed once before the pending-size check and reused by both the no-op and heal paths. Contract-guard test extended to assert failed[] AND nextStep are present on every return path (null for deferred/no-plan; the next unfinished step for nothing-pending). 1187 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Harden stage matching: shared normalizeStageLabel for keys too Follow-up to the step-sync fix. Factored the "strip leading 'Deploy to '" logic into a shared normalizeStageLabel() helper and routed BOTH setStepStatus AND the per-stage object-key paths (validationRuns / manualImports / activations) through it. Previously only setStepStatus stripped the prefix; the key paths used the resolved stage verbatim, so if a marker ever carried "Deploy to {label}" the renderer (which keys by the bare label) would silently miss the run. Markers there currently emit the bare label so this is defensive — it closes the whole class of mismatch in one place rather than one code path. Added a test that a "Deploy to Staging" stage keys validationRuns["Staging"]. 1188 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Address Copilot fourth-pass on #191 - refresh-alm-plan-data.js header comment: document the nextStep stdout shape as `{ name, skill: string | null } | null` (skill is null for internal/unmapped steps) — the SKILL.md guidance + the JSDoc already branch on this; the file header was the last spot still showing the old non-nullable shape. - test fixtures: align STRATEGY to the canonical schema value `pp-pipelines` (SKILL.md planData + render-alm-plan.js both use it; `pipeline` was non-schema). All 5 fixtures updated. refresh-alm-plan-data.js doesn't branch on STRATEGY, so this is correctness/representativeness only — no behavior change. - plan-alm next-steps guidance: the PP-path line implied deploy always activates; clarified that the deploy flow activates the site but if you DEFER activation (activationStatus "Pending"), run /power-pages:activate-site. 1188 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * EDM (enhanced/standard data-model) Power Pages site support in ALM discovery Enhanced data-model sites are downloaded with `pac pages download` (not download-code-site): they have NO `powerpages.config.json` and no SPA build output — just `.powerpages-site/` with a config tree + `website.yml`. The shared discovery helpers hard-required `powerpages.config.json`, so every ALM skill broke on EDM sites. - `findProjectRoot` (validation-helpers.js): treats a `.powerpages-site/` directory as a project-root marker, not just `powerpages.config.json`. - `detect-project-context.js`: falls back to `.powerpages-site/website.yml` (`id`→websiteRecordId, `name`→siteName) when no `powerpages.config.json`; returns a new `siteType` ("code" | "data-model"); exits 1 only when neither marker exists. - `check-activation-status.js`: same fallback (verified live against an EDM site → resolves identity + activation status instead of erroring). Backward-compatible: `powerpages.config.json` stays the primary signal for code sites. PR 2 of 4 (stacked, on plan-alm-plan-only). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Site-referenced table discovery + dependency-aware solution splitting Replaces publisher-prefix table discovery (which over-counts catastrophically with a shared/default publisher — a 6-table site matched 22 unrelated tables — and misses real tables from a different prefix) with site-referenced scoping: the custom tables the site's `.powerpages-site/table-permissions/` (+ datamodel manifest) actually reference, intersected with the env's custom-unmanaged tables. SME-confirmed: table permissions are the complete signal. Replaces the old one-solution-per-table-name-stem split heuristic (which produced ~one solution per table — e.g. a 21-solution split) with a dependency-aware, capacity-bounded packer: union-find connected-component clusters over table relationships (lookups + N:N), then first-fit-decreasing bin-packing of whole clusters into the fewest solutions under maxTableCount/maxSchemaAttrs, capped at maxSchemaSplitSolutions (8). The split trigger + thresholds are unchanged — only the packing. New shared libs: - resolve-site-tables.js — site-referenced table scoping (single source of truth) - query-metadata.js — consolidated custom-unmanaged-table query - query-table-relationships.js — relationship edges (lib; audit-permissions CLI is now a thin wrapper) - validation-helpers.js — odataGet/odataGetAll shared paginator estimate-solution-size.js now emits tableCountScope + tableRelationships[]; compute-split-plan.js consumes the edges. setup-solution Phase 5.2.D uses the shared discovery helper. 1209 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix FFD bin-packing under-allocation that overflowed the schema-attr cap deriveDomainsByCapacity seeded the packer's bin count from a LOWER bound (max(ceil(tables/maxTableCount), ceil(attrs/maxSchemaAttrs))), so when independent (no-edge) clusters fragment, FFD ran out of bins and dropped the non-fitting cluster into the least-loaded bucket — overflowing it past maxSchemaAttrs with no warning (the oversized-cluster guard only checks per- cluster table COUNT, not attrs). Verified repro: 4 independent 8000-attr tables, maxSchemaAttrs 15000 -> seed n=3 -> one bucket holds 16000 attrs. Seed the packer with the maximum permitted bins instead (one per cluster, capped at maxSchemaSplitSolutions). FFD still consolidates — clusters that fit together share a bin and empty bins are dropped, so the solution count stays minimal — but a cluster that fits nowhere opens a NEW bin rather than overflowing. The existing 16000-attr/2-solution test is unchanged (FFD still consolidates); added a regression test for the 4-independent-table overflow case. 1211 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Wire --projectRoot into the remaining discover-site-components consumers The site-referenced table discovery (this PR) made discover-site-components return customTables (and thus missing.customTables) ONLY when --projectRoot (or --datamodelManifest) is passed — without a local signal it returns [] rather than the old publisher-prefix dump. setup-solution Step D was updated to pass --projectRoot, but three other consumers that read missing.customTables were missed, so their "custom tables missing from the solution" completeness check silently reported 0 for every site: - export-solution Phase 2.5 (pre-export completeness) - plan-alm Phase 1 (pre-plan completeness) - deploy-pipeline Phase 3.5 (pre-sync completeness) Added --projectRoot "." to all three so the check is restored with the correct site-scoped count. (setup-solution Phase 5.4b/5.4c calls consume missing.envVars / missing.powerpagecomponents / siteLanguages, not customTables, so they're correct as-is with --publisherPrefix/--solutionId.) 1211 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Enforce ALM plan refresh via PostToolUse reconcile backstop (auto-heal) The refresh-alm-plan-data.js calls in each ALM SKILL.md are advisory markdown — silently dropped on session fragmentation, manual execution, or oversight. Three observed gaps where the rendered plan never reflected real run state: ensure-pipelines-host (no refresh call at all), setup-pipeline (refresh lives in Phase 7; phases 5-6 run manually after a resume skip it), activate-site (Phase 5.2b refresh missed). Fix is auto-heal, not fail-validation (per feedback_skill_validation_hooks: never hard-block). After ANY ALM plan skill completes, the centralized PostToolUse hook spawnSyncs `refresh-alm-plan-data.js --reconcile --render`, which ingests any marker (docs/alm/last-*.json) newer than the plan. Because it fires on any ALM skill (not just the marker's writer), a skip in skill A is healed when the next ALM skill B completes. Best-effort and non-blocking: never changes the hook's exit code, honors .alm-deferred, idempotent. - powerpages-hook-utils.js: ALM_PLAN_SKILLS set + isAlmPlanSkill(value) - run-skill-posttool-validation.js: reconcile backstop after the validator - ensure-pipelines-host SKILL.md: explicit Phase 6 self-refresh (direct gap-1 fix; hook reconcile remains the backstop) The reconcile mode + ensure-pipelines-host phase in refresh-alm-plan-data.js ship in the plan-only PR earlier in this stack. 1212 tests pass, alm-lint 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Surface reconcile failures in the hook + document failed[] contract Follows the Copilot review fix in #191 (reconcile no longer swallows per-phase errors): the PostToolUse hook now reads result.failed alongside reconciled and prints a one-line, non-blocking notice naming the phases that could not heal, so a marker-schema break is diagnosable at the hook level (details still go to the lib's stderr). AGENTS.md reconcile bullet updated to document the failed[] field. 1213 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix stale "plan-alm Phases 6/7/8" reference in AGENTS.md Regression pass finding: the refresh-alm-plan-data.js doc entry still said it was "Used by plan-alm Phases 6 / 7 / 8", but those phases were deleted when plan-alm was converted to a plan-only 4-phase planner. The helper is now driven by the execution skills' final-phase refresh + the PostToolUse --reconcile backstop, not by plan-alm. Corrected the clause (CLAUDE.md is a symlink to AGENTS.md, so both are fixed in one edit). 1218 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Activate the PLAN_STATUS lifecycle: Approved -> In Execution -> Completed The plan had an elaborate heartbeat/active-chain lifecycle (check-alm-plan.js: Draft|Approved|In Execution|Completed + 60-min LAST_INVOCATION_AT heartbeat, stale-heartbeat reclassification) that was entirely DEAD: nothing ever set "In Execution", and refreshFinalize ("Completed") was never called. So every plan was stuck at "Approved" and the heartbeat machinery never engaged. Gap 1 — Approved -> In Execution (check-alm-plan.js): plan-alm is plan-only and leaves the plan "Approved"; the FIRST execution skill's Phase 0 call now promotes it to "In Execution" and writes the first heartbeat (one atomic write). Gated on writeHeartbeat, so read-only callers don't mutate the plan — and plan-alm's own deferral check now passes --no-heartbeat (a planner re-run isn't execution and must not promote). Gap 5 — In Execution -> Completed (refresh-alm-plan-data.js): a completion evaluator runs after every phase's step-sync (refresh + reconcile) and flips PLAN_STATUS to "Completed" + stamps COMPLETED_AT once every non-skip step is completed and none failed. The last execution skill terminates the plan automatically — no skill needs to call --phase finalize. A failed step blocks completion so a failed deploy can't look "done". Gaps 2/3/4 (ensure-pipelines-host / setup-pipeline / activate-site refresh calls) were already in place + covered by the reconcile backstop — no change needed. +8 tests (3 promotion in check-alm-plan, 5 completion in refresh); 1227 pass, alm-lint 0 findings. AGENTS.md documents both transitions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Surface COMPLETED_AT in the ALM plan footer When the lifecycle reaches "Completed", the plan footer now shows a "Completed: <timestamp>" line beneath the approver/approval-date, rendered from COMPLETED_AT. Conditional — the `__COMPLETED_LINE__` placeholder maps to an empty string until the plan completes, so there's no orphan token and the line only appears once execution is done. (The template already styled the .completed and .in-execution status badges; this adds the timestamp detail.) +2 renderer tests. 1229 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Site-referenced table discovery + dependency-aware solution splitting Replaces publisher-prefix table discovery (which over-counts catastrophically with a shared/default publisher — a 6-table site matched 22 unrelated tables — and misses real tables from a different prefix) with site-referenced scoping: the custom tables the site's `.powerpages-site/table-permissions/` (+ datamodel manifest) actually reference, intersected with the env's custom-unmanaged tables. SME-confirmed: table permissions are the complete signal. Replaces the old one-solution-per-table-name-stem split heuristic (which produced ~one solution per table — e.g. a 21-solution split) with a dependency-aware, capacity-bounded packer: union-find connected-component clusters over table relationships (lookups + N:N), then first-fit-decreasing bin-packing of whole clusters into the fewest solutions under maxTableCount/maxSchemaAttrs, capped at maxSchemaSplitSolutions (8). The split trigger + thresholds are unchanged — only the packing. New shared libs: - resolve-site-tables.js — site-referenced table scoping (single source of truth) - query-metadata.js — consolidated custom-unmanaged-table query - query-table-relationships.js — relationship edges (lib; audit-permissions CLI is now a thin wrapper) - validation-helpers.js — odataGet/odataGetAll shared paginator estimate-solution-size.js now emits tableCountScope + tableRelationships[]; compute-split-plan.js consumes the edges. setup-solution Phase 5.2.D uses the shared discovery helper. 1209 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix FFD bin-packing under-allocation that overflowed the schema-attr cap deriveDomainsByCapacity seeded the packer's bin count from a LOWER bound (max(ceil(tables/maxTableCount), ceil(attrs/maxSchemaAttrs))), so when independent (no-edge) clusters fragment, FFD ran out of bins and dropped the non-fitting cluster into the least-loaded bucket — overflowing it past maxSchemaAttrs with no warning (the oversized-cluster guard only checks per- cluster table COUNT, not attrs). Verified repro: 4 independent 8000-attr tables, maxSchemaAttrs 15000 -> seed n=3 -> one bucket holds 16000 attrs. Seed the packer with the maximum permitted bins instead (one per cluster, capped at maxSchemaSplitSolutions). FFD still consolidates — clusters that fit together share a bin and empty bins are dropped, so the solution count stays minimal — but a cluster that fits nowhere opens a NEW bin rather than overflowing. The existing 16000-attr/2-solution test is unchanged (FFD still consolidates); added a regression test for the 4-independent-table overflow case. 1211 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Wire --projectRoot into the remaining discover-site-components consumers The site-referenced table discovery (this PR) made discover-site-components return customTables (and thus missing.customTables) ONLY when --projectRoot (or --datamodelManifest) is passed — without a local signal it returns [] rather than the old publisher-prefix dump. setup-solution Step D was updated to pass --projectRoot, but three other consumers that read missing.customTables were missed, so their "custom tables missing from the solution" completeness check silently reported 0 for every site: - export-solution Phase 2.5 (pre-export completeness) - plan-alm Phase 1 (pre-plan completeness) - deploy-pipeline Phase 3.5 (pre-sync completeness) Added --projectRoot "." to all three so the check is restored with the correct site-scoped count. (setup-solution Phase 5.4b/5.4c calls consume missing.envVars / missing.powerpagecomponents / siteLanguages, not customTables, so they're correct as-is with --publisherPrefix/--solutionId.) 1211 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address #193 review: attr-cap overflow warning + componentCount proxy + 4 more Substantive (both reviews flagged): - **maxSchemaAttrs overflow now warned at the ceiling boundary** (compute-split-plan.js). Commit 2 fixed the under-ceiling case (seed n=min(clusters,ceiling)); at the maxSchemaSplitSolutions ceiling (>8 independent attr-heavy clusters) the FFD least-loaded fallback can still co-locate clusters and bust maxSchemaAttrs. The oversized guard only checked table COUNT — added a companion attr-cap warning (summed attributeCount > maxSchemaAttrs). Regression test with 9×14000-attr clusters. - **Table-domain componentCount is now a schema-component proxy** (sum(attributeCount) + 1/table), not the table count. Counting 1/table severely undercounts solution components and could let an over-cap Table solution slip past validateSplits (and distorted the Site solution's subtracted count). Test asserts 500+300+2 = 802. Copilot inline: - **odataGetAll FAILS CLOSED**: throws if it hits maxPages with @odata.nextLink still present, instead of silently returning a truncated set (wrong ALM counts). Test added. - **resolve-site-tables: sources.tablePermissions counts permission FILES**, not parsed records — a malformed file no longer makes a real site look manifest-only/unavailable (which drives tableCountScope). Test with a malformed file. - **Renamed local odataGet → odataGetPath** in estimate-solution-size.js to avoid the name collision with validation-helpers' shared odataGet (different arg order). Minor: - **discoverTableRelationships now uses bounded concurrency (5)** instead of fully serial (~2 calls/table; a 34-table site was 68 sequential round-trips). Edge assembly stays sequential for deterministic dedup. - AGENTS.md: corrected the packing claim (both cap-exceeding cases now warn, not "always under the caps"). Deferred (tracked as follow-up): renderer doesn't surface tableCountScope:'unavailable' distinctly — edge-case (plan-alm always passes --projectRoot) + touches the merged #191 renderer; bundling with the renderer/terminology follow-up. 1228 tests pass, alm-lint 0 findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Resolve merge conflicts with origin/users/nityagi/table-discovery-fix (clean) * Fix incomplete Open Plugins resolution in the merge (#194) The "Merge main into branch" resolution (525405a) left three problems that broke validate-repository-metadata + the env-var convention: - Restored the `.claude-plugin/plugin.json` legacy symlink (the merge deleted it → validator "missing legacy plugin manifest"). - marketplace.json power-pages version 2.4.0 -> 2.5.0 (it was left out of sync with .plugin/plugin.json's 2.5.0 → validator version-mismatch). - ensure-pipelines-host SKILL.md:906 (this PR's Phase 6 refresh call) ${CLAUDE_PLUGIN_ROOT} -> ${PLUGIN_ROOT} (post Open Plugins migration convention). validate-legacy-compatibility: metadata in sync. 1238 tests pass, alm-lint 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address #194 review: DRY plan path + hook smoke test + completion-edge tests Three minor findings from the /review 194 pass: 1. Hardcoded plan path — `docs/.alm-plan-data.json` / `docs/alm-plan.html` were inlined at four call-sites (the PostToolUse hook, check-alm-plan.js, and refresh-alm-plan-data.js reconcile + refresh). Centralize them in alm-paths.js as `planDataPath()` / `planHtmlPath()` (these artifacts live at the docs/ ROOT, not docs/alm/) and route all four callers through the helper so the path can't drift. + tests guarding the docs-root invariant. 2. Coverage gap — no automated test for the hook's reconcile spawn. Add run-skill-posttool-validation.test.js: the backstop heals a skipped refresh after an ALM skill, is skipped for non-ALM skills, and is exit-code-neutral (a blocking validator's status is unchanged whether or not the plan exists). 3. Completion edge — pin evaluatePlanCompletion's status gate: the defensive Approved → Completed fallback completes, a Draft plan never auto-completes, and a failed step blocks completion so a failed deploy can't look "done". Full suite: 1246 pass. legacy-compat + alm-lint: clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address #194 Copilot review comments (3) 1. hook: forward reconcile failure detail to stderr. The empty JSON.parse catch swallowed spawn errors / timeouts / non-zero exits, and rec.stderr was never forwarded — so the "See stderr for details" pointer was empty. Now track spawnFailed (rec.error / non-zero status / signal) and a parsed flag, report on STDERR only on actual failure (clean runs stay quiet — the hook fires on every Skill use), and forward the child's stderr verbatim where refresh-alm-plan-data.js already writes its per-phase error detail. Stays non-blocking (validator's exit code unchanged). + test: malformed plan → broken reconcile is surfaced AND exit code is unchanged; + quiet-on-success assertion on the heal test. 2. render-alm-plan.js: COMPLETED_AT is optional, not required. Move it out of the required-keys list into its own "optional lifecycle key" note — it's present only once the plan reaches "Completed"; the renderer omits the footer line otherwise. 3. powerpages-hook-utils.js: isAlmPlanSkill JSDoc said @PARAM {string} but the function (and its tests) accept non-strings (null/undefined → false). Widen to {*} and note the contract. Full suite: 1247 pass. legacy-compat + alm-lint: clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan-alm: move PLAN_STATUS badge next to the "Generated" line The Draft/Approved/In Execution/Completed status badge sat top-right of the topbar (justify-content:space-between). Move it inline next to the "Generated <timestamp>" sub-line via a new .topbar-sub-row flexbox, so the status reads alongside the plan metadata rather than floating in the corner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan-alm: deterministic Draft/Approved status write + consistency guard + in-place approve The Draft/Approved tag was the only PLAN_STATUS transition with no helper behind it — plan-alm Phase 4 set it via hand-authored Edits to the HTML spans AND the JSON. Since the badge + approver are re-derived from docs/.alm-plan-data.json on every render, the manual HTML Edit was non-durable, and a partial write left the plan "approver recorded but PLAN_STATUS=Draft" — stuck forever (check-alm-plan only promotes from Approved; evaluatePlanCompletion ignores Draft). - NEW scripts/lib/set-plan-status.js: single deterministic owner of the creation-time Draft/Approved write. Writes PLAN_STATUS + PLAN_MODE + APPROVED_BY + APPROVAL_DATE together (atomic temp+rename) and optionally re-renders (reuses refresh-alm-plan-data.js findRendererPath/invokeRenderer, now exported). Invariants: only Draft/Approved settable here; Approved requires a non-empty approver; Draft clears the approver; a live (In Execution/Completed) plan is not re-drafted without --force. + 10 unit tests. - validate-plan-alm.js: consistency guard blocks the two half-written states (Draft+approver, Approved+no-approver) for plans created the old way or hand-edited. + 5 tests. - plan-alm Phase 4: both save options now call set-plan-status.js instead of hand-editing the HTML spans + JSON. - plan-alm Phase 1 step 0b: in-place Draft -> Approved fast-path (new gate plan-alm:1.approve-draft) — approve an existing draft without a full re-plan. - approval-gates.md: catalog the new gate. AGENTS.md: document the helper + lifecycle ownership. Version -> 2.6.0 (2.5.0 held by #194). Full suite: 1262 pass. alm-lint: 0 findings. legacy-compat: in sync. End-to-end: stuck state blocks (2) -> set-plan-status -> badge Approved -> validator approves (0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Sync legacy JSON mirrors to power-pages v2.6.0 (mirror drift after merging main) The merge resolution left .claude-plugin/marketplace.json and plugins/power-pages/.claude-plugin/plugin.json at 2.5.0 while the sources (marketplace.json + .plugin/plugin.json) are bumped to 2.6.0 for this PR. validate-legacy-compatibility requires each mirror to JSON-deep-equal its source (main #203), so bump both mirrors to 2.6.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan-alm: address #202 review — robust approver coercion + runnable remediation cmds Three Copilot findings on validate-plan-alm.js: 1. APPROVED_BY was assumed to be a string — a hand-edited plan-data with a truthy non-string (number/object) made `.trim()` throw, which escaped runValidation and SILENTLY APPROVED, bypassing the consistency guard. Coerce with String() first. Added a regression test (Draft + numeric APPROVED_BY must still block, exit 2). 2-3. The two block-message remediation hints showed a bare `scripts/lib/set-plan-status.js --status ...` with no `node` and no --projectRoot, so following them literally failed with "--projectRoot is required". Replace with a copy/pasteable command matching the skill docs: `node "${PLUGIN_ROOT}/scripts/lib/set-plan-status.js" --projectRoot "<root>" --status Approved --approver "..." --render`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan-alm: fix 5 issues surfaced by an EDM-site end-to-end run All five confirmed against source (and PAC 2.8.1) during a data-model/EDM site run of plan-alm: 1. getEnvironmentUrl() grepped "Environment URL:" but `pac env who` on PAC 2.8.x prints the URL under "Org URL:" → returned null, so every caller relying on the pac-env-who fallback (verify-alm-prerequisites without --envUrl, the datamodel / solution / permissions validators, and the #204 declarative-site path) silently failed. Match either label. Extracted parseEnvironmentUrl() (pure) + tests. 2. `pac env list --output json` is INVALID on PAC 2.8.1 (env list accepts only --filter), so ENV_LIST pre-fill never worked. New list-environments.js parses the plain `pac env list` table into JSON {displayName, environmentId, environmentUrl, uniqueName, active}; verified against live PAC (237 rows). Updated all call sites (plan-alm, setup-pipeline, ensure-pipelines-host, cicd-pipeline-patterns.md) + AGENTS.md. Pure parseEnvList() + tests. 3. estimate-solution-size.js hardcoded siteType:'code-site', mislabeling every EDM/data-model site. Now resolves via new --siteType arg (plan-alm passes SITE_TYPE from Phase 1) with a local marker fallback, emitting canonical 'code' | 'data-model' | 'unknown'. resolveSiteType() + tests. 4. plan-alm risk rule #9 emitted a spurious "pipeline host resolution did not run" warning whenever a pipeline already exists (rawDiscovery.hostResolution is legitimately null then, per the Phase 1 Step 12 skip rule). Added a PIPELINE_DONE carve-out so the warning only fires for fresh-pipeline projects. 5. NEW env-match guard (plan-alm Phase 1 step 6b, gate plan-alm:1.env-match, warn+prompt): cross-checks `pac env who` against the project's recorded env URL (powerpages.config.json / .solution-manifest.json) and a websiteRecordId existence probe, so discovery can't silently run against the wrong environment. Catalogued in approval-gates.md §6.1; alm-lint 0 findings. 1279 tests pass (+10). Riding the existing 2.6.0 bump on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan-alm: rename siteType data-model → declarative + final-review fixes Rename (canonical, build-axis label only — nothing branches on the value, and it is NOT rendered in the plan HTML; it lives in docs/.alm-plan-data.json + the estimator's diagnostic field + agent prose): - detect-project-context.js now emits siteType 'declarative' (was 'data-model'); 'data-model' documented as the legacy alias (older plan-data stays equivalent). - estimate-solution-size.js resolveSiteType returns 'declarative' and NORMALIZES a legacy 'data-model' arg → 'declarative'; non-canonical values (e.g. an unsubstituted "{SITE_TYPE}" literal) are ignored in favor of the marker probe. - plan-alm SKILL.md SITE_TYPE prose + planData comment; AGENTS.md contract; validation-helpers / check-activation-status comments. Left the unrelated "Dataverse data model / EDM" prose in pipeline skills untouched. Tests updated. - Corrected the SKILL.md claim that siteType is "surfaced in the plan" — it isn't. Final-review fixes (from the 8-angle review pass): - plan-alm Step 4: stop telling the agent to grep "Environment URL" from `pac env who` — PAC 2.8.x prints "Org URL:"; read either label (matches the getEnvironmentUrl fix), and fall back to verify-alm-prerequisites' resolved URL. - Removed mojibake ("À-côté" → "or wrong") in the Step 6b env-match prose. - approval-gates.md §6.1 header count 16 → 17 (matches the 17 catalog rows). - list-environments.js docstring: corrected the `pac admin list` rationale (admin-scoped + different shape, not "tenant-wide admin-only"). 1281 tests pass (+2). alm-lint 0, legacy-compat in sync, version-check pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * set-plan-status: make --render atomic across plan-data + HTML Addresses #202 review: previously the new plan-data was renamed into place BEFORE the renderer ran, so a renderer failure left docs/.alm-plan-data.json updated but docs/alm-plan.html stale while the CLI exited non-zero — a caller committing docs/ would ship a new JSON beside a stale HTML. Now stage plan-data to a temp file, render FROM the temp into a temp HTML, and only swap both into place after a clean render. A render failure discards both staged files, leaving plan-data AND the HTML byte-for-byte unchanged so the caller can retry cleanly. Added a regression test (failing renderer → both files untouched, no leftover temps). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * estimate-solution-size: fix resolveSiteType docstring (legacy alias is normalized, not echoed) Addresses #202 review: the comment said a caller passing the legacy 'data-model' value is "echoed unchanged", but the code normalizes 'data-model' -> 'declarative'. Corrected the docstring to say it is normalized so the output is always canonical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On some Windows/client setups,
.claude-plugin/marketplace.jsonis materialized as symlink text (../marketplace.json) instead of dereferenced JSON, causing/plugin marketplace add microsoft/power-platform-skillsto fail withUnexpected token '.'. This change makes legacy manifest paths always parseable JSON while preserving backward compatibility metadata.Legacy manifest compatibility model
.claude-plugin/marketplace.jsonnow mirrorsmarketplace.jsonplugins/*/.claude-plugin/plugin.jsonnow mirrorsplugins/*/.plugin/plugin.jsonCompatibility validation update
scripts/validate-legacy-compatibility.jsto validate JSON equivalence between Open Plugins manifests and legacy.claude-pluginmirrors (instead of asserting symlink targets).Repository guidance update
AGENTS.mdandREADME.mdto document mirror-based legacy compatibility and the requirement to keep source + legacy manifests in sync on metadata changes.