Conversation
- Add missing REST endpoints: Set Voice Channel Status, Get Guild Role Member Counts, Invite Target Users (get/update/job status) - Add missing gateway events: VoiceChannelStatusUpdate, VoiceChannelStartTimeUpdate - Update model fields across Message, Guild, GuildMember, User, Invite, Attachment, Reaction, MessageReference, PartialAttachment - Add new structs: MessageSnapshot, MessageCall, SharedClientTheme, ReactionCountDetails, IncidentsData, WelcomeScreen, AvatarDecorationData, Collectibles, InviteTargetUsersJobStatus - Add attachment editing support (description, is_spoiler) via PartialAttachment and editMessage attachments param - Update PermissionBitset with setVoiceChannelStatus - Update Gateway and AuditLog models with voice status fields - Add GUILD_MEDIA channel type to documentation - Add putMultipart and putFile helpers to HTTPClient - Humanize README: restructure, replace em dashes, highlight SwiftDisc differentiators (actor safety, typed models, gateway lifecycle, DX)
… ApplicationCommandCreate - Convert SlashCommandBuilder from class @unchecked Sendable to struct w/ value semantics - Add type(.user)/type(.message) support for context menu commands - Add nsfw, contexts, nameLocalizations, descriptionLocalizations to builder - Add subCommand/subCommandGroup option builders, integer/number choices - Update ApplicationCommandCreate with type, nsfw, contexts fields - Add UserSelectMenuBuilder, RoleSelectMenuBuilder, MentionableSelectMenuBuilder to ComponentsBuilder (with minValues/maxValues support) - Rewrite ShardingBot example to use ShardingGatewayManager actor API
…x Voice Channel Status events - Add getGuildWelcomeScreen / modifyGuildWelcomeScreen REST endpoints - Add 20+ missing gateway event callbacks (onGuildEmojisUpdate, onGuildStickersUpdate, onWebhooksUpdate, onIntegrationsUpdate, onInviteCreate/Delete, onAuditLogEntryCreate, onAutoModRuleCRUD, onUserUpdate, onChannelPinsUpdate, onThreadMemberUpdate, onThreadListSync, onGuildMembersChunk, onScheduledEventUserAdd/Remove, onVoiceChannelStatusUpdate, onVoiceChannelStartTimeUpdate, onChannelInfo) - Fix voiceChannelStatusUpdate and voiceChannelStartTimeUpdate being silently dropped in EventDispatcher (no switch case existed) - Remove all remaining break-only cases in EventDispatcher; every DiscordEvent now invokes its callback - Update GAP_ANALYSIS.md to reflect current state - .gitignore: exclude GAP_ANALYSIS.md from commits
…come, MFA, Incidents Typed enums added: - ChannelType (replaces raw Int on Channel.type) - MessageType (replaces raw Int? on Message.type) - MessageFlags OptionSet (replaces raw Int? on Message.flags) - InteractionType (replaces raw Int on Interaction.type) - GuildFeature (replaces [String] on Guild.features) - GuildMemberFlags OptionSet (replaces Int? on GuildMember.flags) - ChannelFlags OptionSet (replaces Int? on Channel.flags) - AuditLogEventType (replaces raw Int on AuditLogEntry.action_type) - EventPrivacyLevel (replaces raw Int on GuildScheduledEvent.privacy_level and StageInstance.privacy_level) - AutoModerationRule.EventType, TriggerType, ActionType (replaces raw Int) - Application model (new file with full fields) - NewMemberWelcome model + GET/PATCH endpoints - MFALevel model + GET/POST endpoints - modifyGuildIncidentActions PUT endpoint All call sites updated: EventDispatcher, DiscordClient, MessagePayload, example bots, and tests.
…OG and GAP_ANALYSIS
… GuildMemberFlags, MessageFlags OptionSet types
… returns, maxUploadBytes guard, ShardingGatewayManager race condition, README wording
- Rename Application phantom tag to ApplicationTag (Snowflake.swift) - Fix GuildFeature internal case syntax (Guild.swift) - Map [String] features to [GuildFeature] in Guild convenience init - Fix JSONValue.double -> .number in SlashCommandBuilder - Fix FileAttachment argument order in HTTPClient.putFile - Use ChannelType.text enum instead of raw 0 in Cache - Replace MessageFlags | with .union() in MessagePayload - Fix DefaultValue type in MentionableSelectMenuBuilder - Refactor AsyncStream init to avoid self-capture in ShardingGatewayManager - Update ShardingBot example for changed init signature and actor isolation
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR updates gateway event coverage and dispatch, expands message payload handling, tightens several model types, and changes supporting collector, cache, REST, retry, and documentation workflows. ChangesRelease docs and workflow updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 333-334: The DocC command is missing the required directory
argument for --allow-writing-to-directory. Update the README’s documentation
generation command to pass the same output directory used by --output-path, so
the Swift package invocation in the generate-documentation example explicitly
allows writing to docs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
This audit-driven release fixes 20 files with 434 insertions and 204 deletions, addressing critical concurrency crashes, task leaks, data loss bugs, and missing functionality identified in a comprehensive package audit. Concurrency & crashes: - GatewayClient: connectReadyContinuation double-resume guard, heartbeat ACK counter single-source, socket re-check after rate-limiter wait, decode error exponential backoff with max-5-consecutive reconnect trigger - AsyncSemaphore: cancellation handling with waiter removal - close() promoted to public for API symmetry with disconnect() Task leaks: - Collectors: 11 typed event streams unified into filteredEventStream<T> with onTermination cleanup; createMessageCollector stores timeoutTask handle - ComponentCollector: timeoutTask stored and cancelled on finish - Cache: eviction task assigned inline in init instead of unstructured Task Data loss (MessagePayload field forwarding): - send(to:_:) now forwards allowedMentions, messageReference in multipart path - edit(channelId:messageId:_:) forwards ALL fields in both paths - respond(to:with:deferred:) includes message_reference, sticker_ids, poll - editMessage/editMessageWithFiles: added allowedMentions, flags, tts, stickerIds, poll parameters with nil defaults Rate limiter: - Timestamp appended after backoff (was counting waited requests against budget) - Dictionary(uniqueKeysWithValues:) replaced with safe reduce(into:) - clearBucket now also clears route-to-bucket mapping Cache additions: - getMessages(channelId:), clear(), removeUser(id:), removeMessagesForChannel(channelId:), ensureChannelStub(id:type:) Model types: - OptionalField: Decodable conformance, Codable constraint - JSONValue: UInt64 decode before Double with Int(exactly:) for precision - RetryPolicy: jitter (default ±10%), noRetry static policy - TokenStorage.rawValue: public → internal for token leak protection Developer ergonomics: - @discardableResult on editMessage, createGuildEmoji, modifyGuildEmoji, crosspostMessage - EmbedBuilder.success/error/info() factory methods - Converters: cached ISO8601DateFormatter, discordOrange corrected to 0xFEE75C, discordFuchsia added, invite code 6-25 chars, mention regexes 17-19 digits - stripMentions: all patterns updated to 17-19 digit range - GatewayClient: validatePrivilegedIntents renamed to logPrivilegedIntentWarnings - Cache.Configuration.init: multi-line formatting Infrastructure: - HTTPClient fallback stub: added putMultipart, postStickerMultipart, putFile - allHeaderFields: safe reduce(into:) instead of uniqueKeysWithValues - Utilities: mention regexes tightened from [0-9]{5,} to [0-9]{17,19} - CooldownManager: auto-cleanup starts in init(), [weak self] capture, null-byte key separator instead of double-colon - CI: migrated from compnerd/gha-setup-swift to swift-actions/setup-swift - Docs CI: migrated to swift-actions/setup-swift for Swift 6.2 Voice: No voice support. All voice-adjacent model fields (bitrate, permission flags, audit log events, etc.) are retained solely for Discord API compliance.
New gateway events (10 added): - STAGE_INSTANCE_CREATE/UPDATE/DELETE with StageInstance model - SUBSCRIPTION_CREATE/UPDATE/DELETE with AppSubscription model - SUBSCRIPTION_GROUP_SUBSCRIPTION_CREATE/UPDATE/DELETE - GUILD_JOIN_REQUEST_UPDATE with GuildJoinRequestUpdate model Typed enums with unknown fallback (7 added): - WebhookType, StickerType, StickerFormatType, InviteType, EntitlementType, ActivityType for PresenceUpdatePayload - Invite.InviteChannel.type migrated to ChannelType? Type safety fixes: - Interaction.version: Int? → Int (default 1) - User.username: String → String? (Discord returns null for deleted users) - All new enums use custom init(from:) with unknown fallback Test fixtures updated for new types.
- MockHTTPTransport & MockWebSocketTransport actors for deterministic testing - Expanded TestFixtures: Channel, GuildMember, Embed, Thread, Components, PresenceUpdate - Expanded CacheTests: user/channel/guild/role CRUD, messages, clear, removeUser - New HTTPClientTests: GET, rate limit headers, 429 retry, route key parsing - New RateLimiterTests: 50/s global limit, bucket tracking, reset/clear - HTTPClient.makeRouteKey promoted to internal for test access
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SwiftDisc/Internal/Cache.swift (1)
154-163: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEviction task forms a retain cycle with
Cache
Cache.init(configuration:)stores aTaskthat capturesselfstrongly, andselfstores the task. That cycle preventsdeinitfrom running, soevictionTask?.cancel()never fires and the TTL loop stays alive. Captureselfweakly or move the task lifecycle out of the actor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/Internal/Cache.swift` around lines 154 - 163, The eviction task created in Cache.init(configuration:) is retaining the Cache actor because the Task closure captures self strongly while evictionTask is stored on self, creating a cycle that prevents deinit from running. Update the task setup in Cache.init(configuration:) and evictionLoop() so the task does not strongly retain Cache, either by capturing self weakly inside the Task or by moving task ownership/lifecycle outside the actor. Keep deinit’s evictionTask?.cancel() as the shutdown path once the cycle is removed.
🧹 Nitpick comments (5)
Sources/SwiftDisc/DiscordClient.swift (1)
1014-1049: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicate payload-encoding logic between
editMessageWithFilesandeditMessage.Both functions define nearly identical nested
Payload/Bodystructs with the same fields, customencode(to:), andCodingKeys. Consider extracting a sharedEncodabletype (e.g., a privateMessageEditPayload) to avoid drift between the two copies as fields are added in the future.Also applies to: 2576-2606
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/DiscordClient.swift` around lines 1014 - 1049, The `editMessage` and `editMessageWithFiles` paths are duplicating the same nested payload encoding logic, which will drift as fields change. Extract the shared `Payload`/`Body` shape into a single private `Encodable` type (for example, a `MessageEditPayload`) and reuse it from both `DiscordClient` methods. Keep the existing fields, custom `encode(to:)`, and `CodingKeys` in one place so both code paths stay consistent.Sources/SwiftDisc/HighLevel/Collectors.swift (3)
13-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMain collector task can still leak if the consumer terminates the stream early.
Unlike
filteredEventStream(Line 98), this stream never setscontinuation.onTermination. If the caller breaks out of iteration beforemaxMessages/timeoutfires (and especially when notimeoutis supplied), the innertaskkeeps consumingself.eventsindefinitely. Since this PR targets collector task leaks, consider mirroring theonTerminationcancellation here.♻️ Suggested cancellation on termination
if let t = timeout { timeoutTask = Task { try? await Task.sleep(nanoseconds: UInt64(t * 1_000_000_000)) continuation.finish() task.cancel() } } + continuation.onTermination = { _ in + task.cancel() + timeoutTask?.cancel() + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/HighLevel/Collectors.swift` around lines 13 - 44, The collector stream in AsyncStream can leak its inner Task when the consumer stops iterating early, because it currently lacks termination cleanup. In the collector setup that creates task and timeoutTask, add continuation.onTermination to cancel the main task and any timeout task, mirroring the cancellation pattern used by filteredEventStream, so self.events is no longer consumed indefinitely after early stream exit.
51-73: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
defer { task = nil }is a no-op and the paging task still leaks.Clearing the captured local
taskon completion has no observable effect, and there is still nocontinuation.onTerminationto cancel the paging loop. If the consumer stops iterating,listGuildMemberspaging continues in the background. Prefer cancelling the task on termination, as done infilteredEventStream.♻️ Suggested fix
AsyncStream(GuildMember.self) { continuation in - var task: Task<Void, Never>? - task = Task { - defer { task = nil } + let task = Task { var after: UserID? = nil ... continuation.finish() } + continuation.onTermination = { _ in task.cancel() } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/HighLevel/Collectors.swift` around lines 51 - 73, The paging task in `Collectors.swift` is not cancelled when the consumer stops iterating, and `defer { task = nil }` inside the `Task` body does not prevent the background loop from continuing. Update the stream setup around the `task`/`continuation` logic to add a `continuation.onTermination` handler that cancels the paging `Task`, mirroring the cancellation pattern used by `filteredEventStream`. Keep the existing paging loop and error handling in `listGuildMembers`, but ensure termination stops further requests immediately.
102-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
onErrorparameters are now silently ignored.Every typed event-stream wrapper still accepts
onError, butfilteredEventStreamneither takes nor invokes it. Callers passing an error handler will get no callbacks, which is misleading. Either drop the parameter from these signatures or thread it throughfilteredEventStream.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/HighLevel/Collectors.swift` around lines 102 - 180, The typed event-stream wrappers in Collectors.swift still expose onError but never use it, so callers cannot receive failures. Update the collector methods like messageEvents, reactionAddEvents, interactionEvents, and the new thread/role/emoji/typing/message wrappers to either remove onError from their signatures or pass it through to filteredEventStream. If you keep the parameter, extend filteredEventStream to accept and invoke the provided error handler consistently.Sources/SwiftDisc/HighLevel/ComponentCollector.swift (1)
7-38: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing
onTerminationcancellation — same leak pattern ascreateMessageCollector.The timeout-task cancellation is fixed, but if the consumer terminates the stream before
max/timeout, the maintaskkeeps consumingself.events. Addcontinuation.onTermination = { _ in task.cancel(); timeoutTask?.cancel() }to fully close the leak.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftDisc/HighLevel/ComponentCollector.swift` around lines 7 - 38, The AsyncStream in ComponentCollector is still leaking work when the consumer stops early because the main task keeps iterating self.events; add an onTermination handler on the continuation so it cancels both the collecting Task and any timeoutTask. Update the stream setup in ComponentCollector’s AsyncStream block to wire continuation.onTermination to task.cancel() and timeoutTask?.cancel(), matching the existing cancellation flow used for max and timeout completion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SwiftDisc/Gateway/GatewayClient.swift`:
- Around line 596-598: The `logPrivilegedIntentWarnings` API is marked `public`
even though `GatewayClient` is still internal, so the visibility is
inconsistent. Remove the `public` modifier from this method and any similar
members on `GatewayClient`, unless you intend to expose the actor externally; if
so, make `GatewayClient` public as well and keep the access levels aligned.
In `@Sources/SwiftDisc/HighLevel/Converters.swift`:
- Around line 115-118: The `discordOrange` constant in `Converters.swift`
currently duplicates `discordYellow` by using the same hex value, so update the
`discordOrange` declaration in the `Converters` color constants to use the
correct orange Discord color instead of `0xFEE75C`. Keep `discordFuchsia`
unchanged and ensure the `discordOrange` symbol remains a distinct value from
`discordYellow`.
In `@Sources/SwiftDisc/HighLevel/CooldownManager.swift`:
- Around line 87-89: The delimiter used by CooldownManager.compoundKey has
changed to a NUL separator, but clearCommandCooldowns still filters with the old
"::" prefix, so it no longer matches any stored entries. Update the prefix
construction in clearCommandCooldowns to use the same "\0" separator as
compoundKey, keeping the key format consistent so command cooldowns can be
cleared correctly.
In `@Sources/SwiftDisc/HighLevel/EmbedBuilder.swift`:
- Around line 140-152: The doc comments for EmbedBuilder.success(_:) and
EmbedBuilder.error(_:) describe checkmark/crossmark prefixes that are not
actually applied. Update the documentation on these two factory methods to match
the current EmbedBuilder().color(...).description(message) behavior, or, if you
intend to keep the docs as-is, change the implementations so the returned
description includes the stated prefix consistently.
In `@Sources/SwiftDisc/Internal/JSONValue.swift`:
- Around line 28-39: JSONValue currently defines a description property but does
not actually conform to CustomStringConvertible, so String(describing:) and
interpolation still use reflection for .object and .array. Update the JSONValue
type declaration itself to adopt CustomStringConvertible, keeping the existing
description implementation as the protocol requirement, so all cases render
consistently through description-based formatting.
In `@Sources/SwiftDisc/REST/HTTPClient.swift`:
- Around line 656-662: The unsupported-platform stubs in HTTPClient should match
the concrete API exactly so call sites compile on all platforms. Update the
`#else` implementations of postStickerMultipart and putFile to use the same
parameter lists and generic shape as the real HTTPClient methods (including the
real names like name/description/tags/file and data/filename), while keeping the
same throwing behavior. Use the existing concrete declarations in HTTPClient as
the source of truth and align the stub signatures to those symbols.
- Around line 57-71: The cancellation path in HTTPClient is removing the wrong
continuation because removeCancelledWaiter() blindly drops the first entry from
waiters, which can corrupt the semaphore queue. Update the waiter management in
HTTPClient so each withCheckedContinuation registration is associated with a
unique token or identifier, then use that token in the
withTaskCancellationHandler onCancel path to find and remove/resume the matching
continuation only. Keep the queue consistent by avoiding any non-matching
removal logic in removeCancelledWaiter().
---
Outside diff comments:
In `@Sources/SwiftDisc/Internal/Cache.swift`:
- Around line 154-163: The eviction task created in Cache.init(configuration:)
is retaining the Cache actor because the Task closure captures self strongly
while evictionTask is stored on self, creating a cycle that prevents deinit from
running. Update the task setup in Cache.init(configuration:) and evictionLoop()
so the task does not strongly retain Cache, either by capturing self weakly
inside the Task or by moving task ownership/lifecycle outside the actor. Keep
deinit’s evictionTask?.cancel() as the shutdown path once the cycle is removed.
---
Nitpick comments:
In `@Sources/SwiftDisc/DiscordClient.swift`:
- Around line 1014-1049: The `editMessage` and `editMessageWithFiles` paths are
duplicating the same nested payload encoding logic, which will drift as fields
change. Extract the shared `Payload`/`Body` shape into a single private
`Encodable` type (for example, a `MessageEditPayload`) and reuse it from both
`DiscordClient` methods. Keep the existing fields, custom `encode(to:)`, and
`CodingKeys` in one place so both code paths stay consistent.
In `@Sources/SwiftDisc/HighLevel/Collectors.swift`:
- Around line 13-44: The collector stream in AsyncStream can leak its inner Task
when the consumer stops iterating early, because it currently lacks termination
cleanup. In the collector setup that creates task and timeoutTask, add
continuation.onTermination to cancel the main task and any timeout task,
mirroring the cancellation pattern used by filteredEventStream, so self.events
is no longer consumed indefinitely after early stream exit.
- Around line 51-73: The paging task in `Collectors.swift` is not cancelled when
the consumer stops iterating, and `defer { task = nil }` inside the `Task` body
does not prevent the background loop from continuing. Update the stream setup
around the `task`/`continuation` logic to add a `continuation.onTermination`
handler that cancels the paging `Task`, mirroring the cancellation pattern used
by `filteredEventStream`. Keep the existing paging loop and error handling in
`listGuildMembers`, but ensure termination stops further requests immediately.
- Around line 102-180: The typed event-stream wrappers in Collectors.swift still
expose onError but never use it, so callers cannot receive failures. Update the
collector methods like messageEvents, reactionAddEvents, interactionEvents, and
the new thread/role/emoji/typing/message wrappers to either remove onError from
their signatures or pass it through to filteredEventStream. If you keep the
parameter, extend filteredEventStream to accept and invoke the provided error
handler consistently.
In `@Sources/SwiftDisc/HighLevel/ComponentCollector.swift`:
- Around line 7-38: The AsyncStream in ComponentCollector is still leaking work
when the consumer stops early because the main task keeps iterating self.events;
add an onTermination handler on the continuation so it cancels both the
collecting Task and any timeoutTask. Update the stream setup in
ComponentCollector’s AsyncStream block to wire continuation.onTermination to
task.cancel() and timeoutTask?.cancel(), matching the existing cancellation flow
used for max and timeout completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac26927c-365f-4b2a-84c6-26a3ee482e23
📒 Files selected for processing (31)
.github/workflows/ci.yml.github/workflows/docs.yml.gitignoreCHANGELOG.mdREADME.mdSources/SwiftDisc/DiscordClient.swiftSources/SwiftDisc/Gateway/GatewayClient.swiftSources/SwiftDisc/Gateway/GatewayModels.swiftSources/SwiftDisc/HighLevel/Collectors.swiftSources/SwiftDisc/HighLevel/ComponentCollector.swiftSources/SwiftDisc/HighLevel/Converters.swiftSources/SwiftDisc/HighLevel/CooldownManager.swiftSources/SwiftDisc/HighLevel/EmbedBuilder.swiftSources/SwiftDisc/HighLevel/MessagePayload.swiftSources/SwiftDisc/HighLevel/Utilities.swiftSources/SwiftDisc/Internal/Cache.swiftSources/SwiftDisc/Internal/EventDispatcher.swiftSources/SwiftDisc/Internal/JSONValue.swiftSources/SwiftDisc/Internal/OptionalField.swiftSources/SwiftDisc/Internal/RetryPolicy.swiftSources/SwiftDisc/Internal/TokenStorage.swiftSources/SwiftDisc/Models/Interaction.swiftSources/SwiftDisc/Models/Invite.swiftSources/SwiftDisc/Models/Monetization.swiftSources/SwiftDisc/Models/Sticker.swiftSources/SwiftDisc/Models/User.swiftSources/SwiftDisc/Models/Webhook.swiftSources/SwiftDisc/REST/HTTPClient.swiftSources/SwiftDisc/REST/RateLimiter.swiftTests/SwiftDiscTests/SlashCommandRouterTests.swiftTests/SwiftDiscTests/TestFixtures.swift
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- CHANGELOG.md
- README.md
Comment on lines
+115
to
+118
| /// Discord fuchsia/pink color (used for stage channels) | ||
| public static let discordFuchsia: Int = 0xEB459E | ||
| /// Discord orange color | ||
| public static let discordOrange: Int = 0xEB459E | ||
| public static let discordOrange: Int = 0xFEE75C |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
discordOrange now duplicates discordYellow (both 0xFEE75C).
The fuchsia value was correctly extracted into discordFuchsia, but discordOrange was assigned yellow's hex rather than an actual orange, so discordOrange == discordYellow. This makes the "orange" constant misleading.
🐛 Suggested fix
/// Discord orange color
- public static let discordOrange: Int = 0xFEE75C
+ public static let discordOrange: Int = 0xE67E22📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Discord fuchsia/pink color (used for stage channels) | |
| public static let discordFuchsia: Int = 0xEB459E | |
| /// Discord orange color | |
| public static let discordOrange: Int = 0xEB459E | |
| public static let discordOrange: Int = 0xFEE75C | |
| /// Discord fuchsia/pink color (used for stage channels) | |
| public static let discordFuchsia: Int = 0xEB459E | |
| /// Discord orange color | |
| public static let discordOrange: Int = 0xE67E22 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SwiftDisc/HighLevel/Converters.swift` around lines 115 - 118, The
`discordOrange` constant in `Converters.swift` currently duplicates
`discordYellow` by using the same hex value, so update the `discordOrange`
declaration in the `Converters` color constants to use the correct orange
Discord color instead of `0xFEE75C`. Keep `discordFuchsia` unchanged and ensure
the `discordOrange` symbol remains a distinct value from `discordYellow`.
1. GatewayClient: remove misleading public on logPrivilegedIntentWarnings (actor is internal, so public was a no-op) 2. Converters: remove discordOrange (duplicated discordYellow at 0xFEE75C; Discord has no official orange in its palette) 3. CooldownManager: fix clearCommandCooldowns prefix from :: to \0 (compoundKey was changed but clear used old separator - critical bug) 4. EmbedBuilder: align doc comments with behavior (remove prefix claims) 5. JSONValue: conform to CustomStringConvertible for proper description 6. AsyncSemaphore: track waiters by ID to fix cancellation correctness 7. HTTPClient fallback stubs: align putFile and postStickerMultipart signatures with real implementations
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SwiftDisc/REST/HTTPClient.swift`:
- Around line 66-74: The cancellation path in HTTPClient.wait() leaves the
CheckedContinuation suspended because removeCancelledWaiter(_:) only removes
bookkeeping and never resumes it. Update wait() to use
withCheckedThrowingContinuation so the cancellation handler can resume the
matching continuation with CancellationError, and make executeWithRetry respect
that by skipping the defer { signal() } path when wait() throws. Use the
existing waiter tracking symbols waiters, waiterOrder, and
removeCancelledWaiter(_:) to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91fb0e8a-f9f9-49d5-bce4-5ec02301a0f5
📒 Files selected for processing (11)
Sources/SwiftDisc/Gateway/GatewayClient.swiftSources/SwiftDisc/HighLevel/Converters.swiftSources/SwiftDisc/HighLevel/CooldownManager.swiftSources/SwiftDisc/HighLevel/EmbedBuilder.swiftSources/SwiftDisc/Internal/JSONValue.swiftSources/SwiftDisc/REST/HTTPClient.swiftTests/SwiftDiscTests/CacheTests.swiftTests/SwiftDiscTests/HTTPClientTests.swiftTests/SwiftDiscTests/MockTransport.swiftTests/SwiftDiscTests/RateLimiterTests.swiftTests/SwiftDiscTests/TestFixtures.swift
💤 Files with no reviewable changes (1)
- Sources/SwiftDisc/HighLevel/Converters.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- Sources/SwiftDisc/Internal/JSONValue.swift
- Sources/SwiftDisc/HighLevel/EmbedBuilder.swift
- Sources/SwiftDisc/HighLevel/CooldownManager.swift
- Sources/SwiftDisc/Gateway/GatewayClient.swift
Comment on lines
+66
to
+74
| } onCancel: { | ||
| Task { await self.removeCancelledWaiter(id) } | ||
| } | ||
| } | ||
|
|
||
| private func removeCancelledWaiter(_ id: Int) { | ||
| waiters.removeValue(forKey: id) | ||
| waiterOrder.removeAll { $0 == id } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Cancelled continuations are never resumed — tasks hang forever.
removeCancelledWaiter removes the CheckedContinuation from waiters and waiterOrder but never calls resume() on it. The withCheckedContinuation body at line 62 will suspend indefinitely — the task can never return. CheckedContinuation will also emit a leak warning in debug builds.
The past review flagged that removeCancelledWaiter() could remove the wrong continuation; the new ID-based tracking fixes the targeting but the continuation is still leaked. The fix requires switching to withCheckedThrowingContinuation so cancellation can resume the continuation with CancellationError, making wait() throw, and updating the call site so the defer { signal() } is skipped when wait() throws (preserving correct permit accounting).
🔧 Proposed fix
private actor AsyncSemaphore {
private var value: Int
private var nextId: Int = 0
- private var waiters: [Int: CheckedContinuation<Void, Never>] = [:]
+ private var waiters: [Int: CheckedContinuation<Void, any Error>] = [:]
private var waiterOrder: [Int] = []
init(value: Int) {
self.value = value
}
- func wait() async {
+ func wait() async throws {
if value > 0 {
value -= 1
return
}
let id = nextId
nextId += 1
await withTaskCancellationHandler {
- await withCheckedContinuation { continuation in
+ try await withCheckedThrowingContinuation { continuation in
waiters[id] = continuation
waiterOrder.append(id)
}
} onCancel: {
Task { await self.removeCancelledWaiter(id) }
}
}
private func removeCancelledWaiter(_ id: Int) {
- waiters.removeValue(forKey: id)
+ if let waiter = waiters.removeValue(forKey: id) {
+ waiter.resume(throwing: CancellationError())
+ }
waiterOrder.removeAll { $0 == id }
}
func signal() {
while let id = waiterOrder.first {
waiterOrder.removeFirst()
if let waiter = waiters.removeValue(forKey: id) {
- waiter.resume()
+ waiter.resume()
return
}
}
value += 1
}
}And update the call site in executeWithRetry so a cancelled wait() skips defer { signal() }:
let semaphore = await bucketSemaphores.semaphore(for: routeKey)
- await semaphore.wait()
+ do {
+ try await semaphore.wait()
+ } catch is CancellationError {
+ throw DiscordError.cancelled
+ }
defer { Task { await semaphore.signal() } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SwiftDisc/REST/HTTPClient.swift` around lines 66 - 74, The
cancellation path in HTTPClient.wait() leaves the CheckedContinuation suspended
because removeCancelledWaiter(_:) only removes bookkeeping and never resumes it.
Update wait() to use withCheckedThrowingContinuation so the cancellation handler
can resume the matching continuation with CancellationError, and make
executeWithRetry respect that by skipping the defer { signal() } path when
wait() throws. Use the existing waiter tracking symbols waiters, waiterOrder,
and removeCancelledWaiter(_:) to locate the fix.
CI: revert workflow files to original compnerd/gha-setup-swift@main (swift-actions/setup-swift@v2 could not resolve Swift 6.2) Build fixes: - ActivityBuilder: stored type as ActivityType instead of Int - DiscordClient.setActivity: Int->ActivityType conversion - ShardingBot example: use .game instead of magic 0 Test fixes: - HTTPClientTests: mock paths corrected to /api/v10/ prefix - HTTPClientTests: EventCapture actor for Sendable-safe capture - CacheTests: ChannelType.guildText->.text, async nil checks - TestFixtures: Channel fixture simplified, .guildPublicThread->.publicThread - MockTransport: @unchecked Sendable, DiscordError.http args, NSLock safety - InternalTests: RetryPolicy tests use jitter:0 for deterministic results
The deprecated compnerd/gha-setup-swift@main action was causing CI failures on all platforms. Replaced with swift-actions/setup-swift@v2 which is the actively maintained Swift setup action for GitHub Actions. Also simplified test scripts to use direct 'swift test' command instead of wrapper scripts that had Docker fallback and PowerShell compatibility issues on CI runners.
swift-actions/setup-swift@v2 does not support Windows. Use compnerd/gha-setup-swift@main specifically for windows-latest while swift-actions/setup-swift@v2 handles macOS and Ubuntu.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit