diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index 6639a0c66e..e0acc27305 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -305,6 +305,76 @@ describe('createGitHubSkillSyncRunner', () => { ); }); + it('scopes mirror cleanup to the current source and deletes only its absent upstream skills', async () => { + const keptId = new Types.ObjectId(); + const staleId = new Types.ObjectId(); + const existingSkill = (upstreamId: string, _id: Types.ObjectId) => + ({ + ...makeSkill({ + name: 'research', + description: 'Research things', + author: new Types.ObjectId(), + source: 'github', + sourceMetadata: { provider: 'github', sourceId: 'librechat-skills', upstreamId }, + } as CreateSkillInput), + _id, + }) as ISkill & { _id: Types.ObjectId }; + const listSkillsBySource = jest.fn(async () => [ + existingSkill('librechat-skills:LibreChat/skills:skills/research', keptId), + existingSkill('librechat-skills:LibreChat/skills:skills/removed', staleId), + ]); + const deps = createDeps({ listSkillsBySource }); + const runner = createGitHubSkillSyncRunner(deps); + const result = await runner.runOnce(); + + expect(result.status).toBe('completed'); + expect(listSkillsBySource).toHaveBeenCalledWith({ + source: 'github', + sourceId: 'librechat-skills', + }); + expect(deps.deleteSkill).toHaveBeenCalledTimes(1); + expect(deps.deleteSkill).toHaveBeenCalledWith(staleId.toString()); + }); + + it('derives distinct synthetic authors for the same source mirrored into different tenants', async () => { + const authorForTenant = async (tenantId: string): Promise => { + let author = ''; + const deps = createDeps({ + getConfig: () => ({ + github: { + enabled: true, + intervalMinutes: 60, + runOnStartup: false, + sources: [ + { + id: 'librechat-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + credentialKey: 'github-skills-prod', + tenantId, + }, + ], + }, + }), + createSkill: jest.fn(async (input: CreateSkillInput): Promise => { + author = input.author.toString(); + return { skill: makeSkill(input), warnings: [] }; + }), + }); + await createGitHubSkillSyncRunner(deps).runOnce(); + return author; + }; + + const [authorA, authorB] = [ + await authorForTenant('tenant-a'), + await authorForTenant('tenant-b'), + ]; + expect(authorA).not.toBe(''); + expect(authorA).not.toBe(authorB); + }); + it('uses distinct synthetic authors so same-named skills can sync from different sources', async () => { const seenNamesByAuthor = new Set(); const deps = createDeps({ diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index 841b2e9dd9..8ce41eed2a 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -217,11 +217,14 @@ function makeUpstreamId(source: SkillSyncGitHubSourceConfig, rootPath: string): } function makeSourceAuthorId(source: SkillSyncGitHubSourceConfig): Types.ObjectId { - const digest = crypto - .createHash('sha256') - .update(`${PROVIDER}:${source.id}`) - .digest('hex') - .slice(0, 24); + // Fold the tenant into the synthetic author so the same source mirrored into + // different tenants gets distinct author ids (clearer audits, no cross-tenant + // author collisions). The tenant suffix is omitted when absent so single-tenant + // author ids stay stable. + const seed = source.tenantId + ? `${PROVIDER}:${source.id}:${source.tenantId}` + : `${PROVIDER}:${source.id}`; + const digest = crypto.createHash('sha256').update(seed).digest('hex').slice(0, 24); return new Types.ObjectId(digest); } @@ -1108,8 +1111,12 @@ async function syncSource(params: { /** * Runs a source sync inside its tenant's async context when `tenantId` is set, * so the tenant-isolation mongoose hooks scope every skill/file/ACL read and - * write to that tenant (required under strict isolation). Without a configured + * write to that tenant (required under strict isolation). Storage writes also + * receive the tenant explicitly via `skill.tenantId`. Without a configured * tenant the sync runs in the ambient context, preserving single-tenant behavior. + * + * The callback is `async` per the tenant-context contract so the ALS store + * propagates across every awaited Mongoose operation in `syncSource`. */ function syncSourceInTenantContext(params: { deps: GitHubSkillSyncDeps; @@ -1120,7 +1127,7 @@ function syncSourceInTenantContext(params: { if (!params.source.tenantId) { return syncSource(params); } - return tenantStorage.run({ tenantId: params.source.tenantId }, () => syncSource(params)); + return tenantStorage.run({ tenantId: params.source.tenantId }, async () => syncSource(params)); } function getGithubConfig(config: SkillSyncConfig | undefined): {