fix: harden skill sync review edges

api/server/routes/admin/skills.js

@@ -42,10 +42,6 @@ function hasResolvedSkillSyncOverride(req) {
   return Boolean(resolved?.github && !isSameSkillSyncConfig(resolved, base));
 }
 
-function hasServerCredentialReference(config) {
-  return Boolean(config?.github?.sources?.some((source) => source.credentialKey || source.token));
-}
-
 async function attachBaseSkillSyncConfig(req, res, next) {
   try {
     const baseConfig = await getAppConfig({ baseOnly: true });
@@ -107,20 +103,16 @@ async function requireSyncRunCapability(req, res, next) {
     });
     if (canManagePlatform) {
       req.skillSyncAllowServerCredentials = true;
+      req.skillSyncCanReadCredentials = true;
       return next();
     }
-    const resolved = parseSkillSyncConfig(req.config?.skillSync);
     if (
       hasResolvedSkillSyncOverride(req) &&
       (await hasSkillCapability(req, SystemCapabilities.MANAGE_SKILLS))
     ) {
-      if (hasServerCredentialReference(resolved)) {
-        return res.status(403).json({
-          message: 'Tenant-scoped skill sync runs cannot use server credentials',
-        });
-      }
-      req.skillSyncAllowServerCredentials = false;
-      return next();
+      return res.status(403).json({
+        message: 'Tenant-scoped manual skill sync requires platform credential access',
+      });
     }
     return res.status(403).json({ message: 'Forbidden' });
   } catch {

api/server/routes/admin/skills.test.js

@@ -210,6 +210,7 @@ describe('admin skills sync routes', () => {
 
     const req = mockHandlers.runSync.mock.calls[0][0];
     expect(req.skillSyncAllowServerCredentials).toBe(true);
+    expect(req.skillSyncCanReadCredentials).toBe(true);
     expect(req.config.config.skillSync).toEqual(skillSync);
   });
 });

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

@@ -239,6 +239,28 @@ describe('GitHub skill sync service', () => {
         ],
       },
     };
