Skip to content

Commit 8145c0c

Browse files
committed
fix: address code review findings
- Fix critical bug where lastResult could be uninitialized if maxRetries is 0 - Add mapPermissions to properly convert DiscordPermission to Permission type - Remove duplicate TemplateExecutor (consolidate to templates/executor.ts) - Export mapPermissions and mapDiscordPermission from templates.ts
1 parent 73ddb9c commit 8145c0c

File tree

4 files changed

+81
-924
lines changed

4 files changed

+81
-924
lines changed

src/templates/executor.ts

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
TemplateCategory,
1313
TemplateChannel,
1414
ChannelType,
15+
DiscordPermission,
1516
} from './types.js';
1617
import { getTemplatePreview, getRawTemplate, hasTemplate } from './index.js';
1718
import {
@@ -40,7 +41,7 @@ import {
4041
TemplateError,
4142
wrapError,
4243
} from '../utils/errors.js';
43-
import type { TemplateCustomization } from '../utils/validation.js';
44+
import type { TemplateCustomization, Permission } from '../utils/validation.js';
4445

4546
// ============================================
4647
// Configuration Constants
@@ -164,6 +165,10 @@ async function executeWithRetry<T>(
164165
maxRetries: number = MAX_RETRIES,
165166
retryDelay: number = RETRY_DELAY
166167
): Promise<{ result: T; retryCount: number }> {
168+
if (maxRetries < 1) {
169+
throw new Error('maxRetries must be at least 1');
170+
}
171+
167172
let lastResult: T;
168173
let retryCount = 0;
169174

@@ -203,6 +208,69 @@ function mapChannelType(type: ChannelType): 'text' | 'voice' | 'announcement' |
203208
}
204209
}
205210

