update Markdown skills

src/lib/skills/registry.js

@@ -1,8 +1,11 @@
 import { PLACEMENT_ROUTES } from '@/components/ai-assistant/placement';
 import { parseYaml } from './yaml';
-import { normalizeSkillUi } from './uiConfig';
+import { normalizeSkillUi, slugify } from './uiConfig';
 import { normalizeSkillOwliver } from './owliverConfig';
-import { SUPPORTED_SKILL_PAGES, canonicalPage, surfaceFor, surfaceForRoute } from './surfaces';
+import {
+  SUPPORTED_SKILL_PAGES, canonicalPage, contextLabel, placementProvides, surfaceFor,
+  surfaceForRoute,
+} from './surfaces';
 
 /**
  * Owliver skill registry.
@@ -30,14 +33,39 @@ const FILES = import.meta.glob('/src/skills/**/*.md', { query: '?raw', import: '
  * A file whose frontmatter cannot be read raises rather than registering a
  * half-understood definition; `parseSkill` decides what to do with that.
  */
-function parseFrontmatter(raw) {
-  const match = /^---\r?\n([\s\S]*?)\r?\n---/.exec(raw);
-  if (!match) return { data: {}, body: raw };
+/**
+ * A definition's text, as the parser needs to see it.
+ *
+ * Files arrive from editors, from Windows, from copy-paste and from downloads,
+ * and four of the things they arrive with used to take the entire frontmatter
+ * block down: a UTF-8 byte-order mark before the opening fence, a blank line
+ * above it, `\r\n` line endings, and trailing spaces after `---`. In every one
+ * of those cases the fence did not match, `parseFrontmatter` returned an empty
+ * record, and the definition registered as `Untitled skill` with no pages —
+ * the file was read, and none of it was believed.
+ *
+ * None of this is a lenient parser: the YAML subset inside the fences is as
+ * strict as it ever was. This is only about recognising that a fence is a
+ * fence.
+ */
+export const normalizeDefinition = (raw) => String(raw ?? '')
+  .replace(/^\uFEFF/, '')
+  .replace(/\r\n?/g, '\n')
+  .replace(/^\s*\n+/, '');
+
+/** Whether this text opens with a frontmatter block at all. */
+export const hasFrontmatter = (raw) => /^---[ \t]*\n[\s\S]*?\n---[ \t]*(?=\n|$)/
+  .test(normalizeDefinition(raw));
+
+export function parseFrontmatter(raw) {
+  const text = normalizeDefinition(raw);
+  const match = /^---[ \t]*\n([\s\S]*?)\n---[ \t]*(?=\n|$)/.exec(text);
+  if (!match) return { data: {}, body: text };
 
   const data = parseYaml(match[1]);
   return {
     data: data && typeof data === 'object' && !Array.isArray(data) ? data : {},
-    body: raw.slice(match[0].length).trim(),
+    body: text.slice(match[0].length).trim(),
   };
 }
 
@@ -261,8 +289,21 @@ export function skillFacets({ data = {}, kind, ui = {}, owliver, conversation =
 export function parseSkill(raw, { path = 'custom', custom = false } = {}) {
   {
     const { data, body } = parseFrontmatter(raw);
-    const pages = Array.isArray(data.pages) ? data.pages : [];
-    const id = data.id || path.split('/').pop().replace(/\.md$/, '');
+    const declaredPages = Array.isArray(data.pages) ? data.pages : [];
+    /**
+     * The definition's id.
+     *
+     * `id:` when it is written, and it always wins — an explicit id is an
+     * address other definitions and stored preferences refer to, and deriving
+     * over the top of one would silently rename a skill.
+     *
+     * The fallback used to be the filename, which is right for a file in
+     * `src/skills/` and wrong for everything else: an uploaded definition is
+     * parsed with the placeholder path `custom`, so a file omitting `id:` was
+     * registered as the skill `custom` and the ID field filled in with the word
+     * "custom". Slugging the name is what an author means by leaving it out.
+     */
+    const id = data.id || slugify(data.name) || path.split('/').pop().replace(/\.md$/, '');
     const levels = sectionLevels(body);
 
     /**
@@ -280,10 +321,21 @@ export function parseSkill(raw, { path = 'custom', custom = false } = {}) {
        closed vocabulary in `surfaces.js`. A definition with no `ui:` block is
        exactly what it was before this existed. */
     const { ui, errors: uiErrors } = normalizeSkillUi(data.ui, {
-      declaredPages: pages,
+      declaredPages,
       skillId: id,
     });
 
+    /**
+     * Where this definition applies.
+     *
+     * `pages:` when it is written. When it is not, the pages its `ui:` entries
+     * name — because a definition that says "put this on Positions and that on
+     * Analytics" has already declared its reach, and making it repeat the list
+     * above the block is the format asking twice. A definition that declares
+     * neither still has none, which is what `validateSkillSource` refuses on.
+     */
+    const pages = declaredPages.length ? declaredPages : Object.keys(ui);
+
     /* The same definition's second consumer. `owliver:` declares what can be
        asked for in the panel, reading the source the page section already
        names — so one file answers "what does this page show" and "what can
@@ -474,6 +526,27 @@ export function readSkillRegistry(customSources = []) {
     }
   }
 
+  /**
+   * A section that registered perfectly and can never draw anything.
+   *
+   * The same check `validateSkillSource` refuses on, run again at load — because
+   * a definition stored before the rule existed was validated under the old one
+   * and is never re-checked. Without this it stays in the workspace as a titled
+   * card reporting that it needs a record the page has no way of giving it, and
+   * the Skills page calls the workspace healthy.
+   */
+  for (const skill of skills) {
+    for (const problem of unresolvableSections(skill)) {
+      diagnostics.push({
+        level: 'error',
+        kind: 'unresolvable',
+        path: skill.path,
+        skillId: skill.id,
+        message: problem.message,
+      });
+    }
+  }
+
   /**
    * Two definitions claiming one phrase on one page.
    *
@@ -530,6 +603,44 @@ export function skillDiagnostics(customSources = []) {
   return readSkillRegistry(customSources).diagnostics;
 }
 
+
+/**
+ * Sections that can never resolve where they are attached.
+ *
+ * A source declares the record it needs; a placement either hands one over or
+ * does not. Nothing compared the two, so the commonest authoring mistake in the
+ * product — `position.activity`, the Board editor's own default, on a page with
+ * no position — validated cleanly, registered, drew its title and then reported
+ * "This section needs a position to read" for good. The definition was never
+ * wrong about anything the product had told it to care about.
+ *
+ * Deliberately *only* the `ui:` half. An Owliver response with an unmet need is
+ * not a dead panel: `resolveEntity` asks which position is meant, and answers
+ * once told — which is why `hiring-activity-assistant` reads `position.activity`
+ * on Positions and works. A card cannot ask. That asymmetry is the reason one
+ * is refused and the other is left alone.
+ */
+export function unresolvableSections(skill) {
+  const problems = [];
+
+  for (const [page, config] of Object.entries(skill?.ui || {})) {
+    for (const section of config.sections || []) {
+      if (!section.context) continue;
+      if (placementProvides(page, section.placement).includes(section.context)) continue;
+      problems.push({
+        page,
+        placement: section.placement,
+        source: section.source,
+        message: `\`${section.source}\` needs ${contextLabel(section.context)} to read, and `
+          + `\`${page}\` supplies none at \`${section.placement}\`. `
+          + `Attach it to a placement that does, or read a source that needs nothing.`,
+      });
+    }
+  }
+
+  return problems;
+}
+
 /**
  * Validates a definition before it is stored. Returns an error string or null.
  *
@@ -565,6 +676,29 @@ export function validateSkillSource(raw) {
      something Owliver offers and then cannot answer. */
   if (skill.owliverErrors?.length) return skill.owliverErrors[0];
 
+  /* A section attached where its source can never be read. See
+     `unresolvableSections` — this is the difference between a definition that
+     is wrong and one that merely looks right. */
+  const unresolvable = unresolvableSections(skill);
+  if (unresolvable.length) return unresolvable[0].message;
+
+  /**
+   * An Owliver block that can answer nothing.
+   *
+   * `owliver: enabled: true` with no capabilities is what the template produces
+   * before an author fills anything in. It registers, claims its own name as a
+   * trigger, can take a question from a definition written to answer it, and
+   * then reads its own description back. Refused here rather than saved and
+   * reported later as "the skill does not work".
+   */
+  if (skill.owliver?.enabled
+    && !skill.owliver.capabilities.length
+    && !skill.conversation.length
+    && !skill.actions.length) {
+    return 'This skill declares no capabilities, so Owliver could only read its description '
+      + 'back. Add a capability, or remove the `owliver:` block.';
+  }
+
   return null;
 }