+    mockRunnerStatus = {
+      enabled: true,
+      intervalMinutes: 60,
+      runOnStartup: false,
+      sources: [
+        {
+          provider: 'github',
+          sourceId: 'tenant-skills',
+          status: 'idle',
+          credentialPresent: false,
+          owner: 'LibreChat',
+          repo: 'skills',
+          ref: 'main',
+          paths: ['skills'],
+          syncedSkillCount: 0,
+          syncedFileCount: 0,
+          deletedSkillCount: 0,
+          deletedFileCount: 0,
+        },
+      ],
+      credentials: [],
+    };
 
     const service = require('./sync');
     const started = await service.maybeRunGitHubSkillSyncForRequest({

packages/api/src/admin/skills.spec.ts

@@ -12,7 +12,10 @@ function createResponse() {
   };
 }
 
-function createHandlers() {
+function createHandlers({
+  statusErrorCode,
+  statusErrorMessage,
+}: { statusErrorCode?: string; statusErrorMessage?: string } = {}) {
   const runner = {
     getStatus: jest.fn(async () => ({
       enabled: true,
@@ -34,8 +37,8 @@ function createHandlers() {
           syncedFileCount: 0,
           deletedSkillCount: 0,
           deletedFileCount: 0,
-          errorCode: undefined,
-          errorMessage: undefined,
+          errorCode: statusErrorCode,
+          errorMessage: statusErrorMessage,
           startedAt: undefined,
           finishedAt: undefined,
           lastSuccessAt: undefined,
@@ -68,8 +71,8 @@ function createHandlers() {
           syncedFileCount: 2,
           deletedSkillCount: 0,
           deletedFileCount: 0,
-          errorCode: undefined,
-          errorMessage: undefined,
+          errorCode: statusErrorCode,
+          errorMessage: statusErrorMessage,
           startedAt: undefined,
           finishedAt: undefined,
           lastSuccessAt: undefined,
@@ -109,8 +112,32 @@ describe('createAdminSkillsSyncHandlers', () => {
     );
   });
 
+  it('redacts credential-related errors from tenant-scoped status reads', async () => {
+    const { handlers } = createHandlers({
+      statusErrorCode: 'MISSING_CREDENTIAL',
+      statusErrorMessage: 'Missing GitHub token environment variable "GITHUB_SKILLS_TOKEN"',
+    });
+    const res = createResponse();
+
+    await handlers.getSyncStatus({ skillSyncCanReadCredentials: false } as never, res);
+
+    expect(res.json).toHaveBeenCalledWith(
+      expect.objectContaining({
+        sources: [
+          expect.objectContaining({
+            errorCode: 'MISSING_CREDENTIAL',
+            errorMessage: 'GitHub skill sync credentials are not available',
+          }),
+        ],
+      }),
+    );
+  });
+
   it('includes credential summaries and source credential metadata for platform status reads', async () => {
-    const { handlers } = createHandlers();
+    const { handlers } = createHandlers({
+      statusErrorCode: 'MISSING_CREDENTIAL',
+      statusErrorMessage: 'Missing GitHub credential "github-skills-prod"',
+    });
     const res = createResponse();
 
     await handlers.getSyncStatus({ skillSyncCanReadCredentials: true } as never, res);
@@ -122,6 +149,7 @@ describe('createAdminSkillsSyncHandlers', () => {
           expect.objectContaining({
             credentialKey: 'github-skills-prod',
             credentialPresent: true,
+            errorMessage: 'Missing GitHub credential "github-skills-prod"',
           }),
         ],
       }),
@@ -129,10 +157,19 @@ describe('createAdminSkillsSyncHandlers', () => {
   });
 
   it('omits source credential metadata from tenant-scoped manual run responses', async () => {
-    const { handlers } = createHandlers();
+    const { handlers } = createHandlers({
+      statusErrorCode: 'MISSING_CREDENTIAL',
+      statusErrorMessage: 'Missing GitHub credential "github-skills-prod"',
+    });
     const res = createResponse();
 
-    await handlers.runSync({ skillSyncAllowServerCredentials: false } as never, res);
+    await handlers.runSync(
+      {
+        skillSyncAllowServerCredentials: true,
+        skillSyncCanReadCredentials: false,
+      } as never,
+      res,
+    );
 
     expect(res.json).toHaveBeenCalledWith(
       expect.objectContaining({
@@ -140,6 +177,7 @@ describe('createAdminSkillsSyncHandlers', () => {
           expect.objectContaining({
             credentialKey: undefined,
             credentialPresent: false,
+            errorMessage: 'GitHub skill sync credentials are not available',
           }),
         ],
       }),

packages/api/src/admin/skills.ts

@@ -52,6 +52,25 @@ function serializeCredential(
   };
 }
 
+function isCredentialError(status: ISkillSyncStatus): boolean {
+  if (status.errorCode === 'MISSING_CREDENTIAL') {
+    return true;
+  }
+  return /credential|token environment variable|server github credentials/i.test(
+    status.errorMessage ?? '',
+  );
+}
+
+function serializeErrorMessage(
+  status: ISkillSyncStatus,
+  { includeCredentialMetadata }: { includeCredentialMetadata: boolean },
+): string | undefined {
+  if (includeCredentialMetadata || !isCredentialError(status)) {
+    return status.errorMessage;
+  }
+  return 'GitHub skill sync credentials are not available';
+}
+
 function serializeSourceStatus(
   status: ISkillSyncStatus & { credentialPresent?: boolean },
   { includeCredentialMetadata = true }: { includeCredentialMetadata?: boolean } = {},
@@ -72,7 +91,7 @@ function serializeSourceStatus(
     lastSuccessAt: toIso(status.lastSuccessAt),
     lastFailureAt: toIso(status.lastFailureAt),
     errorCode: status.errorCode,
-    errorMessage: status.errorMessage,
+    errorMessage: serializeErrorMessage(status, { includeCredentialMetadata }),
     syncedSkillCount: status.syncedSkillCount,
     syncedFileCount: status.syncedFileCount,
     deletedSkillCount: status.deletedSkillCount,
@@ -116,7 +135,7 @@ export function createAdminSkillsSyncHandlers(deps: AdminSkillSyncDeps) {
   }
 
   async function runSync(req: AdminSkillsRequest, res: Response) {
-    const includeCredentialMetadata = req.skillSyncAllowServerCredentials === true;
+    const includeCredentialMetadata = req.skillSyncCanReadCredentials === true;
     const result = await getRunner(req).runOnce();
     const response: TGitHubSkillSyncManualRunResponse = {
       status: result.status,

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

@@ -8,6 +8,8 @@ import type {
   CreateSkillResult,
   ISkillSyncStatus,
   SkillSyncStatusInput,
+  UpdateSkillInput,
+  UpdateSkillResult,
 } from '@librechat/data-schemas';
 import type { GitHubSkillSyncDeps } from './github';
 import { DEFAULT_SKILL_IMPORT_LIMITS } from '../limits';
@@ -306,6 +308,75 @@ describe('createGitHubSkillSyncRunner', () => {
     );
   });
 
+  it('fails duplicate discovered 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: 'skills',
+              mode: '040000',
+              type: 'tree',
+              sha: 'skills-tree-sha',
+              url: 'https://api.github.test/tree/skills',
+            },
+          ],
+        });
+      }
+      if (url.includes('/git/trees/skills-tree-sha')) {
+        return response({
+          sha: 'skills-tree-sha',
+          truncated: false,
+          tree: [
+            {
+              path: 'research/SKILL.md',
+              mode: '100644',
+              type: 'blob',
+              sha: 'skill-a-sha',
+              size: 50,
+              url: 'https://api.github.test/blob/skill-a',
+            },
+            {
+              path: 'analysis/SKILL.md',
+              mode: '100644',
+              type: 'blob',
+              sha: 'skill-b-sha',
+              size: 50,
+              url: 'https://api.github.test/blob/skill-b',
+            },
+          ],
+        });
+      }
+      if (url.includes('/git/blobs/skill-a-sha')) {
+        return response(blob('---\nname: duplicate\ndescription: First\n---\nBody'));
+      }
+      if (url.includes('/git/blobs/skill-b-sha')) {
+        return response(blob('---\nname: duplicate\ndescription: Second\n---\nBody'));
+      }
+      return response({ message: 'not found' }, 404);
+    }) as unknown as typeof fetch;
+    const deps = createDeps({ fetchFn: duplicateFetch });
+    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) => {
@@ -721,6 +792,85 @@ describe('createGitHubSkillSyncRunner', () => {
     expect(deps.deleteSkill).toHaveBeenCalledWith(staleId.toString());
   });
 
+  it('deletes stale name-conflicting mirrors before applying same-commit renames', async () => {
+    const staleId = new Types.ObjectId();
+    const existingId = new Types.ObjectId();
+    const author = makeSourceAuthorId();
+    const existingSkill = (
+      upstreamId: string,
+      _id: Types.ObjectId,
+      name: string,
+    ): ISkill & { _id: Types.ObjectId } => {
+      const skill = makeSkill({
+        name,
+        description: `${name} skill`,
+        body: 'Old body',
+        author,
+        authorName: 'GitHub Sync',
+        source: 'github',
+        sourceMetadata: { provider: 'github', sourceId: 'librechat-skills', upstreamId },
+      });
+      skill._id = _id;
+      return skill;
+    };
+    const staleSkill = existingSkill('librechat-skills:skills/removed', staleId, 'renamed');
+    const syncedSkill = existingSkill('librechat-skills:skills/research', existingId, 'research');
+    const existingById = new Map([[existingId.toString(), syncedSkill]]);
+    const deletedIds = new Set<string>();
+    const listSkillsBySource = jest.fn(async () =>
+      [staleSkill, syncedSkill].filter((skill) => !deletedIds.has(skill._id.toString())),
+    );
+    const deleteSkill = jest.fn(async (id: string) => {
+      deletedIds.add(id);
+      return { deleted: true };
+    });
+    const updateSkill = jest.fn(
+      async ({
+        id,
+        update,
+      }: {
+        id: string;
+        expectedVersion: number;
+        update: UpdateSkillInput;
+      }): Promise<UpdateSkillResult> => {
+        if (!deletedIds.has(staleId.toString()) && update.name === 'renamed') {
+          throw new Error('duplicate key');
+        }
+        const skill = existingById.get(id);
+        if (!skill) {
+          return { status: 'not_found' as const };
+        }
+        const updated = { ...skill, ...update, version: skill.version + 1 };
+        existingById.set(id, updated);
+        return { status: 'updated' as const, skill: updated, warnings: [] };
+      },
+    );
+    const deps = createDeps({
+      fetchFn: githubFetch('---\nname: renamed\ndescription: Renamed skill\n---\nBody'),
+      findSkillBySourceIdentity: jest.fn(async ({ upstreamId }) =>
+        upstreamId === 'librechat-skills:skills/research' ? syncedSkill : null,
+      ),
+      getSkillById: jest.fn(async (id) => existingById.get(id.toString()) ?? null),
+      listSkillsBySource,
+      deleteSkill,
+      updateSkill,
+    });
+    const runner = createGitHubSkillSyncRunner(deps);
+    const result = await runner.runOnce();
+
+    expect(result.status).toBe('completed');
+    expect(deleteSkill).toHaveBeenCalledWith(staleId.toString());
+    expect(updateSkill).toHaveBeenCalledWith(
+      expect.objectContaining({
+        id: existingId.toString(),
+        update: expect.objectContaining({ name: 'renamed' }),
+      }),
+    );
+    expect(deleteSkill.mock.invocationCallOrder[0]).toBeLessThan(
+      updateSkill.mock.invocationCallOrder[0],
+    );
+  });
+
   it("does not mirror-delete another tenant's skills from an ambient source run", async () => {
     const ambientStaleId = new Types.ObjectId();
     const otherTenantId = new Types.ObjectId();

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

@@ -97,6 +97,11 @@ type PreparedExistingRemoteSkill = PreparedRemoteSkill & {
   existing: ISkill & { _id: Types.ObjectId };
 };
 
+type PreparedDiscoveredSkill = {
+  discovered: DiscoveredSkill;
+  prepared: PreparedRemoteSkill;
+};
+
 type SaveBufferResult = {
   filepath: string;
   source: string;
@@ -1057,6 +1062,96 @@ function findMovedSourceSkill(params: {
   );
 }
 
+function getMirrorNameKey(params: {
+  tenantId?: string;
+  author: string;
+  name: string | undefined;
+}): string {
+  return `${params.tenantId ?? ''}:${params.author}:${params.name ?? ''}`;
+}
+
+function assertNoDuplicatePreparedSkillNames(
+  source: SkillSyncGitHubSourceConfig,
+  preparedSkills: PreparedDiscoveredSkill[],
+): void {
+  const sourceTenantId = source.tenantId ?? undefined;
+  const seen = new Map<string, string>();
+  for (const { discovered, prepared } of preparedSkills) {
+    const key = getMirrorNameKey({
+      tenantId: sourceTenantId,
+      author: prepared.createInput.author.toString(),
+      name: prepared.createInput.name,
+    });
+    const previousRoot = seen.get(key);
+    if (previousRoot) {
+      throw new SkillSyncError(
+        'DUPLICATE_SKILL_NAME',
+        `GitHub source "${source.id}" contains multiple skills named "${prepared.createInput.name}"`,
+      );
+    }
+    seen.set(key, discovered.rootPath);
+  }
+}
+
+async function deleteNameConflictingStaleSkills(params: {
+  deps: GitHubSkillSyncDeps;
+  source: SkillSyncGitHubSourceConfig;
+  preparedSkills: PreparedDiscoveredSkill[];
+  existingSyncedSkills: Array<ISkill & { _id: Types.ObjectId }>;
+  discoveredUpstreamIds: Set<string>;
+  assertNotCancelled: AssertNotCancelled;
+}): Promise<{
+  remainingSkills: Array<ISkill & { _id: Types.ObjectId }>;
+  deletedSkillCount: number;
+  deletedFileCount: number;
+}> {
+  const sourceTenantId = params.source.tenantId ?? undefined;
+  const conflictingUpdateKeys = new Set(
+    params.preparedSkills
+      .filter(({ prepared }) => prepared.existing)
+      .map(({ prepared }) =>
+        getMirrorNameKey({
+          tenantId: sourceTenantId,
+          author: prepared.createInput.author.toString(),
+          name: prepared.update.name,
+        }),
+      ),
+  );
+  if (conflictingUpdateKeys.size === 0) {
+    return {
+      remainingSkills: params.existingSyncedSkills,
+      deletedSkillCount: 0,
+      deletedFileCount: 0,
+    };
+  }
+
+  const remainingSkills: Array<ISkill & { _id: Types.ObjectId }> = [];
+  let deletedSkillCount = 0;
+  let deletedFileCount = 0;
+  for (const skill of params.existingSyncedSkills) {
+    params.assertNotCancelled();
+    const upstreamId = getSourceMetadataString(skill, 'upstreamId');
+    const shouldDelete =
+      (skill.tenantId ?? undefined) === sourceTenantId &&
+      (!upstreamId || !params.discoveredUpstreamIds.has(upstreamId)) &&
+      conflictingUpdateKeys.has(
+        getMirrorNameKey({
+          tenantId: sourceTenantId,
+          author: skill.author.toString(),
+          name: skill.name,
+        }),
+      );
+    if (!shouldDelete) {
+      remainingSkills.push(skill);
+      continue;
+    }
+    deletedFileCount += await deleteSyncedSkill(params.deps, skill);
+    deletedSkillCount++;
+  }
+
+  return { remainingSkills, deletedSkillCount, deletedFileCount };
+}
+
 async function syncSkillFiles(params: {
   deps: GitHubSkillSyncDeps;
   token: string;
@@ -1256,6 +1351,7 @@ async function syncSource(params: {
       deletedFileCount: 0,
     };
     const syncedAt = new Date();
+    const preparedSkills: PreparedDiscoveredSkill[] = [];
 
     for (const discovered of discoveredSkills) {
       assertNotCancelled();
@@ -1277,6 +1373,27 @@ async function syncSource(params: {
         commitSha: commit.sha,
         syncedAt,
       });
+      preparedSkills.push({ discovered, prepared });
+    }
+
+    const discoveredUpstreamIds = new Set(
+      preparedSkills.map(({ discovered }) => makeUpstreamId(source, discovered.rootPath)),
+    );
+    assertNoDuplicatePreparedSkillNames(source, preparedSkills);
+    const staleConflictCleanup = await deleteNameConflictingStaleSkills({
+      deps,
+      source,
+      preparedSkills,
+      existingSyncedSkills: await getExistingSyncedSkills(),
+      discoveredUpstreamIds,
+      assertNotCancelled,
+    });
+    existingSyncedSkills = staleConflictCleanup.remainingSkills;
+    counts.deletedSkillCount += staleConflictCleanup.deletedSkillCount;
+    counts.deletedFileCount += staleConflictCleanup.deletedFileCount;
+
+    for (const { discovered, prepared } of preparedSkills) {
+      assertNotCancelled();
       const movedExisting = prepared.existing
         ? null
         : findMovedSourceSkill({