🩹 fix: Keep Skill Parser Mock-Safe and Read Continued Body Flags

Three follow-ups on the invocation-mode work.

CI: parse.ts built its key lookup at module scope from a `SKILL_BOOLEAN_FLAGS`
value imported out of data-schemas. Suites that replace that module with a
partial mock (agents/openai/service.spec.ts mocks it as `logger` alone) left the
import undefined, so the map construction threw before any test ran and took six
`api` suites plus one `@librechat/api` shard down with it. The table is declared
locally again — this module is pure text parsing and must load without the DB
package initialized — and parse.test.ts asserts it still matches data-schemas.

Codex P1: a body-only edit unset the derived column but left the stored
frontmatter bag's copy in place, so `backfillDerivedFromFrontmatter` read the
restriction back on the next `getSkillByName` and the release undid itself. A
body-driven update now clears the bag's flag keys, handing authority to the
columns; the SKILL.md body still carries the declarations.

Codex P2 / Copilot: the body scanner treated `user-invocable:` with its value on
the following line as an unwritten placeholder, and skipping every indented line
also blinded it to a frontmatter block indented as a whole. It now reads keys at
the mapping's own indentation and follows a lone indented scalar as a
continuation value, matching what the import/sync parser already accepted.

packages/api/src/skills/__tests__/parse.test.ts

@@ -0,0 +1,15 @@
+import { SKILL_BOOLEAN_FLAGS as SCHEMA_BOOLEAN_FLAGS } from '@librechat/data-schemas';
+import { SKILL_BOOLEAN_FLAGS } from '../parse';
+
+/**
+ * The parser keeps its own copy of the flag table so it stays loadable when a
+ * suite replaces `@librechat/data-schemas` with a partial mock. This test is
+ * what keeps that copy honest: the parser decides which keys are read out of a
+ * SKILL.md, data-schemas decides which columns they feed, and a mismatch would
+ * silently drop a flag on one side only.
+ */
+describe('SKILL_BOOLEAN_FLAGS', () => {
+  it('matches the table in data-schemas exactly', () => {
+    expect(SKILL_BOOLEAN_FLAGS).toEqual(SCHEMA_BOOLEAN_FLAGS);
+  });
+});

packages/api/src/skills/parse.ts

@@ -1,7 +1,21 @@
 import yaml from 'js-yaml';
-import { SKILL_BOOLEAN_FLAGS } from '@librechat/data-schemas';
 import type { SkillBooleanFlag, SkillBooleanColumn } from '@librechat/data-schemas';
 
