fix: Resolve global access role outside tenant context for synced skill grants

Addresses Codex P2 (github.ts:1166): default access roles (incl. skill_viewer) are seeded globally with no tenantId under runAsSystem, but a tenant-scoped sync wraps ensurePublicViewer in the source's tenant context. The PermissionService grantPermission resolved the role via a tenant-isolated AccessRole query, so the global role did not match and tenant-scoped syncs failed with 'Role skill_viewer not found'. The sync adapter now resolves the role inside runAsSystem (matching the global seed) and writes the ACL entry in the active tenant context, so the AclEntry is tenant-scoped (visible to tenant users) while the role lookup still succeeds. Covered by service tests for the resolve-vs-write split and the missing-role failure.

api/server/services/Skills/sync.js

@@ -4,9 +4,9 @@ const {
   createGitHubSkillSyncRunner,
   startGitHubSkillSyncScheduler,
 } = require('@librechat/api');
+const { runAsSystem } = require('@librechat/data-schemas');
 const db = require('~/models');
 const { getAppConfig } = require('~/server/services/Config');
-const { grantPermission } = require('~/server/services/PermissionService');
 const { getStrategyFunctions } = require('~/server/services/Files/strategies');
 const { getFileStrategy } = require('~/server/utils/getFileStrategy');
 
@@ -77,7 +77,38 @@ function createRunner() {
     upsertSkillFile: db.upsertSkillFile,
     deleteSkillFile: db.deleteSkillFile,
     deleteSkill: db.deleteSkill,
-    grantPermission,
+    grantPermission: async ({
+      principalType,
+      principalId,
+      resourceType,
+      resourceId,
+      accessRoleId,
+      grantedBy,
+    }) => {
+      // Default access roles are seeded globally (no tenantId) under runAsSystem,
+      // but the runner may execute inside a source's tenant context. Resolve the
+      // role outside tenant isolation so the global role matches, then write the
+      // ACL entry in the active (tenant) context so tenant users can see it.
+      const role = await runAsSystem(() => db.findRoleByIdentifier(accessRoleId));
+      if (!role) {
+        throw new Error(`Role ${accessRoleId} not found`);
+      }
+      if (role.resourceType !== resourceType) {
+        throw new Error(
+          `Role ${accessRoleId} is for ${role.resourceType} resources, not ${resourceType}`,
+        );
+      }
+      return db.grantPermission(
+        principalType,
+        principalId,
+        resourceType,
+        resourceId,
+        role.permBits,
+        grantedBy,
+        undefined,
+        role._id,
+      );
+    },
     saveBuffer: async ({ userId, buffer, fileName, basePath, isImage, tenantId }) => {
       const storage = await resolveSkillStorage({ isImage });
       const filepath = await storage.saveBuffer({

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

@@ -1,6 +1,8 @@
 const mockGetAppConfig = jest.fn();
 const mockGetStrategyFunctions = jest.fn();
 const mockGetFileStrategy = jest.fn();
+const mockFindRoleByIdentifier = jest.fn();
+const mockGrantPermission = jest.fn();
 let mockRunnerDeps;
 
 jest.mock('~/server/services/Config', () => ({
@@ -23,8 +25,10 @@ jest.mock('@librechat/data-schemas', () => ({
   runAsSystem: jest.fn((fn) => fn()),
 }));
 
-jest.mock('~/models', () => ({}));
-jest.mock('~/server/services/PermissionService', () => ({ grantPermission: jest.fn() }));
+jest.mock('~/models', () => ({
+  findRoleByIdentifier: mockFindRoleByIdentifier,
+  grantPermission: mockGrantPermission,
+}));
 jest.mock('~/server/services/Files/strategies', () => ({
   getStrategyFunctions: mockGetStrategyFunctions,
 }));
@@ -36,6 +40,8 @@ describe('GitHub skill sync service', () => {
     mockGetAppConfig.mockReset();
     mockGetStrategyFunctions.mockReset();
     mockGetFileStrategy.mockReset();
+    mockFindRoleByIdentifier.mockReset();
+    mockGrantPermission.mockReset();
     mockRunnerDeps = undefined;
   });
 
@@ -133,4 +139,59 @@ describe('GitHub skill sync service', () => {
 
     expect(runAsSystem).not.toHaveBeenCalled();
   });
+
+  it('resolves the access role outside tenant isolation but writes the ACL in context', async () => {
+    const { runAsSystem } = require('@librechat/data-schemas');
+    mockGetAppConfig.mockResolvedValue({ skillSync: undefined, paths: {} });
+    mockFindRoleByIdentifier.mockResolvedValue({
+      _id: 'role-object-id',
+      resourceType: 'skill',
+      permBits: 1,
+    });
+    mockGrantPermission.mockResolvedValue({ _id: 'acl-entry-id' });
+
+    const service = require('./sync');
+    service.initializeGitHubSkillSync({ skillSync: undefined });
+    await mockRunnerDeps.grantPermission({
+      principalType: 'public',
+      principalId: null,
+      resourceType: 'skill',
+      resourceId: 'skill-id',
+      accessRoleId: 'skill_viewer',
+      grantedBy: 'system',
+    });
+
+    expect(runAsSystem).toHaveBeenCalledTimes(1);
+    expect(mockFindRoleByIdentifier).toHaveBeenCalledWith('skill_viewer');
+    expect(mockGrantPermission).toHaveBeenCalledWith(
+      'public',
+      null,
+      'skill',
+      'skill-id',
+      1,
+      'system',
+      undefined,
+      'role-object-id',
+    );
+  });
+
+  it('fails the grant when the access role does not exist', async () => {
+    mockGetAppConfig.mockResolvedValue({ skillSync: undefined, paths: {} });
+    mockFindRoleByIdentifier.mockResolvedValue(null);
+
+    const service = require('./sync');
+    service.initializeGitHubSkillSync({ skillSync: undefined });
+
+    await expect(
+      mockRunnerDeps.grantPermission({
+        principalType: 'public',
+        principalId: null,
+        resourceType: 'skill',
+        resourceId: 'skill-id',
+        accessRoleId: 'skill_viewer',
+        grantedBy: 'system',
+      }),
+    ).rejects.toThrow('Role skill_viewer not found');
+    expect(mockGrantPermission).not.toHaveBeenCalled();
+  });
 });