211+
/**
212+
* Map DiscordPermission enum values to Permission type values
213+
* The validation schema uses slightly different naming conventions
214+
*/
215+
function mapDiscordPermission(permission: DiscordPermission): Permission | null {
216+
const mapping: Record<DiscordPermission, Permission | null> = {
217+
[DiscordPermission.Administrator]: 'ADMINISTRATOR',
218+
[DiscordPermission.ViewChannels]: 'VIEW_CHANNEL',
219+
[DiscordPermission.ManageChannels]: 'MANAGE_CHANNELS',
220+
[DiscordPermission.ManageRoles]: 'MANAGE_ROLES',
221+
[DiscordPermission.ManageEmojis]: 'MANAGE_EXPRESSIONS',
222+
[DiscordPermission.ViewAuditLog]: 'VIEW_AUDIT_LOG',
223+
[DiscordPermission.ManageWebhooks]: 'MANAGE_WEBHOOKS',
224+
[DiscordPermission.ManageServer]: 'MANAGE_SERVER',
225+
[DiscordPermission.CreateInvite]: 'CREATE_INVITE',
226+
[DiscordPermission.ChangeNickname]: 'CHANGE_NICKNAME',
227+
[DiscordPermission.ManageNicknames]: 'MANAGE_NICKNAMES',
228+
[DiscordPermission.KickMembers]: 'KICK_MEMBERS',
229+
[DiscordPermission.BanMembers]: 'BAN_MEMBERS',
230+
[DiscordPermission.TimeoutMembers]: 'TIMEOUT_MEMBERS',
231+
[DiscordPermission.SendMessages]: 'SEND_MESSAGES',
232+
[DiscordPermission.SendMessagesInThreads]: 'SEND_MESSAGES_IN_THREADS',
233+
[DiscordPermission.CreatePublicThreads]: 'CREATE_PUBLIC_THREADS',
234+
[DiscordPermission.CreatePrivateThreads]: 'CREATE_PRIVATE_THREADS',
235+
[DiscordPermission.EmbedLinks]: 'EMBED_LINKS',
236+
[DiscordPermission.AttachFiles]: 'ATTACH_FILES',
237+
[DiscordPermission.AddReactions]: 'ADD_REACTIONS',
238+
[DiscordPermission.UseExternalEmojis]: 'USE_EXTERNAL_EMOJI',
239+
[DiscordPermission.UseExternalStickers]: 'USE_EXTERNAL_STICKERS',
240+
[DiscordPermission.MentionEveryone]: 'MENTION_EVERYONE',
241+
[DiscordPermission.ManageMessages]: 'MANAGE_MESSAGES',
242+
[DiscordPermission.ManageThreads]: 'MANAGE_THREADS',
243+
[DiscordPermission.ReadMessageHistory]: 'READ_MESSAGE_HISTORY',
244+
[DiscordPermission.SendTTSMessages]: 'SEND_TTS_MESSAGES',
245+
[DiscordPermission.UseApplicationCommands]: 'USE_APPLICATION_COMMANDS',
246+
[DiscordPermission.Connect]: 'CONNECT',
247+
[DiscordPermission.Speak]: 'SPEAK',
248+
[DiscordPermission.Video]: 'VIDEO',
249+
[DiscordPermission.UseActivities]: 'USE_EMBEDDED_ACTIVITIES',
250+
[DiscordPermission.UseSoundboard]: 'USE_SOUNDBOARD',
251+
[DiscordPermission.UseExternalSounds]: 'USE_EXTERNAL_SOUNDS',
252+
[DiscordPermission.UseVoiceActivity]: 'USE_VOICE_ACTIVITY',
253+
[DiscordPermission.PrioritySpeaker]: 'PRIORITY_SPEAKER',
254+
[DiscordPermission.MuteMembers]: 'MUTE_MEMBERS',
255+
[DiscordPermission.DeafenMembers]: 'DEAFEN_MEMBERS',
256+
[DiscordPermission.MoveMembers]: 'MOVE_MEMBERS',
257+
[DiscordPermission.CreateEvents]: null, // Not in validation schema
258+
[DiscordPermission.ManageEvents]: null, // Not in validation schema
259+
};
260+
261+
return mapping[permission] ?? null;
262+
}
263+
264+
/**
265+
* Map an array of DiscordPermission to Permission array
266+
* Filters out any permissions that don't have a mapping
267+
*/
268+
function mapPermissions(permissions: DiscordPermission[]): Permission[] {
269+
return permissions
270+
.map(mapDiscordPermission)
271+
.filter((p): p is Permission => p !== null);
272+
}
273+
206274
// ============================================
207275
// Template Executor Class
208276
// ============================================
@@ -429,7 +497,7 @@ export class TemplateExecutor {
429497
color: roleColor,
430498
hoist: role.hoist,
431499
mentionable: role.mentionable,
432-
permissions: role.permissions as string[],
500+
permissions: mapPermissions(role.permissions),
433501
},
434502
});
435503

@@ -489,7 +557,7 @@ export class TemplateExecutor {
489557
color: role.color,
490558
hoist: role.hoist ?? false,
491559
mentionable: role.mentionable ?? false,
492-
permissions: (role.permissions ?? []) as string[],
560+
permissions: role.permissions ?? [],
493561
},
494562
});
495563

src/tools/index.ts

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -152,25 +152,12 @@ export {
152152
type ChannelToolError,
153153
} from './channels.js';
154154

155-
// Template executor tools
155+
// Template executor (from templates/executor.ts)
156+
// Note: TemplateExecutor functionality is now consolidated in ../templates/executor.ts
157+
// and exposed via apply_template_full tool in templates.ts
156158
export {
157-
// Main executor class
158-
TemplateExecutor,
159-
// Handlers
160-
executeTemplateHandler,
161-
handleTemplateExecutorToolCall,
162-
// Tool definitions
163-
executeTemplateToolDefinition,
164-
templateExecutorToolDefinitions,
165-
templateExecutorToolHandlers,
166-
// Input schemas
167-
ExecuteTemplateInputSchema,
168-
// Types
169-
type ExecuteTemplateInput,
170-
type ExecuteTemplateResult,
171-
type ExecuteTemplateError,
172-
type ExecutionPhase,
173-
type ExecutionProgress,
174-
type StepResult,
175-
type ProgressCallback,
176-
} from './template-executor.js';
159+
applyTemplateFullHandler,
160+
applyTemplateFullToolDefinition,
161+
ApplyTemplateFullInputSchema,
162+
type ApplyTemplateFullInput,
163+
} from './templates.js';

0 commit comments

Comments
 (0)