Commit 64b8451
[Pages][ALM] plan-alm: deterministic Draft/Approved status writer + EDM-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>1 parent c419336 commit 64b8451
31 files changed
Lines changed: 1187 additions & 64 deletions
File tree
- .claude-plugin
- plugins/power-pages
- .claude-plugin
- .plugin
- references
- scripts
- lib
- tests
- skills
- deploy-pipeline
- ensure-pipelines-host
- plan-alm
- scripts
- setup-pipeline
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
200 | | - | |
201 | | - | |
| 200 | + | |
| 201 | + | |
202 | 202 | | |
203 | 203 | | |
| 204 | + | |
| 205 | + | |
204 | 206 | | |
205 | 207 | | |
206 | 208 | | |
| |||
231 | 233 | | |
232 | 234 | | |
233 | 235 | | |
| 236 | + | |
234 | 237 | | |
235 | 238 | | |
236 | 239 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
256 | 256 | | |
257 | 257 | | |
258 | 258 | | |
259 | | - | |
| 259 | + | |
260 | 260 | | |
261 | 261 | | |
262 | 262 | | |
263 | 263 | | |
264 | 264 | | |
265 | 265 | | |
| 266 | + | |
266 | 267 | | |
| 268 | + | |
267 | 269 | | |
268 | 270 | | |
269 | 271 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
513 | 513 | | |
514 | 514 | | |
515 | 515 | | |
516 | | - | |
| 516 | + | |
517 | 517 | | |
518 | 518 | | |
519 | | - | |
| 519 | + | |
520 | 520 | | |
521 | 521 | | |
522 | | - | |
| 522 | + | |
523 | 523 | | |
524 | 524 | | |
525 | 525 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
0 commit comments