+/**
+ * Boolean frontmatter flags mirrored onto first-class skill columns, declared
+ * locally on purpose. This module is pure text parsing and must stay loadable
+ * without `@librechat/data-schemas` initialized: several suites replace that
+ * module with a partial mock, and reading a value export from it at module
+ * scope resolves to `undefined` and throws before any test runs. The type is
+ * still imported (erased at compile time), and `parse.test.ts` asserts this
+ * table matches `SKILL_BOOLEAN_FLAGS` in data-schemas so the two cannot drift.
+ */
+export const SKILL_BOOLEAN_FLAGS: readonly SkillBooleanFlag[] = [
+  { column: 'alwaysApply', key: 'always-apply', aliases: ['alwaysApply'] },
+  { column: 'userInvocable', key: 'user-invocable', aliases: [] },
+  { column: 'disableModelInvocation', key: 'disable-model-invocation', aliases: [] },
+];
+
 export type ParsedSkillMarkdown = {
   name: string;
   description: string;

packages/data-schemas/src/methods/skill.spec.ts

@@ -968,6 +968,78 @@ describe('Skill CRUD methods', () => {
       expect(updated.skill.disableModelInvocation).toBeUndefined();
     });
 
+    it('releasing a restriction survives a lookup that backfills from frontmatter', async () => {
+      /* An imported skill carries the flags in BOTH its body and its stored
+         bag. A body-only edit unsets the column, but if the bag kept its copy
+         `backfillDerivedFromFrontmatter` would read the restriction back on the
+         next lookup and the release would silently undo itself. */
+      const { skill } = await methods.createSkill(
+        makeSkillInput({
+          name: 'release-survives',
+          body: bodyWithFlags,
+          frontmatter: { 'user-invocable': false, 'disable-model-invocation': true },
+        }),
+      );
+      expect(skill.userInvocable).toBe(false);
+
+      await methods.updateSkill({
+        id: skill._id.toString(),
+        expectedVersion: skill.version,
+        update: {
+          body: '---\nname: release-survives\ndescription: A demo skill.\n---\n\nBody.',
+        },
+      });
+
+      const viaId = await methods.getSkillById(skill._id);
+      expect(viaId?.frontmatter).not.toHaveProperty('user-invocable');
+      expect(viaId?.frontmatter).not.toHaveProperty('disable-model-invocation');
+
+      /* getSkillByName runs the backfill; the restriction must stay released. */
+      const viaName = await methods.getSkillByName('release-survives', [skill._id]);
+      expect(viaName?.userInvocable).toBeUndefined();
+      expect(viaName?.disableModelInvocation).toBeUndefined();
+    });
+
+    it('reads a flag whose YAML value continues on the next line', async () => {
+      const { skill } = await methods.createSkill(
+        makeSkillInput({
+          name: 'continued-value',
+          body: '---\nname: continued-value\ndescription: A demo skill.\nuser-invocable:\n  false\n---\n\nBody.',
+          frontmatter: undefined,
+        }),
+      );
+      expect(skill.userInvocable).toBe(false);
+    });
+
+    it('reads flags from a frontmatter block that is indented as a whole', async () => {
+      const { skill } = await methods.createSkill(
+        makeSkillInput({
+          name: 'indented-block',
+          body: '---\n  name: indented-block\n  description: A demo skill.\n  disable-model-invocation: true\n---\n\nBody.',
+          frontmatter: undefined,
+        }),
+      );
+      expect(skill.disableModelInvocation).toBe(true);
+    });
+
+    it('does not unset a column when the body still declares it on a continued line', async () => {
+      const { skill } = await methods.createSkill(
+        makeSkillInput({ name: 'continued-keeps', body: bodyWithFlags, frontmatter: undefined }),
+      );
+      expect(skill.userInvocable).toBe(false);
+
+      const updated = await methods.updateSkill({
+        id: skill._id.toString(),
+        expectedVersion: skill.version,
+        update: {
+          body: '---\nname: continued-keeps\ndescription: A demo skill.\nuser-invocable:\n  false\n---\n\nEdited.',
+        },
+      });
+      expect(updated.status).toBe('updated');
+      if (updated.status !== 'updated') return;
+      expect(updated.skill.userInvocable).toBe(false);
+    });
+
     it('updateSkill leaves the columns alone when the update touches neither body nor frontmatter', async () => {
       const { skill } = await methods.createSkill(
         makeSkillInput({ name: 'untouched-columns', body: bodyWithFlags, frontmatter: undefined }),

packages/data-schemas/src/methods/skill.ts

@@ -797,6 +797,55 @@ function extractBodyFrontmatterBlock(body: string | undefined): string | null {
   return after.slice(0, closingIdx);
 }
 
+function indentWidth(line: string): number {
+  let width = 0;
+  while (width < line.length && (line[width] === ' ' || line[width] === '\t')) {
+    width++;
+  }
+  return width;
+}
+
+/**
+ * Indentation of the frontmatter mapping's own keys, taken from its first
+ * content line. Usually zero, but a block whose every key is indented is still
+ * valid YAML and its keys are still top-level.
+ */
+function findMappingIndent(lines: string[]): number | null {
+  for (const line of lines) {
+    const trimmed = line.trim();
+    if (trimmed.length === 0 || trimmed.startsWith('#')) {
+      continue;
+    }
+    return indentWidth(line);
+  }
+  return null;
+}
+
+/**
+ * Read a flag value that YAML continues on a following line, as in
+ * `user-invocable:` followed by an indented `false`. Only a lone indented
+ * scalar qualifies — anything carrying its own `:` is a nested mapping, which
+ * makes the flag a mapping rather than a boolean.
+ */
+function readContinuedFlagValue(
+  lines: string[],
+  keyIndex: number,
+  baseIndent: number,
+): BodyAlwaysApplyResult | null {
+  for (let i = keyIndex + 1; i < lines.length; i++) {
+    const line = lines[i];
+    const trimmed = line.trim();
+    if (trimmed.length === 0 || trimmed.startsWith('#')) {
+      continue;
+    }
+    if (indentWidth(line) <= baseIndent || trimmed.includes(':')) {
+      return null;
+    }
+    return readBodyFlagValue(trimmed);
+  }
+  return null;
+}
+
 function readBodyFlagValue(rawValue: string): BodyAlwaysApplyResult {
   /* Strip the YAML inline comment BEFORE unquoting — a line like
      `always-apply: "true" # note` has both, and handling whole-line quoting
@@ -844,9 +893,12 @@ function readBodyFlagValue(rawValue: string): BodyAlwaysApplyResult {
  *    from what the saved SKILL.md text says.
  *
  * The first canonical spelling wins; a legacy alias (`alwaysApply`) is only
- * consulted when the canonical key never appears. Indented lines are skipped:
- * a nested mapping that reuses a flag name (`metadata:` → `  user-invocable:`)
- * is not a top-level declaration and must not be read as one.
+ * consulted when the canonical key never appears. Only keys at the mapping's
+ * own indentation are read, so a nested mapping reusing a flag name
+ * (`metadata:` → `  user-invocable:`) is not mistaken for a declaration, while
+ * a frontmatter block that is indented as a whole still parses. A flag whose
+ * value YAML continues onto the following line is read from there rather than
+ * being treated as an unwritten placeholder.
  */
 function extractBooleanFlagsFromBody(body: string | undefined): BodyFlagResults {
   const results: BodyFlagResults = {
@@ -858,22 +910,32 @@ function extractBooleanFlagsFromBody(body: string | undefined): BodyFlagResults
   if (block === null) {
     return results;
   }
+  const lines = block.split('\n');
+  const baseIndent = findMappingIndent(lines);
+  if (baseIndent === null) {
+    return results;
+  }
   const canonical = new Map<SkillBooleanColumn, BodyAlwaysApplyResult>();
   const aliased = new Map<SkillBooleanColumn, BodyAlwaysApplyResult>();
-  for (const line of block.split('\n')) {
-    if (line.length === 0 || line[0] === ' ' || line[0] === '\t') {
+  for (let i = 0; i < lines.length; i++) {
+    const line = lines[i];
+    const trimmed = line.trim();
+    if (trimmed.length === 0 || trimmed.startsWith('#') || indentWidth(line) !== baseIndent) {
       continue;
     }
-    const colon = line.indexOf(':');
+    const colon = trimmed.indexOf(':');
     if (colon === -1) {
       continue;
     }
-    const key = line.slice(0, colon).trim().toLowerCase();
+    const key = trimmed.slice(0, colon).trim().toLowerCase();
     const flag = BODY_FLAG_BY_KEY.get(key);
     if (!flag) {
       continue;
     }
-    const result = readBodyFlagValue(line.slice(colon + 1));
+    let result = readBodyFlagValue(trimmed.slice(colon + 1));
+    if (result.status === 'absent') {
+      result = readContinuedFlagValue(lines, i, baseIndent) ?? result;
+    }
     if (key === flag.key) {
       if (!canonical.has(flag.column)) {
         canonical.set(flag.column, result);
@@ -1609,6 +1671,22 @@ export function createSkillMethods(
         unsetPayload[column] = '';
       }
     }
+    /**
+     * A body-only edit rewrites the SKILL.md text without sending a bag, so the
+     * stored bag would keep flag keys the edited text no longer declares — and
+     * `backfillDerivedFromFrontmatter` reads those back over an unset column on
+     * the next lookup, resurrecting a restriction the author just removed.
+     * Hand flag authority to the columns resolved above by clearing the bag's
+     * copies; the SKILL.md body still carries the declarations, so nothing is
+     * lost, and the next save that does send a bag repopulates them.
+     */
+    if (update.frontmatter === undefined && bodyFlags) {
+      for (const flag of SKILL_BOOLEAN_FLAGS) {
+        for (const key of [flag.key, ...flag.aliases]) {
+          unsetPayload[`frontmatter.${key}`] = '';
+        }
+      }
+    }
     if (update.category !== undefined) setPayload.category = update.category;
     /**
      * Keep the indexed `alwaysApply` column in sync with whatever the update