fix: Harden GitHub skill sync review paths

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

@@ -173,6 +173,15 @@ describe('parseFrontmatter', () => {
     });
   });
 
+  it('returns empty fields when frontmatter YAML is malformed', () => {
+    const raw = `---\nname: [\n---\n\nbody`;
+    expect(parseFrontmatter(raw)).toEqual({
+      name: '',
+      description: '',
+      invalidBooleans: [],
+    });
+  });
+
   it('ignores always-apply appearing outside the frontmatter block', () => {
     const raw = `---\nname: n\ndescription: d\n---\n\nalways-apply: true (but this is in the body)`;
     const result = parseFrontmatter(raw);

packages/api/src/skills/parse.ts

@@ -6,6 +6,7 @@ export type ParsedSkillMarkdown = {
   alwaysApply?: boolean;
   frontmatter?: Record<string, unknown>;
   invalidBooleans: string[];
+  parseError?: string;
 };
 
 function isPlainObject(value: unknown): value is Record<string, unknown> {
@@ -58,7 +59,17 @@ export function parseSkillMarkdown(raw: string): ParsedSkillMarkdown {
   if (!block) {
     return { name: '', description: '', invalidBooleans: [] };
   }
-  const parsed = yaml.load(block);
+  let parsed: unknown;
+  try {
+    parsed = yaml.load(block);
+  } catch (error) {
+    return {
+      name: '',
+      description: '',
+      invalidBooleans: [],
+      parseError: error instanceof Error ? error.message : 'Invalid YAML frontmatter',
+    };
+  }
   const frontmatter = isPlainObject(parsed) ? normalizeFrontmatterKeys(parsed) : {};
   const nameValue = getCaseInsensitive(frontmatter, 'name');
   const descriptionValue = getCaseInsensitive(frontmatter, 'description');

packages/api/src/skills/sync/github.spec.ts

@@ -30,6 +30,47 @@ function blob(content: string) {
   };
 }
 
+function githubFetch(
+  skillMarkdown = '---\nname: research\ndescription: Research things\nalways-apply: true\n---\nBody',
+): typeof fetch {
+  return jest.fn(async (input: RequestInfo | URL) => {
+    const url = input.toString();
+    if (url.includes('/commits/main')) {
+      return response({ sha: 'commit-sha', commit: { tree: { sha: 'tree-sha' } } });
+    }
+    if (url.includes('/git/trees/tree-sha')) {
+      return response({
+        sha: 'tree-sha',
+        truncated: false,
+        tree: [
+          {
+            path: 'skills/research/SKILL.md',
+            mode: '100644',
+            type: 'blob',
+            sha: 'skill-md-sha',
+            url: 'https://api.github.test/blob/skill',
+          },
+          {
+            path: 'skills/research/scripts/run.sh',
+            mode: '100644',
+            type: 'blob',
+            sha: 'file-sha',
+            size: 7,
+            url: 'https://api.github.test/blob/file',
+          },
+        ],
+      });
+    }
+    if (url.includes('/git/blobs/skill-md-sha')) {
+      return response(blob(skillMarkdown));
+    }
+    if (url.includes('/git/blobs/file-sha')) {
+      return response(blob('echo ok'));
+    }
+    return response({ message: 'not found' }, 404);
+  }) as unknown as typeof fetch;
+}
+
 function makeSkill(input: CreateSkillInput): ISkill & { _id: Types.ObjectId } {
   return {
     _id: new Types.ObjectId(),
@@ -135,44 +176,7 @@ function createDeps(
     saveBuffer: jest.fn(async () => ({ filepath: '/uploads/file-id__run.sh', source: 'local' })),
     deleteFile: jest.fn(async () => undefined),
     grantPermission: jest.fn(async () => undefined),
-    fetchFn: jest.fn(async (input: RequestInfo | URL) => {
-      const url = input.toString();
-      if (url.includes('/commits/main')) {
-        return response({ sha: 'commit-sha', commit: { tree: { sha: 'tree-sha' } } });
-      }
-      if (url.includes('/git/trees/tree-sha')) {
-        return response({
-          sha: 'tree-sha',
-          truncated: false,
-          tree: [
-            {
-              path: 'skills/research/SKILL.md',
-              mode: '100644',
-              type: 'blob',
-              sha: 'skill-md-sha',
-              url: 'https://api.github.test/blob/skill',
-            },
-            {
-              path: 'skills/research/scripts/run.sh',
-              mode: '100644',
-              type: 'blob',
-              sha: 'file-sha',
-              size: 7,
-              url: 'https://api.github.test/blob/file',
-            },
-          ],
-        });
-      }
-      if (url.includes('/git/blobs/skill-md-sha')) {
-        return response(
-          blob('---\nname: research\ndescription: Research things\nalways-apply: true\n---\nBody'),
-        );
-      }
-      if (url.includes('/git/blobs/file-sha')) {
-        return response(blob('echo ok'));
-      }
-      return response({ message: 'not found' }, 404);
-    }) as unknown as typeof fetch,
+    fetchFn: githubFetch(),
     ...overrides,
   };
   return deps;
@@ -234,4 +238,24 @@ describe('createGitHubSkillSyncRunner', () => {
       }),
     );
   });
+
+  it('marks a source failed and skips mirror deletion when SKILL.md frontmatter is malformed', async () => {
+    const deps = createDeps({
+      fetchFn: githubFetch('---\nname: [\n---\nBody'),
+    });
+    const runner = createGitHubSkillSyncRunner(deps);
+    const result = await runner.runOnce();
+
+    expect(result.status).toBe('failed');
+    expect(deps.createSkill).not.toHaveBeenCalled();
+    expect(deps.listSkillsBySource).not.toHaveBeenCalled();
+    expect(deps.deleteSkill).not.toHaveBeenCalled();
+    expect(deps.upsertStatus).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        status: 'failed',
+        errorCode: 'SKILL_PARSE_FAILED',
+        errorMessage: expect.stringContaining('skills/research/SKILL.md'),
+      }),
+    );
+  });
 });

