fix: resolve codex skill sync review findings

api/server/services/Skills/sync.js

@@ -176,7 +176,10 @@ function getGitHubSkillSyncRunnerForRequest(req) {
 
 async function maybeRunGitHubSkillSyncForRequest(req) {
   const baseConfig = await loadCurrentAppConfig();
-  return triggerOrchestrator.maybeRunForRequest(withBaseSkillSyncConfig(req, baseConfig));
+  return triggerOrchestrator.maybeRunForRequest({
+    ...withBaseSkillSyncConfig(req, baseConfig),
+    skillSyncAllowServerCredentials: true,
+  });
 }
 
 function initializeGitHubSkillSync(appConfig) {

api/server/services/Skills/sync.test.js

@@ -210,7 +210,7 @@ describe('GitHub skill sync service', () => {
     const requestRunner = mockCreatedRunners[0].runner;
     const requestConfig = await mockCreatedRunners[0].deps.getConfig();
     expect(started).toBe(true);
-    expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(false);
+    expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(true);
     expect(requestRunner.runOnce).toHaveBeenCalledTimes(1);
     expect(requestConfig.github.runOnStartup).toBe(false);
     expect(requestConfig.github.sources[0]).toEqual(
@@ -269,7 +269,7 @@ describe('GitHub skill sync service', () => {
     });
 
     expect(started).toBe(false);
-    expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(false);
+    expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(true);
     expect(mockCreatedRunners[0].runner.runOnce).not.toHaveBeenCalled();
   });
 

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

@@ -377,6 +377,79 @@ describe('createGitHubSkillSyncRunner', () => {
     );
   });
 
+  it('fails duplicate root and nested skill names before publishing partial mirrors', async () => {
+    const duplicateFetch = jest.fn(async (input: RequestInfo | URL) => {
+      const url = input.toString();
+      if (url.includes('/commits/')) {
+        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: 'SKILL.md',
+              mode: '100644',
+              type: 'blob',
+              sha: 'root-skill-sha',
+              size: 50,
+              url: 'https://api.github.test/blob/root-skill',
+            },
+            {
+              path: 'child/SKILL.md',
+              mode: '100644',
+              type: 'blob',
+              sha: 'child-skill-sha',
+              size: 50,
+              url: 'https://api.github.test/blob/child-skill',
+            },
+          ],
+        });
+      }
+      if (url.includes('/git/blobs/root-skill-sha')) {
+        return response(blob('---\nname: duplicate\ndescription: Root\n---\nBody'));
+      }
+      if (url.includes('/git/blobs/child-skill-sha')) {
+        return response(blob('---\nname: duplicate\ndescription: Child\n---\nBody'));
+      }
+      return response({ message: 'not found' }, 404);
+    }) as unknown as typeof fetch;
+    const deps = createDeps({
+      fetchFn: duplicateFetch,
+      getConfig: () => ({
+        github: {
+          enabled: true,
+          intervalMinutes: 60,
+          runOnStartup: false,
+          sources: [
+            {
+              id: 'librechat-skills',
+              owner: 'LibreChat',
+              repo: 'skills',
+              ref: 'main',
+              paths: [''],
+              credentialKey: 'github-skills-prod',
+            },
+          ],
+        },
+      }),
+    });
+    const runner = createGitHubSkillSyncRunner(deps);
+    const result = await runner.runOnce();
+
+    expect(result.status).toBe('failed');
+    expect(deps.createSkill).not.toHaveBeenCalled();
+    expect(deps.updateSkill).not.toHaveBeenCalled();
+    expect(deps.upsertStatus).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        status: 'failed',
+        errorCode: 'DUPLICATE_SKILL_NAME',
+        errorMessage: 'GitHub source "librechat-skills" contains multiple skills named "duplicate"',
+      }),
+    );
+  });
+
   it('discovers nested skill roots within the configured discovery depth', async () => {
     const skillMarkdown = '---\nname: tdd\ndescription: Test-driven development\n---\nBody';
     const fetchFn = jest.fn(async (input: RequestInfo | URL) => {

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

@@ -1082,8 +1082,7 @@ function assertNoDuplicatePreparedSkillNames(
       author: prepared.createInput.author.toString(),
       name: prepared.createInput.name,
     });
-    const previousRoot = seen.get(key);
-    if (previousRoot) {
+    if (seen.has(key)) {
       throw new SkillSyncError(
         'DUPLICATE_SKILL_NAME',
         `GitHub source "${source.id}" contains multiple skills named "${prepared.createInput.name}"`,

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

@@ -169,6 +169,21 @@ describe('createSkillSyncTriggerOrchestrator', () => {
     expect(runners[0].runner.runOnce).not.toHaveBeenCalled();
   });
 
+  it('starts request sync with server credentials when the caller opts in', async () => {
+    const config = skillSync();
+    const { orchestrator, runners } = createHarness();
+
+    const started = await orchestrator.maybeRunForRequest({
+      config: { skillSync: config, config: {} },
+      user: { tenantId: 'tenant-a' },
+      skillSyncAllowServerCredentials: true,
+    });
+
+    expect(started).toBe(true);
+    expect(runners[0].input.allowServerCredentials).toBe(true);
+    expect(runners[0].runner.runOnce).toHaveBeenCalledTimes(1);
+  });
+
   it('does not start request sync for base YAML skillSync config', async () => {
     const config = skillSync();
     const { createRunner, orchestrator } = createHarness();

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

@@ -225,7 +225,7 @@ export function createSkillSyncTriggerOrchestrator(deps: SkillSyncTriggerOrchest
     const requestRunner = deps.createRunner({
       getConfig: async () => config,
       loadAppConfig: async () => request.config,
-      allowServerCredentials: false,
+      allowServerCredentials: Boolean(request.skillSyncAllowServerCredentials),
     });
     const status = await requestRunner.getStatus();
     if (!shouldRunRequestSync(status, { minIntervalMs, staleRunningMs })) {