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>

Author: tyaginidhi

plugins/power-pages/hooks/run-skill-posttool-validation.js

@@ -71,26 +71,45 @@ process.stdin.on('end', () => {
           cwd,
           timeout: 20000,
         });
+        // spawnSync surfaces a spawn/timeout failure on rec.error (e.g. ETIMEDOUT)
+        // and a non-zero / signalled exit on rec.status / rec.signal — none of which
+        // produce parseable stdout. Track those so a broken reconcile is reported
+        // rather than silently swallowed by the JSON.parse catch below.
+        const spawnFailed = !!rec.error || rec.status !== 0 || !!rec.signal;
         let reconciled = [];
         let failed = [];
+        let parsed = false;
         try {
           const out = JSON.parse((rec.stdout || '').trim());
           reconciled = out.reconciled || [];
           failed = out.failed || [];
-        } catch {}
+          parsed = true;
+        } catch { /* parsed stays false — surfaced in the spawnFailed/!parsed branch */ }
         if (reconciled.length > 0) {
           process.stdout.write(
             `[power-pages] ALM plan was out of sync with ${reconciled.length} run marker(s) — refreshed automatically (${reconciled.join(', ')}).\n`,
           );
         }
+        // Failure reporting goes to STDERR (only on an actual failure, never on the
+        // happy path — the hook fires on every Skill use, so clean runs must stay
+        // quiet). A swallowed reconcile failure is exactly what makes a stale plan
+        // impossible to diagnose, so we forward the child's stderr verbatim — that's
+        // where refresh-alm-plan-data.js already writes its per-phase error detail,
+        // which is what makes the summary line below actionable.
         if (failed.length > 0) {
-          // Non-blocking, but surfaced — a swallowed reconcile failure is exactly
-          // what makes a stale plan impossible to diagnose.
-          process.stdout.write(
-            `[power-pages] ALM plan reconcile could not heal ${failed.length} phase(s): ${failed.map((f) => f.phase).join(', ')}. See stderr for details.\n`,
+          process.stderr.write(
+            `[power-pages] ALM plan reconcile could not heal ${failed.length} phase(s): ${failed.map((f) => f.phase).join(', ')}. Details below.\n`,
           );
+          if (rec.stderr) process.stderr.write(rec.stderr);
+        } else if (spawnFailed || !parsed) {
+          // The reconcile didn't even produce a parseable result (spawn error,
+          // timeout, non-zero exit, or garbled stdout). Non-blocking, but the user
+          // should still see why the auto-heal didn't run.
+          const why = rec.error ? rec.error.message : rec.signal ? `signal ${rec.signal}` : `exit ${rec.status}`;
+          process.stderr.write(`[power-pages] ALM plan reconcile did not complete (${why}). Details below.\n`);
+          if (rec.stderr) process.stderr.write(rec.stderr);
         }
-        debug(`[power-pages hook] reconcile reconciled=${JSON.stringify(reconciled)} failed=${JSON.stringify(failed)}\n`);
+        debug(`[power-pages hook] reconcile reconciled=${JSON.stringify(reconciled)} failed=${JSON.stringify(failed)} spawnFailed=${spawnFailed} parsed=${parsed}\n`);
       } catch (e) {
         // Best-effort — a reconcile failure must never break the skill or the hook.
         debug(`[power-pages hook] reconcile error (ignored): ${e.message}\n`);

plugins/power-pages/scripts/lib/powerpages-hook-utils.js

@@ -126,7 +126,9 @@ const ALM_PLAN_SKILLS = new Set([
  * True when `value` (a raw skill name, `/skill`, or `power-pages:skill`) resolves
  * to an ALM plan skill. Normalizes via `detectTrackedSkill`, so it also confirms
  * the skill actually exists in this plugin.
- * @param {string} value
+ * Accepts any value — non-strings (including null/undefined) resolve to false
+ * via detectTrackedSkill, so callers may pass an unvalidated skill name.
+ * @param {*} value
  * @returns {boolean}
  */
 function isAlmPlanSkill(value) {

plugins/power-pages/scripts/tests/run-skill-posttool-validation.test.js

@@ -84,6 +84,28 @@ test('hook spawns the reconcile backstop and heals a skipped refresh after an AL
   const planData = readJson(path.join(root, 'docs', '.alm-plan-data.json'));
   assert.equal(planData.pipelineMeta.lastDeploy.status, 'Succeeded');
   assert.equal(planData.pipelineMeta.lastDeploy.componentCount, 118);
+  // A clean reconcile must stay quiet on stderr — the hook fires on every Skill
+  // use, so success must not produce failure noise.
+  assert.doesNotMatch(res.stderr, /reconcile/i, 'a successful reconcile must not write failure noise to stderr');
+});
+
+test('hook forwards reconcile failure detail to stderr without changing the exit code', (t) => {
+  // A malformed plan file makes refresh-alm-plan-data.js --reconcile throw and exit
+  // non-zero with its reason on stderr. The hook must (a) surface that — the prior
+  // empty JSON.parse catch swallowed spawn errors / timeouts / non-zero exits — and
+  // (b) stay non-blocking (the reconcile is best-effort; the validator's status stands).
+  const root = makeProject(t);
+  fs.writeFileSync(path.join(root, 'docs', '.alm-plan-data.json'), 'not json {{{', 'utf8');
+  // A newer marker guarantees the reconcile reaches the plan-parse (and would heal if it could).
+  writeJson(path.join(root, 'docs', 'alm', 'last-export.json'), { solutionUniqueName: 'S', exportedAt: '2026-06-16T00:00:00.000Z' });
+  backdatePlan(root);
+
+  // export-solution is an ALM skill; its validator gracefully approves (no zip) → exit 0.
+  const res = runHook(root, 'export-solution');
+
+  assert.equal(res.status, 0, 'a broken reconcile must not change the validator-determined exit code');
+  assert.match(res.stderr, /reconcile did not complete/i, 'the hook must report the broken reconcile');
+  assert.match(res.stderr, /Could not parse/i, 'the child reconcile stderr must be forwarded verbatim');
 });
 
 test('hook does NOT reconcile for a non-ALM skill even when a plan + newer marker exist', (t) => {

plugins/power-pages/skills/plan-alm/scripts/render-alm-plan.js

@@ -6,9 +6,14 @@
  *   node render-alm-plan.js --output <path> --data <json-file>
  *
  * Required top-level keys in the JSON data file:
- *   SITE_NAME, GENERATED_AT, STRATEGY, PLAN_STATUS, APPROVED_BY, APPROVAL_DATE, COMPLETED_AT,
+ *   SITE_NAME, GENERATED_AT, STRATEGY, PLAN_STATUS, APPROVED_BY, APPROVAL_DATE,
  *   stages, steps, risks
  *
+ * Optional lifecycle key:
+ *   COMPLETED_AT — present only once the plan reaches PLAN_STATUS "Completed";
+ *   the renderer emits the footer "Completed" line when it exists and omits it
+ *   otherwise (an in-flight plan has no COMPLETED_AT).
+ *
  * Optional v2 keys (added for split-solutions support):
  *   sizeAnalysis, assetAdvisory, proposedSolutions, appliedStrategies,
  *   recommendations, envVars, breakdown, estimationMethod, estimationAccuracyPct