packages/api/src/skills/sync/github.ts

@@ -485,6 +485,12 @@ async function upsertRemoteSkill(params: {
 }): Promise<ISkill & { _id: Types.ObjectId }> {
   const { deps, source, discovered, skillMdContent, commitSha, syncedAt } = params;
   const parsed = parseSkillMarkdown(skillMdContent);
+  if (parsed.parseError) {
+    throw new SkillSyncError(
+      'SKILL_PARSE_FAILED',
+      `${discovered.rootPath}/SKILL.md contains invalid YAML frontmatter: ${parsed.parseError}`,
+    );
+  }
   if (parsed.invalidBooleans.length > 0) {
     throw new SkillSyncError(
       'SKILL_PARSE_FAILED',

packages/api/src/skills/sync/scheduler.ts

@@ -12,7 +12,14 @@ export function startGitHubSkillSyncScheduler(params: {
   runner: GitHubSkillSyncRunner;
 }): GitHubSkillSyncScheduler | null {
   const github = params.getConfig()?.github;
-  if (!github?.enabled || github.sources.length === 0) {
+  const sources = Array.isArray(github?.sources) ? github.sources : [];
+  const intervalMinutes =
+    typeof github?.intervalMinutes === 'number' &&
+    Number.isFinite(github.intervalMinutes) &&
+    github.intervalMinutes >= 5
+      ? github.intervalMinutes
+      : 60;
+  if (!github?.enabled || sources.length === 0) {
     return null;
   }
   let stopped = false;
@@ -31,7 +38,7 @@ export function startGitHubSkillSyncScheduler(params: {
     run();
   }
 
-  timer = setInterval(run, github.intervalMinutes * 60 * 1000);
+  timer = setInterval(run, intervalMinutes * 60 * 1000);
 
   const scheduler = {
     stop: () => {

packages/data-schemas/src/models/skillSyncCredential.ts

@@ -2,6 +2,8 @@ import skillSyncCredentialSchema from '~/schema/skillSyncCredential';
 import type { ISkillSyncCredentialDocument } from '~/types/skillSync';
 
 export function createSkillSyncCredentialModel(mongoose: typeof import('mongoose')) {
+  // GitHub skill sync is intentionally app-wide in v1; credentials are referenced by
+  // admin-managed config keys and are never returned by tenant-scoped APIs.
   return (
     mongoose.models.SkillSyncCredential ||
     mongoose.model<ISkillSyncCredentialDocument>('SkillSyncCredential', skillSyncCredentialSchema)

packages/data-schemas/src/models/skillSyncStatus.ts

@@ -2,6 +2,8 @@ import skillSyncStatusSchema from '~/schema/skillSyncStatus';
 import type { ISkillSyncStatusDocument } from '~/types/skillSync';
 
 export function createSkillSyncStatusModel(mongoose: typeof import('mongoose')) {
+  // GitHub skill sync status is intentionally app-wide in v1, matching the
+  // shared synced skills and the single scheduler lock across app processes.
   return (
     mongoose.models.SkillSyncStatus ||
     mongoose.model<ISkillSyncStatusDocument>('SkillSyncStatus', skillSyncStatusSchema)